From 182250846bae9712216861fda59cc699e12f2cd9 Mon Sep 17 00:00:00 2001
From: Wido den Hollander <wido@widodh.nl>
Date: Thu, 30 Jul 2020 12:31:11 +0200
Subject: [PATCH] kvm/ceph: Only if a port number has been specified define in
 the XML

Ceph used to use port 6789 (no need to specify it), but with the messenger v2
from Ceph it switched to port 3300 while 6789 still works.

librados/librbd/libvirt will automatically figure out the ports to use if none is
specified.

Therefor there is no need for CloudStack to explicitely define the port in the XML
passed to Libvirt or Qemu.

Leave blank if no port number has been defined by the user.
---
 .../kvm/resource/LibvirtStoragePoolDef.java   |  7 +++++-
 .../kvm/storage/KVMPhysicalDisk.java          |  2 +-
 .../resource/LibvirtStoragePoolDefTest.java   | 23 ++++++++++++++++++-
 ...oudStackPrimaryDataStoreLifeCycleImpl.java |  2 +-
 4 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java
index 31fe88f36ab0..56519aed3a41 100644
--- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java
@@ -147,7 +147,12 @@ public String toString() {
         }
         if (_poolType == PoolType.RBD) {
             storagePoolBuilder.append("<source>\n");
-            storagePoolBuilder.append("<host name='" + _sourceHost + "' port='" + _sourcePort + "'/>\n");
+            if (_sourcePort > 0) {
+                storagePoolBuilder.append("<host name='" + _sourceHost + "' port='" + _sourcePort + "'/>\n");
+            } else {
+                storagePoolBuilder.append("<host name='" + _sourceHost + "'/>\n");
+            }
+
             storagePoolBuilder.append("<name>" + _sourceDir + "</name>\n");
             if (_authUsername != null) {
                 storagePoolBuilder.append("<auth username='" + _authUsername + "' type='" + _authType + "'>\n");
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMPhysicalDisk.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMPhysicalDisk.java
index eaa143ac29d6..221a3d7c1808 100644
--- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMPhysicalDisk.java
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMPhysicalDisk.java
@@ -28,7 +28,7 @@ public static String RBDStringBuilder(String monHost, int monPort, String authUs
 
         rbdOpts = "rbd:" + image;
         rbdOpts += ":mon_host=" + monHost;
-        if (monPort != 6789) {
+        if (monPort > 0) {
             rbdOpts += "\\\\:" + monPort;
         }
 
diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDefTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDefTest.java
index ec22e3fed4fe..a1a43447e1cc 100644
--- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDefTest.java
+++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDefTest.java
@@ -81,4 +81,25 @@ public void testRbdStoragePool() {
 
         assertEquals(expectedXml, pool.toString());
     }
-}
\ No newline at end of file
+
+    public void testRbdStoragePoolWithoutPort() {
+        PoolType type = PoolType.RBD;
+        String name = "myRBDPool";
+        String uuid = "30a5fb6f-7277-44ce-9065-67e2bfdb0ebb";
+        String host = "::1";
+        String dir  = "rbd";
+        String authUsername = "admin";
+        String secretUuid = "d0d616dd-3446-409e-84d7-44465e325b35";
+        AuthenticationType auth = AuthenticationType.CEPH;
+        int port = 0;
+
+        LibvirtStoragePoolDef pool = new LibvirtStoragePoolDef(type, name, uuid, host, port, dir, authUsername, auth, secretUuid);
+
+        String expectedXml = "<pool type='" + type.toString() + "'>\n<name>" + name + "</name>\n<uuid>" + uuid + "</uuid>\n" +
+                             "<source>\n<host name='" + host + "'/>\n<name>" + dir + "</name>\n" +
+                             "<auth username='" + authUsername + "' type='" + auth.toString() + "'>\n<secret uuid='" + secretUuid + "'/>\n" +
+                             "</auth>\n</source>\n</pool>\n";
+
+        assertEquals(expectedXml, pool.toString());
+    }
+}
diff --git a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
index a500fdb63543..bdaeddca4c02 100644
--- a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
+++ b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
@@ -242,7 +242,7 @@ public DataStore initialize(Map<String, Object> dsInfos) {
             parameters.setPath(hostPath.replaceFirst("/", ""));
         } else if (scheme.equalsIgnoreCase("rbd")) {
             if (port == -1) {
-                port = 6789;
+                port = 0;
             }
             parameters.setType(StoragePoolType.RBD);
             parameters.setHost(storageHost);