From c3554ec31dafbdfaa0ed646afb17a6f3378571f5 Mon Sep 17 00:00:00 2001
From: Wido den Hollander <wido@widodh.nl>
Date: Wed, 5 Aug 2020 10:14:40 +0200
Subject: [PATCH] kvm: For ceph only if a port number has been specified define
 in the XML (#4231)

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 31fe88f36ab..56519aed3a4 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 class LibvirtStoragePoolDef {
         }
         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 eaa143ac29d..221a3d7c180 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 class KVMPhysicalDisk {
 
         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 ec22e3fed4f..a1a43447e1c 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 class LibvirtStoragePoolDefTest extends TestCase {
 
         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 a500fdb6354..bdaeddca4c0 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 class CloudStackPrimaryDataStoreLifeCycleImpl implements PrimaryDataStore
             parameters.setPath(hostPath.replaceFirst("/", ""));
         } else if (scheme.equalsIgnoreCase("rbd")) {
             if (port == -1) {
-                port = 6789;
+                port = 0;
             }
             parameters.setType(StoragePoolType.RBD);
             parameters.setHost(storageHost);