From 750b17f0b2b32b1a7e4e86ed33653166a326f6b1 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 14 Oct 2026 15:07:15 +0200 Subject: [PATCH 1/3] utils: add RbdUtils for the Ceph monitors of an RBD pool Everything that talks to Ceph needs the pool's monitors as a list the client can parse, and the rules are the same wherever that list is built: give each monitor the pool's port unless it names one already, and put a bare IPv6 address in square brackets so its own colons stay distinguishable from the separator before the port. The mon_host option of a disk's rbd: string wants the same list again, with ':' and ';' escaped, because those two separate the options of that string and its monitors. None of it depends on a storage pool, so collect it in RbdUtils as static methods with their unit tests, rather than leaving each caller to grow a copy that is right in a different set of cases. --- .../main/java/com/cloud/utils/RbdUtils.java | 78 ++++++++++ .../java/com/cloud/utils/RbdUtilsTest.java | 143 ++++++++++++++++++ 2 files changed, 221 insertions(+) create mode 100644 utils/src/main/java/com/cloud/utils/RbdUtils.java create mode 100644 utils/src/test/java/com/cloud/utils/RbdUtilsTest.java diff --git a/utils/src/main/java/com/cloud/utils/RbdUtils.java b/utils/src/main/java/com/cloud/utils/RbdUtils.java new file mode 100644 index 000000000000..33b40d4b1ac9 --- /dev/null +++ b/utils/src/main/java/com/cloud/utils/RbdUtils.java @@ -0,0 +1,78 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package com.cloud.utils; + +import java.util.ArrayList; +import java.util.List; + +import org.apache.commons.lang3.StringUtils; + +public class RbdUtils { + + private RbdUtils() { + } + + /** + * Returns the Ceph monitors as expected by "--mon-host": the comma-separated monitors of the pool, trimmed + * and without empty entries. If the pool has a monitor port, it is added to each monitor which has none yet. + * + * @param sourceHost the comma-separated monitors, IPv4 or IPv6 addresses or host names + * @param sourcePort the port of the monitors, or 0 (or less) if the pool has none + */ + public static String getRbdMonitors(String sourceHost, int sourcePort) { + List monitors = new ArrayList<>(); + for (String monitor : sourceHost.split(",")) { + monitor = monitor.trim(); + if (monitor.isEmpty()) { + continue; + } + monitors.add(sourcePort > 0 ? addPortToRbdMonitor(monitor, sourcePort) : monitor); + } + return String.join(",", monitors); + } + + public static String addPortToRbdMonitor(String monitor, int port) { + if (monitor.startsWith("[")) { + // IPv6 address in square brackets, which has a port if followed by ":" + return monitor.contains("]:") ? monitor : monitor + ":" + port; + } + int colons = StringUtils.countMatches(monitor, ":"); + if (colons == 0) { + return monitor + ":" + port; + } + if (colons == 1) { + // IPv4 address or host name, with a port + return monitor; + } + // IPv6 address without square brackets, so without a port + return "[" + monitor + "]:" + port; + } + + /** + * The monitor list as the {@code rbd:} option string of a disk needs it: ':' separates the + * options of that string and ';' its monitors, so both are escaped inside the list, as is + * every ':' of an IPv6 address. + */ + public static String composeOptionForMonHosts(String monHost, int monPort) { + List hosts = new ArrayList<>(); + for (String monitor : getRbdMonitors(monHost, monPort).split(",")) { + hosts.add(monitor.replace(":", "\\:")); + } + return StringUtils.join(hosts, "\\;"); + } +} diff --git a/utils/src/test/java/com/cloud/utils/RbdUtilsTest.java b/utils/src/test/java/com/cloud/utils/RbdUtilsTest.java new file mode 100644 index 000000000000..9fef0266d362 --- /dev/null +++ b/utils/src/test/java/com/cloud/utils/RbdUtilsTest.java @@ -0,0 +1,143 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package com.cloud.utils; + +import static org.junit.Assert.assertEquals; + +import org.junit.Test; + +public class RbdUtilsTest { + + @Test + public void testRbdMonitorsWithoutPort() { + assertEquals("10.0.0.1", RbdUtils.getRbdMonitors("10.0.0.1", 0)); + assertEquals("10.0.0.1,10.0.0.2,10.0.0.3", RbdUtils.getRbdMonitors("10.0.0.1,10.0.0.2,10.0.0.3", 0)); + assertEquals("fd00::1,fd00::2", RbdUtils.getRbdMonitors("fd00::1,fd00::2", 0)); + } + + @Test + public void testRbdMonitorsIpv4WithPort() { + assertEquals("10.0.0.1:6789", RbdUtils.getRbdMonitors("10.0.0.1", 6789)); + assertEquals("10.0.0.1:3300,10.0.0.2:3300,10.0.0.3:3300", RbdUtils.getRbdMonitors("10.0.0.1,10.0.0.2,10.0.0.3", 3300)); + } + + @Test + public void testRbdMonitorsIpv6WithPort() { + assertEquals("[fd00::1]:3300", RbdUtils.getRbdMonitors("fd00::1", 3300)); + assertEquals("[fd00::1]:3300,[fd00::2]:3300", RbdUtils.getRbdMonitors("fd00::1,fd00::2", 3300)); + // already enclosed in square brackets + assertEquals("[fd00::1]:3300,[fd00::2]:3300", RbdUtils.getRbdMonitors("[fd00::1],[fd00::2]", 3300)); + } + + @Test + public void testRbdMonitorsMixedIpv4AndIpv6WithPort() { + assertEquals("10.0.0.1:3300,[fd00::1]:3300,[fd00::2]:3300,mon4.example.com:3300", + RbdUtils.getRbdMonitors("10.0.0.1, fd00::1,[fd00::2] ,mon4.example.com", 3300)); + } + + @Test + public void testRbdMonitorsMultipleIpv4AndIpv6WithoutPort() { + // no port: the default Ceph port is used, so the monitors are passed as they are + assertEquals("[fd00::1],[fd00::2],[fd00::3]", RbdUtils.getRbdMonitors("[fd00::1],[fd00::2],[fd00::3]", 0)); + assertEquals("10.0.0.1,10.0.0.2,fd00::1,[fd00::2]", RbdUtils.getRbdMonitors("10.0.0.1,10.0.0.2,fd00::1,[fd00::2]", 0)); + assertEquals("10.0.0.1,fd00::1", RbdUtils.getRbdMonitors("10.0.0.1,fd00::1", -1)); + } + + @Test + public void testRbdMonitorsMultipleIpv4AndIpv6WithPort() { + assertEquals("10.0.0.1:6789,10.0.0.2:6789,[fd00::1]:6789,[fd00::2]:6789", + RbdUtils.getRbdMonitors("10.0.0.1,10.0.0.2,fd00::1,[fd00::2]", 6789)); + assertEquals("[fd00::1]:6789,10.0.0.1:6789,[fd00::2]:6789,10.0.0.2:6789", + RbdUtils.getRbdMonitors("fd00::1,10.0.0.1,[fd00::2],10.0.0.2", 6789)); + } + + @Test + public void testRbdMonitorsAreTrimmedAndEmptyEntriesSkipped() { + assertEquals("10.0.0.1,10.0.0.2,fd00::1", RbdUtils.getRbdMonitors(" 10.0.0.1, 10.0.0.2,,fd00::1 ,", 0)); + assertEquals("10.0.0.1:6789,10.0.0.2:6789,[fd00::1]:6789", RbdUtils.getRbdMonitors(" 10.0.0.1, 10.0.0.2,,fd00::1 ,", 6789)); + } + + @Test + public void testRbdMonitorsWhichHaveAPortAlready() { + // the monitors keep their port, with or without a port of the pool + String withPorts = "10.0.0.1:6789,mon.example.com:3300,[fd00::1]:6789"; + assertEquals(withPorts, RbdUtils.getRbdMonitors(withPorts, 0)); + assertEquals(withPorts, RbdUtils.getRbdMonitors(withPorts, 6789)); + assertEquals(withPorts, RbdUtils.getRbdMonitors(withPorts, 3300)); + } + + @Test + public void testRbdMonitorsWithAndWithoutAPortAreMixed() { + assertEquals("10.0.0.1:6789,10.0.0.2:3300,mon.example.com:3300,[fd00::1]:3300,[fd00::2]:6789,[fd00::3]:3300", + RbdUtils.getRbdMonitors("10.0.0.1:6789,10.0.0.2,mon.example.com,fd00::1,[fd00::2]:6789,[fd00::3]", 3300)); + } + + @Test + public void testAddPortToRbdMonitor() { + assertEquals("10.0.0.1:6789", RbdUtils.addPortToRbdMonitor("10.0.0.1", 6789)); + assertEquals("10.0.0.1:3300", RbdUtils.addPortToRbdMonitor("10.0.0.1:3300", 6789)); + assertEquals("mon.example.com:6789", RbdUtils.addPortToRbdMonitor("mon.example.com", 6789)); + assertEquals("[fd00::1]:6789", RbdUtils.addPortToRbdMonitor("fd00::1", 6789)); + assertEquals("[fd00::1]:6789", RbdUtils.addPortToRbdMonitor("[fd00::1]", 6789)); + assertEquals("[fd00::1]:3300", RbdUtils.addPortToRbdMonitor("[fd00::1]:3300", 6789)); + } + + @Test + public void testComposeOptionForMonHostsEscapesThePortSeparator() { + // ':' separates the options of an rbd: string, so the one before the port is escaped. + assertEquals("ceph-monitor\\:8000", RbdUtils.composeOptionForMonHosts("ceph-monitor", 8000)); + } + + @Test + public void testComposeOptionForMonHostsJoinsMonitorsWithAnEscapedSemicolon() { + assertEquals("mon1\\:3300\\;mon2\\:3300\\;mon3\\:3300", + RbdUtils.composeOptionForMonHosts("mon1,mon2,mon3", 3300)); + } + + @Test + public void testComposeOptionForMonHostsEscapesEveryColonOfAnIpv6Monitor() { + // Without this the address' own colons would read as option separators. + assertEquals("[fc00\\:1234\\:\\:1]\\:3300", + RbdUtils.composeOptionForMonHosts("[fc00:1234::1]", 3300)); + } + + @Test + public void testComposeOptionForMonHostsWithoutAPortLeavesTheMonitorAlone() { + assertEquals("ceph-monitor", RbdUtils.composeOptionForMonHosts("ceph-monitor", 0)); + } + + @Test + public void testComposeOptionForMonHostsMixesIpv4AndIpv6() { + assertEquals("10.0.0.1\\:3300\\;[fc00\\:1234\\:\\:1]\\:3300", + RbdUtils.composeOptionForMonHosts("10.0.0.1,[fc00:1234::1]", 3300)); + } + + @Test + public void testComposeOptionForMonHostsBracketsABareIpv6MonitorBeforeEscapingIt() { + // Inherited from getRbdMonitors: without the brackets the address' own colons would be + // indistinguishable from the one before the port. + assertEquals("[fd00\\:\\:1]\\:3300", + RbdUtils.composeOptionForMonHosts("fd00::1", 3300)); + } + + @Test + public void testComposeOptionForMonHostsLeavesAMonitorWhichHasAPortAlready() { + // Inherited from getRbdMonitors: the pool's port is not appended a second time. + assertEquals("mon1\\:6789", RbdUtils.composeOptionForMonHosts("mon1:6789", 3300)); + } +} From 1f3f501861b40ecf4ab148e152b4f75ed2a71c40 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 14 Oct 2026 15:07:16 +0200 Subject: [PATCH 2/3] kvm: build every Ceph monitor list with RbdUtils The agent assembled the monitor list in several places, each with its own idea of the rules. Opening a RADOS connection, or writing a ceph.conf for one, appended the pool's port to the host string as a whole, so with several monitors only the last was given it and a monitor that already named its port was handed a second. The rbd: option string of a disk escaped the colons of an IPv6 address only when it was bracketed already, which left a bare one reading as though it carried options of its own. Use the shared helpers for all of them. Two inputs therefore come out differently, both previously wrong: a monitor that names its port keeps just that one, and a bare IPv6 address is bracketed before anything is appended to it or escaped in it. The iscsiadm portals in IscsiAdmStorageAdaptor read the same way but are not monitor lists, so they keep their own formatting. --- .../LibvirtBackupSnapshotCommandWrapper.java | 3 ++- .../LibvirtManageSnapshotCommandWrapper.java | 3 ++- .../LibvirtRevertSnapshotCommandWrapper.java | 3 ++- .../kvm/storage/KVMPhysicalDisk.java | 25 ++----------------- .../kvm/storage/KVMStorageProcessor.java | 5 ++-- .../kvm/storage/LibvirtStorageAdaptor.java | 13 +++++----- .../cloudstack/utils/rbd/RbdEncryption.java | 3 ++- 7 files changed, 20 insertions(+), 35 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtBackupSnapshotCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtBackupSnapshotCommandWrapper.java index 964e6591878c..9d47f497196e 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtBackupSnapshotCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtBackupSnapshotCommandWrapper.java @@ -47,6 +47,7 @@ import com.cloud.resource.CommandWrapper; import com.cloud.resource.ResourceWrapper; import com.cloud.storage.Storage.StoragePoolType; +import com.cloud.utils.RbdUtils; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.Script; @@ -98,7 +99,7 @@ public Answer execute(final BackupSnapshotCommand command, final LibvirtComputin if (primaryPool.getType() == StoragePoolType.RBD) { try { final Rados r = new Rados(primaryPool.getAuthUserName()); - r.confSet("mon_host", primaryPool.getSourceHost() + ":" + primaryPool.getSourcePort()); + r.confSet("mon_host", RbdUtils.getRbdMonitors(primaryPool.getSourceHost(), primaryPool.getSourcePort())); r.confSet("key", primaryPool.getAuthSecret()); r.confSet("client_mount_timeout", "30"); r.connect(); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtManageSnapshotCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtManageSnapshotCommandWrapper.java index ec900e9981e0..792737db1cf7 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtManageSnapshotCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtManageSnapshotCommandWrapper.java @@ -43,6 +43,7 @@ import com.cloud.resource.CommandWrapper; import com.cloud.resource.ResourceWrapper; import com.cloud.storage.Storage.StoragePoolType; +import com.cloud.utils.RbdUtils; import com.cloud.utils.script.Script; @ResourceWrapper(handles = ManageSnapshotCommand.class) @@ -115,7 +116,7 @@ public Answer execute(final ManageSnapshotCommand command, final LibvirtComputin if (primaryPool.getType() == StoragePoolType.RBD) { try { final Rados r = new Rados(primaryPool.getAuthUserName()); - r.confSet("mon_host", primaryPool.getSourceHost() + ":" + primaryPool.getSourcePort()); + r.confSet("mon_host", RbdUtils.getRbdMonitors(primaryPool.getSourceHost(), primaryPool.getSourcePort())); r.confSet("key", primaryPool.getAuthSecret()); r.confSet("client_mount_timeout", "30"); r.connect(); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java index 507744fdc316..e27b8460f624 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertSnapshotCommandWrapper.java @@ -51,6 +51,7 @@ import com.cloud.resource.ResourceWrapper; import com.cloud.storage.DataStoreRole; import com.cloud.storage.Storage.StoragePoolType; +import com.cloud.utils.RbdUtils; import com.cloud.utils.Pair; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.Script; @@ -96,7 +97,7 @@ public Answer execute(final RevertSnapshotCommand command, final LibvirtComputin if (primaryPool.getType() == StoragePoolType.RBD) { Rados rados = new Rados(primaryPool.getAuthUserName()); - rados.confSet(MON_HOST, primaryPool.getSourceHost() + ":" + primaryPool.getSourcePort()); + rados.confSet(MON_HOST, RbdUtils.getRbdMonitors(primaryPool.getSourceHost(), primaryPool.getSourcePort())); rados.confSet(KEY, primaryPool.getAuthSecret()); rados.confSet(CLIENT_MOUNT_TIMEOUT, RADOS_CONNECTION_TIMEOUT); rados.connect(); 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 b3b55f484e4d..2cc650520c49 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 @@ -16,6 +16,7 @@ // under the License. package com.cloud.hypervisor.kvm.storage; +import com.cloud.utils.RbdUtils; import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.storage.formatinspector.Qcow2Inspector; import org.apache.cloudstack.utils.imagestore.ImageStoreUtil; @@ -25,11 +26,8 @@ import org.apache.cloudstack.utils.qemu.QemuImgFile; import org.apache.cloudstack.utils.qemu.QemuObject; import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; -import org.apache.commons.lang3.StringUtils; import org.libvirt.LibvirtException; -import java.util.ArrayList; -import java.util.List; import java.util.Map; public class KVMPhysicalDisk { @@ -51,7 +49,7 @@ public static String RBDStringBuilder(KVMStoragePool storagePool, String image) String dataPool = (details == null) ? null : details.get(RBD_DEFAULT_DATA_POOL); String rbdOpts = "rbd:" + image; - rbdOpts += ":mon_host=" + composeOptionForMonHosts(monHost, monPort); + rbdOpts += ":mon_host=" + RbdUtils.composeOptionForMonHosts(monHost, monPort); if (authUserName == null) { rbdOpts += ":auth_client_required=none"; @@ -71,25 +69,6 @@ public static String RBDStringBuilder(KVMStoragePool storagePool, String image) return rbdOpts; } - private static String composeOptionForMonHosts(String monHost, int monPort) { - List hosts = new ArrayList<>(); - for (String host : monHost.split(",")) { - if (monPort > 0) { - hosts.add(replaceHostAddress(host) + "\\:" + monPort); - } else { - hosts.add(replaceHostAddress(host)); - } - } - return StringUtils.join(hosts, "\\;"); - } - - private static String replaceHostAddress(String hostIp) { - if (hostIp != null && hostIp.startsWith("[") && hostIp.endsWith("]")) { - return hostIp.replaceAll("\\:", "\\\\:"); - } - return hostIp; - } - public static long getVirtualSizeFromFile(String path) { try { QemuImg qemu = new QemuImg(0); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 0cb32da318ea..27a43c54df3f 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -156,6 +156,7 @@ import com.cloud.storage.template.QCOW2Processor; import com.cloud.storage.template.TemplateConstants; import com.cloud.storage.template.TemplateLocation; +import com.cloud.utils.RbdUtils; import com.cloud.utils.Pair; import com.cloud.utils.UriUtils; import com.cloud.utils.exception.CloudRuntimeException; @@ -2911,7 +2912,7 @@ protected boolean isAvailablePoolSizeDividedByDiskSizeLesserThanMinRate(long ava protected Rados radosConnect(final KVMStoragePool primaryPool) throws RadosException { Rados r = new Rados(primaryPool.getAuthUserName()); - r.confSet(CEPH_MON_HOST, primaryPool.getSourceHost() + ":" + primaryPool.getSourcePort()); + r.confSet(CEPH_MON_HOST, RbdUtils.getRbdMonitors(primaryPool.getSourceHost(), primaryPool.getSourcePort())); r.confSet(CEPH_AUTH_KEY, primaryPool.getAuthSecret()); r.confSet(CEPH_CLIENT_MOUNT_TIMEOUT, CEPH_DEFAULT_MOUNT_TIMEOUT); r.connect(); @@ -3128,7 +3129,7 @@ private KVMPhysicalDisk createRBDvolumeFromRBDSnapshot(KVMPhysicalDisk volume, S try { r = new Rados(srcPool.getAuthUserName()); - r.confSet("mon_host", srcPool.getSourceHost() + ":" + srcPool.getSourcePort()); + r.confSet("mon_host", RbdUtils.getRbdMonitors(srcPool.getSourceHost(), srcPool.getSourcePort())); r.confSet("key", srcPool.getAuthSecret()); r.confSet("client_mount_timeout", "30"); r.connect(); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index 8a1a7b5bbe2e..23e78e9e01a9 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -77,6 +77,7 @@ import com.cloud.storage.Storage; import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.StorageLayer; +import com.cloud.utils.RbdUtils; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.Script; import com.cloud.utils.storage.TemplateDownloaderUtil; @@ -1353,7 +1354,7 @@ private KVMPhysicalDisk createDiskFromTemplateOnRBD(KVMPhysicalDisk template, logger.debug("Trying to perform a RBD clone (layering) since we are operating in the same storage pool"); Rados r = new Rados(srcPool.getAuthUserName()); - r.confSet("mon_host", srcPool.getSourceHost() + ":" + srcPool.getSourcePort()); + r.confSet("mon_host", RbdUtils.getRbdMonitors(srcPool.getSourceHost(), srcPool.getSourcePort())); r.confSet("key", srcPool.getAuthSecret()); r.confSet("client_mount_timeout", "30"); if (dataPool != null) { @@ -1426,14 +1427,14 @@ private KVMPhysicalDisk createDiskFromTemplateOnRBD(KVMPhysicalDisk template, logger.debug("Both the source and destination are RBD, but not the same Ceph cluster. Performing a copy"); Rados rSrc = new Rados(srcPool.getAuthUserName()); - rSrc.confSet("mon_host", srcPool.getSourceHost() + ":" + srcPool.getSourcePort()); + rSrc.confSet("mon_host", RbdUtils.getRbdMonitors(srcPool.getSourceHost(), srcPool.getSourcePort())); rSrc.confSet("key", srcPool.getAuthSecret()); rSrc.confSet("client_mount_timeout", "30"); rSrc.connect(); logger.debug("Successfully connected to source Ceph cluster at " + rSrc.confGet("mon_host")); Rados rDest = new Rados(destPool.getAuthUserName()); - rDest.confSet("mon_host", destPool.getSourceHost() + ":" + destPool.getSourcePort()); + rDest.confSet("mon_host", RbdUtils.getRbdMonitors(destPool.getSourceHost(), destPool.getSourcePort())); rDest.confSet("key", destPool.getAuthSecret()); rDest.confSet("client_mount_timeout", "30"); if (dataPool != null) { @@ -1498,7 +1499,7 @@ private KVMPhysicalDisk createEncryptedRootCoWClone(KVMPhysicalDisk template, KV RbdImage templateImage = null; try { radosConnection = new Rados(destPool.getAuthUserName()); - radosConnection.confSet("mon_host", destPool.getSourceHost() + ":" + destPool.getSourcePort()); + radosConnection.confSet("mon_host", RbdUtils.getRbdMonitors(destPool.getSourceHost(), destPool.getSourcePort())); radosConnection.confSet("key", destPool.getAuthSecret()); radosConnection.confSet("client_mount_timeout", "30"); radosConnection.connect(); @@ -1559,7 +1560,7 @@ private KVMPhysicalDisk createEncryptedRootFullCopy(KVMStoragePool srcPool, KVMP IoCTX ioContext = null; try { radosConnection = new Rados(destPool.getAuthUserName()); - radosConnection.confSet("mon_host", destPool.getSourceHost() + ":" + destPool.getSourcePort()); + radosConnection.confSet("mon_host", RbdUtils.getRbdMonitors(destPool.getSourceHost(), destPool.getSourcePort())); radosConnection.confSet("key", destPool.getAuthSecret()); radosConnection.confSet("client_mount_timeout", "30"); radosConnection.connect(); @@ -1738,7 +1739,7 @@ to support snapshots(backuped) as qcow2 files. */ /* We have to stat the RBD image to see how big it became afterwards */ Rados r = new Rados(destPool.getAuthUserName()); - r.confSet("mon_host", destPool.getSourceHost() + ":" + destPool.getSourcePort()); + r.confSet("mon_host", RbdUtils.getRbdMonitors(destPool.getSourceHost(), destPool.getSourcePort())); r.confSet("key", destPool.getAuthSecret()); r.confSet("client_mount_timeout", "30"); r.connect(); diff --git a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/rbd/RbdEncryption.java b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/rbd/RbdEncryption.java index a23ce93d2b82..e932cc889742 100644 --- a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/rbd/RbdEncryption.java +++ b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/rbd/RbdEncryption.java @@ -16,6 +16,7 @@ // under the License. package org.apache.cloudstack.utils.rbd; +import com.cloud.utils.RbdUtils; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.Script; import org.apache.cloudstack.utils.cryptsetup.CryptSetup; @@ -67,7 +68,7 @@ public RbdEncryption(String commandPath) { } private static String monSpec(String monHost, int monPort) { - return monPort > 0 ? monHost + ":" + monPort : monHost; + return RbdUtils.getRbdMonitors(monHost, monPort); } /** From 708ba29888dd2da74609a74fbd3dd201e5e272af Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Thu, 15 Oct 2026 13:56:53 +0200 Subject: [PATCH 3/3] utils: test the escaped colons of an IPv6 Ceph monitor without a port composeOptionForMonHosts escapes every ':' of a monitor, because the colons separate the options of the rbd: string. That includes the colons of an IPv6 address, also when the pool has no port to add. Test that case, and make addPortToRbdMonitor, which is only used by RbdUtils itself, protected. --- utils/src/main/java/com/cloud/utils/RbdUtils.java | 2 +- utils/src/test/java/com/cloud/utils/RbdUtilsTest.java | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/utils/src/main/java/com/cloud/utils/RbdUtils.java b/utils/src/main/java/com/cloud/utils/RbdUtils.java index 33b40d4b1ac9..9e8ba7ed7b3d 100644 --- a/utils/src/main/java/com/cloud/utils/RbdUtils.java +++ b/utils/src/main/java/com/cloud/utils/RbdUtils.java @@ -46,7 +46,7 @@ public static String getRbdMonitors(String sourceHost, int sourcePort) { return String.join(",", monitors); } - public static String addPortToRbdMonitor(String monitor, int port) { + protected static String addPortToRbdMonitor(String monitor, int port) { if (monitor.startsWith("[")) { // IPv6 address in square brackets, which has a port if followed by ":" return monitor.contains("]:") ? monitor : monitor + ":" + port; diff --git a/utils/src/test/java/com/cloud/utils/RbdUtilsTest.java b/utils/src/test/java/com/cloud/utils/RbdUtilsTest.java index 9fef0266d362..6419f4e42f42 100644 --- a/utils/src/test/java/com/cloud/utils/RbdUtilsTest.java +++ b/utils/src/test/java/com/cloud/utils/RbdUtilsTest.java @@ -119,6 +119,7 @@ public void testComposeOptionForMonHostsEscapesEveryColonOfAnIpv6Monitor() { @Test public void testComposeOptionForMonHostsWithoutAPortLeavesTheMonitorAlone() { assertEquals("ceph-monitor", RbdUtils.composeOptionForMonHosts("ceph-monitor", 0)); + assertEquals("fd00\\:\\:1", RbdUtils.composeOptionForMonHosts("fd00::1", 0)); } @Test