kvm: For ceph only if a port number has been specified define in the XML (apache#4231)

wido · web-flow · commit c3554ec31daf · 2020-08-05T13:44:40.000+05:30
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.
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
@@ -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
@@ -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
@@ -81,4 +81,25 @@ public void testRbdStoragePool() {
 
         assertEquals(expectedXml, pool.toString());
     }
-}
+
+    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
@@ -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);

Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,7 @@ public static String RBDStringBuilder(String monHost, int monPort, String authUs`
`28`	`28`
`29`	`29`	`rbdOpts = "rbd:" + image;`
`30`	`30`	`rbdOpts += ":mon_host=" + monHost;`
`31`		`- if (monPort != 6789) {`
	`31`	`+ if (monPort > 0) {`
`32`	`32`	`rbdOpts += "\\\\:" + monPort;`
`33`	`33`	`}`
`34`	`34`
Original file line number	Diff line number	Diff line change
`@@ -242,7 +242,7 @@ public DataStore initialize(Map<String, Object> dsInfos) {`
`242`	`242`	`parameters.setPath(hostPath.replaceFirst("/", ""));`
`243`	`243`	`} else if (scheme.equalsIgnoreCase("rbd")) {`
`244`	`244`	`if (port == -1) {`
`245`		`- port = 6789;`
	`245`	`+ port = 0;`
`246`	`246`	`}`
`247`	`247`	`parameters.setType(StoragePoolType.RBD);`
`248`	`248`	`parameters.setHost(storageHost);`