From efe1db7cd1f6d4d4e10ee18c9ea34770c80c6605 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 14 Oct 2026 15:07:15 +0200 Subject: [PATCH 1/4] 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 6cc5a182ea313c48637560e8cbede8e20f0ce23b Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 7 Oct 2026 17:21:53 +0200 Subject: [PATCH 2/4] KVM: live volume migration on ceph - enable migrate volume between ceph - regenerate disk xml for virsh blockcopy - use SHALLOW flag instead of REUSE_EXT --- .../api/storage/MigrateVolumeCommand.java | 8 + .../motion/AncientDataMotionStrategy.java | 2 + .../endpoint/DefaultEndPointSelector.java | 2 +- .../LibvirtMigrateVolumeCommandWrapper.java | 207 +++++++++++++++++- .../kvm/storage/KVMStoragePool.java | 4 + .../kvm/storage/LibvirtStorageAdaptor.java | 1 + .../kvm/storage/LibvirtStoragePool.java | 10 + .../cloud/storage/VolumeApiServiceImpl.java | 16 +- 8 files changed, 244 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/com/cloud/agent/api/storage/MigrateVolumeCommand.java b/core/src/main/java/com/cloud/agent/api/storage/MigrateVolumeCommand.java index 70375c30a1bb..5559057ecd24 100644 --- a/core/src/main/java/com/cloud/agent/api/storage/MigrateVolumeCommand.java +++ b/core/src/main/java/com/cloud/agent/api/storage/MigrateVolumeCommand.java @@ -124,6 +124,14 @@ public DataTO getDestData() { return destData; } + public void setSrcData(DataTO srcData) { + this.srcData = srcData; + } + + public void setDestData(DataTO destData) { + this.destData = destData; + } + public void setSrcDetails(Map details) { srcDetails = details; } diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java index 78d720737b3e..05ab5374af5b 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java @@ -642,6 +642,8 @@ protected Answer migrateVolumeToPool(DataObject srcData, DataObject destData) { command.setContextParam(DiskTO.PROTOCOL_TYPE, Storage.StoragePoolType.DatastoreCluster.toString()); } + command.setSrcData(srcData.getTO()); + command.setDestData(destData.getTO()); boolean encryptionRequired = anyVolumeRequiresEncryption(srcData, destData); EndPoint ep = selector.select(srcData, StorageAction.MIGRATEVOLUME); diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java index 0ce2cedb944c..5e63fc8d1311 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java @@ -603,7 +603,7 @@ public EndPoint select(DataObject object, StorageAction action, boolean encrypti } case MIGRATEVOLUME: { VolumeInfo volume = (VolumeInfo) object; - if (volume.getHypervisorType() == Hypervisor.HypervisorType.Hyperv || volume.getHypervisorType() == Hypervisor.HypervisorType.VMware) { + if (List.of(Hypervisor.HypervisorType.VMware, Hypervisor.HypervisorType.Hyperv, Hypervisor.HypervisorType.KVM).contains(volume.getHypervisorType())) { VirtualMachine vm = volume.getAttachedVM(); if ((vm != null) && (vm.getState() == VirtualMachine.State.Running)) { Long hostId = vm.getHostId(); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateVolumeCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateVolumeCommandWrapper.java index 85574c447417..de3a07010ffc 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateVolumeCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateVolumeCommandWrapper.java @@ -26,6 +26,7 @@ import com.cloud.agent.api.to.DiskTO; import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; import com.cloud.hypervisor.kvm.resource.disconnecthook.VolumeMigrationCancelHook; +import com.cloud.hypervisor.kvm.resource.LibvirtVMDef; import com.cloud.hypervisor.kvm.storage.KVMPhysicalDisk; import com.cloud.hypervisor.kvm.storage.KVMStoragePool; import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager; @@ -38,6 +39,7 @@ import java.io.IOException; import java.io.StringWriter; import java.util.Map; +import java.util.Objects; import java.util.UUID; import org.apache.cloudstack.storage.datastore.client.ScaleIOGatewayClient; @@ -56,6 +58,8 @@ import org.libvirt.LibvirtException; import org.w3c.dom.Document; import org.w3c.dom.Element; +import org.w3c.dom.NamedNodeMap; +import org.w3c.dom.Node; import org.w3c.dom.NodeList; import org.xml.sax.SAXException; @@ -78,8 +82,10 @@ public Answer execute(final MigrateVolumeCommand command, final LibvirtComputing PrimaryDataStoreTO srcPrimaryDataStore = (PrimaryDataStoreTO)srcVolumeObjectTO.getDataStore(); MigrateVolumeAnswer answer; - if (srcPrimaryDataStore.getPoolType().equals(Storage.StoragePoolType.PowerFlex)) { + if (Objects.equals(Storage.StoragePoolType.PowerFlex, srcPrimaryDataStore.getPoolType())) { answer = migratePowerFlexVolume(command, libvirtComputingResource); + } else if (Objects.equals(Storage.StoragePoolType.RBD, srcPrimaryDataStore.getPoolType())) { + answer = migrateRBDVolume(command, libvirtComputingResource); } else { answer = migrateRegularVolume(command, libvirtComputingResource); } @@ -134,7 +140,7 @@ protected MigrateVolumeAnswer migratePowerFlexVolume(final MigrateVolumeCommand destSecretUUID = libvirtComputingResource.createLibvirtVolumeSecret(conn, destVolumeObjectTO.getPath(), destVolumeObjectTO.getPassphrase()); } - String diskdef = generateDestinationDiskXML(dm, srcVolumeId, diskFilePath, destSecretUUID); + String diskdef = generateDestinationDiskXMLForPowerFlex(dm, srcVolumeId, diskFilePath, destSecretUUID); destDiskLabel = generateDestinationDiskLabel(diskdef); TypedUlongParameter parameter = new TypedUlongParameter("bandwidth", 0); @@ -249,7 +255,7 @@ private String generateDestinationDiskLabel(String diskXml) throws ParserConfigu return diskLabel; } - protected String generateDestinationDiskXML(Domain dm, String srcVolumeId, String diskFilePath, String destSecretUUID) throws LibvirtException, ParserConfigurationException, IOException, TransformerException, SAXException { + protected String generateDestinationDiskXMLForPowerFlex(Domain dm, String srcVolumeId, String diskFilePath, String destSecretUUID) throws LibvirtException, ParserConfigurationException, IOException, TransformerException, SAXException { final String domXml = dm.getXMLDesc(0); DocumentBuilderFactory dbFactory = ParserUtils.getSaferDocumentBuilderFactory(); @@ -374,4 +380,199 @@ protected MigrateVolumeAnswer migrateRegularVolume(final MigrateVolumeCommand co return new MigrateVolumeAnswer(command, true, null, destPath); } + + + protected MigrateVolumeAnswer migrateRBDVolume(final MigrateVolumeCommand command, final LibvirtComputingResource libvirtComputingResource) { + final String vmName = command.getAttachedVmName(); + + VolumeObjectTO srcVolumeObjectTO = (VolumeObjectTO)command.getSrcData(); + PrimaryDataStoreTO sourcePool = (PrimaryDataStoreTO)srcVolumeObjectTO.getDataStore(); + final String sourceFilePath = String.format("%s/%s", sourcePool.getPath(), srcVolumeObjectTO.getPath()); + + VolumeObjectTO destVolumeObjectTO = (VolumeObjectTO)command.getDestData(); + PrimaryDataStoreTO destPool = (PrimaryDataStoreTO)destVolumeObjectTO.getDataStore(); + final String destFilePath = String.format("%s/%s", destPool.getPath(), destVolumeObjectTO.getPath()); + + String destDiskLabel = null; + + VolumeMigrationCancelHook cancelHook = null; + + final KVMStoragePoolManager storagePoolMgr = libvirtComputingResource.getStoragePoolMgr(); + + Domain dm = null; + MigrateVolumeAnswer answer = null; + try { + final LibvirtUtilitiesHelper libvirtUtilitiesHelper = libvirtComputingResource.getLibvirtUtilitiesHelper(); + Connect conn = libvirtUtilitiesHelper.getConnection(); + dm = libvirtComputingResource.getDomain(conn, vmName); + if (dm == null) { + return new MigrateVolumeAnswer(command, false, "Migrate volume failed due to can not find vm: " + vmName, null); + } + + DomainInfo.DomainState domainState = dm.getInfo().state ; + if (domainState != DomainInfo.DomainState.VIR_DOMAIN_RUNNING) { + return new MigrateVolumeAnswer(command, false, "Migrate volume failed due to VM is not running: " + vmName + " with domainState = " + domainState, null); + } + + KVMStoragePool pool = storagePoolMgr.getStoragePool(destPool.getPoolType(), destPool.getUuid()); + pool.connectPhysicalDisk(destVolumeObjectTO.getPath(), null); + + String srcSecretUUID = null; + String destSecretUUID = null; + if (ArrayUtils.isNotEmpty(destVolumeObjectTO.getPassphrase())) { + srcSecretUUID = libvirtComputingResource.createLibvirtVolumeSecret(conn, srcVolumeObjectTO.getPath(), srcVolumeObjectTO.getPassphrase()); + destSecretUUID = libvirtComputingResource.createLibvirtVolumeSecret(conn, destVolumeObjectTO.getPath(), destVolumeObjectTO.getPassphrase()); + } + + String diskdef = generateDestinationDiskXMLForRBD(dm, sourceFilePath, destFilePath, destSecretUUID, pool); + destDiskLabel = generateDestinationDiskLabel(diskdef); + + TypedUlongParameter parameter = new TypedUlongParameter("bandwidth", 0); + TypedParameter[] parameters = new TypedParameter[1]; + parameters[0] = parameter; + + cancelHook = new VolumeMigrationCancelHook(dm, destDiskLabel); + libvirtComputingResource.addDisconnectHook(cancelHook); + + libvirtComputingResource.createOrUpdateLogFileForCommand(command, Command.State.PROCESSING_IN_BACKEND); + + dm.blockCopy(destDiskLabel, diskdef, parameters, Domain.BlockCopyFlags.SHALLOW); + logger.info(String.format("Block copy has started for the volume %s : %s to %s", destDiskLabel, sourceFilePath, destFilePath)); + + answer = checkBlockJobStatus(command, dm, destDiskLabel, sourceFilePath, destFilePath, libvirtComputingResource, conn, srcSecretUUID); + if (answer != null) { + if (answer.getResult()) { + libvirtComputingResource.createOrUpdateLogFileForCommand(command, Command.State.COMPLETED); + } else { + libvirtComputingResource.createOrUpdateLogFileForCommand(command, Command.State.FAILED); + } + } + if (answer != null && answer.getResult()) { + answer = new MigrateVolumeAnswer(command, true, null, destVolumeObjectTO.getPath()); + } + return answer; + } catch (Exception e) { + String msg = "Migrate volume failed due to " + e.toString(); + logger.warn(msg, e); + if (destDiskLabel != null) { + try { + dm.blockJobAbort(destDiskLabel, Domain.BlockJobAbortFlags.ASYNC); + } catch (LibvirtException ex) { + logger.error("Migrate volume failed while aborting the block job due to " + ex.getMessage()); + } + } + libvirtComputingResource.createOrUpdateLogFileForCommand(command, Command.State.FAILED); + return new MigrateVolumeAnswer(command, false, msg, null); + } finally { + if (cancelHook != null) { + libvirtComputingResource.removeDisconnectHook(cancelHook); + } + if (answer != null && answer.getResult()) { + // Remove old volume after successful migration + KVMStoragePool libvirtSourcePool = storagePoolMgr.getStoragePool(sourcePool.getPoolType(), sourcePool.getUuid()); + libvirtSourcePool.deletePhysicalDisk(srcVolumeObjectTO.getPath(), Storage.ImageFormat.RAW); + } + if (dm != null) { + try { + dm.free(); + } catch (LibvirtException l) { + logger.trace("Ignoring libvirt error.", l); + } + } + } + } + + protected String generateDestinationDiskXMLForRBD(Domain dm, String srcVolumePath, String diskFilePath, String destSecretUUID, KVMStoragePool pool) throws LibvirtException, ParserConfigurationException, IOException, TransformerException, SAXException { + final String domXml = dm.getXMLDesc(0); + + DocumentBuilderFactory dbFactory = DocumentBuilderFactory.newInstance(); + DocumentBuilder dBuilder = dbFactory.newDocumentBuilder(); + Document doc = dBuilder.parse(new ByteArrayInputStream(domXml.getBytes("UTF-8"))); + doc.getDocumentElement().normalize(); + + NodeList disks = doc.getElementsByTagName("disk"); + + for (int i = 0; i < disks.getLength(); i++) { + Node diskNode = disks.item(i); + Node diskTypeAttribute = diskNode.getAttributes().getNamedItem("type"); + if (diskTypeAttribute == null || !diskTypeAttribute.getNodeValue().equalsIgnoreCase(LibvirtVMDef.DiskDef.DiskType.NETWORK.toString())) { + continue; + } + + NodeList diskChildNodes = diskNode.getChildNodes(); + boolean found = false; + for (int j = 0; j < diskChildNodes.getLength(); j++) { + Node diskChildNode = diskChildNodes.item(j); + + if ("source".equals(diskChildNode.getNodeName())) { + NamedNodeMap diskNodeAttributes = diskChildNode.getAttributes(); + Node diskNodeAttribute = diskNodeAttributes.getNamedItem("protocol"); + if (diskNodeAttribute == null || !diskNodeAttribute.getNodeValue().equalsIgnoreCase(LibvirtVMDef.DiskDef.DiskProtocol.RBD.toString())) { + logger.debug("Skipped disk node with protocol: " + diskNodeAttribute); + continue; + } + diskNodeAttribute = diskNodeAttributes.getNamedItem("name"); + if (diskNodeAttribute == null || !diskNodeAttribute.getNodeValue().contains(srcVolumePath)) { + logger.debug("Skipped disk node with name: " + diskNodeAttribute); + continue; + } + + logger.debug(String.format("Found disk node with source RBD volume path %s: %s", srcVolumePath, diskNodeAttribute)); + diskNode.removeChild(diskChildNode); + Element newChildSourceNode = doc.createElement("source"); + newChildSourceNode.setAttribute("protocol", LibvirtVMDef.DiskDef.DiskProtocol.RBD.toString().toLowerCase()); + newChildSourceNode.setAttribute("name", diskFilePath); + for (String sourceHost : pool.getSourceHost().split(",")) { + Element newChildDiskNodeForHost = doc.createElement("host"); + newChildDiskNodeForHost.setAttribute("name", sourceHost); + if (pool.getSourcePort() != 0) { + newChildDiskNodeForHost.setAttribute("port", String.valueOf(pool.getSourcePort())); + } + newChildSourceNode.appendChild(newChildDiskNodeForHost); + } + diskNode.appendChild(newChildSourceNode); + + found = true; + break; + } + } + if (found) { + for (int j = 0; j < diskChildNodes.getLength(); j++) { + Node diskChildNode = diskChildNodes.item(j); + if ("auth".equals(diskChildNode.getNodeName())) { + NamedNodeMap diskNodeAttributes = diskChildNode.getAttributes(); + Node diskNodeAttribute = diskNodeAttributes.getNamedItem("username"); + if (diskNodeAttribute != null) { + diskNodeAttribute.setNodeValue(pool.getAuthUserName()); + } + for (int m = 0; m < diskChildNode.getChildNodes().getLength(); m++) { + Node authChild = diskChildNode.getChildNodes().item(m); + if ("secret".equals(authChild.getNodeName())) { + NamedNodeMap secretAttributes = authChild.getAttributes(); + Node uuidAttribute = secretAttributes.getNamedItem("uuid"); + uuidAttribute.setTextContent(pool.getAuthSecretUUID()); + } + } + } else if ("encryption".equals(diskChildNode.getNodeName())) { + for (int n = 0; n < diskChildNode.getChildNodes().getLength(); n++) { + Node encryptionChild = diskChildNode.getChildNodes().item(n); + if ("secret".equals(encryptionChild.getNodeName())) { + NamedNodeMap secretAttributes = encryptionChild.getAttributes(); + Node uuidAttribute = secretAttributes.getNamedItem("uuid"); + uuidAttribute.setTextContent(destSecretUUID); + } + } + } + } + + StringWriter diskSection = new StringWriter(); + Transformer xformer = TransformerFactory.newInstance().newTransformer(); + xformer.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "yes"); + xformer.transform(new DOMSource(diskNode), new StreamResult(diskSection)); + + return diskSection.toString(); + } + } + return null; + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java index 18aed95b0587..4707f18002b6 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java @@ -169,4 +169,8 @@ default LibvirtVMDef.DiskDef.BlockIOSize getSupportedPhysicalBlockSize() { default void customizeLibvirtDiskDef(LibvirtVMDef.DiskDef disk) { } + + default String getAuthSecretUUID() { + return null; + } } 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..c26ff0155692 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 @@ -606,6 +606,7 @@ public KVMStoragePool getStoragePool(String uuid, boolean refreshInfo) { pool.setAuthSecret(secretValue); } } + pool.setAuthSecretUUID(spd.getSecretUUID()); /** * On large (RBD) storage pools it can take up to a couple of minutes diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java index 7a70f2497cda..ed3c4d41db50 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java @@ -56,6 +56,7 @@ public class LibvirtStoragePool implements KVMStoragePool { protected StoragePool _pool; protected String authUsername; protected String authSecret; + protected String authSecretUUID; protected String sourceHost; protected int sourcePort; protected String sourceDir; @@ -309,6 +310,15 @@ public void setType(StoragePoolType type) { this.type = type; } + @Override + public String getAuthSecretUUID() { + return authSecretUUID; + } + + public void setAuthSecretUUID(String authSecretUUID) { + this.authSecretUUID = authSecretUUID; + } + public StoragePool getPool() { return this._pool; } diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 8674014addd7..3d84dfcbca85 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -3755,6 +3755,7 @@ public Volume migrateVolume(MigrateVolumeCmd cmd) { boolean liveMigrateVolume = false; boolean srcAndDestOnStorPool = false; + boolean srcAndDestOnRBD = false; Long instanceId = vol.getInstanceId(); Long srcClusterId = null; VMInstanceVO vm = null; @@ -3795,12 +3796,13 @@ public Volume migrateVolume(MigrateVolumeCmd cmd) { if (liveMigrateVolume && HypervisorType.KVM.equals(host.getHypervisorType()) && !srcStoragePoolVO.getPoolType().equals(Storage.StoragePoolType.PowerFlex)) { StoragePoolVO destinationStoragePoolVo = _storagePoolDao.findById(storagePoolId); - if (isSourceOrDestNotOnStorPool(srcStoragePoolVO, destinationStoragePoolVo)) { + if (isSourceOrDestNotOnStorPool(srcStoragePoolVO, destinationStoragePoolVo) && isSourceOrDestNotOnRBD(srcStoragePoolVO, destinationStoragePoolVo)) { throw new InvalidParameterValueException("KVM does not support volume live migration due to the limited possibility to refresh VM XML domain. " + "Therefore, to live migrate a volume between storage pools, one must migrate the VM to a different host as well to force the VM XML domain update. " + "Use 'migrateVirtualMachineWithVolumes' instead."); } srcAndDestOnStorPool = isSourceAndDestOnStorPool(srcStoragePoolVO, destinationStoragePoolVo); + srcAndDestOnRBD = isSourceAndDestOnRBD(srcStoragePoolVO, destinationStoragePoolVo); } } @@ -3814,7 +3816,7 @@ public Volume migrateVolume(MigrateVolumeCmd cmd) { } } - if ((vol.getPassphraseId() != null || vol.getKmsKeyId() != null) && !srcAndDestOnStorPool && !srcStoragePoolVO.getPoolType().equals(Storage.StoragePoolType.PowerFlex)) { + if ((vol.getPassphraseId() != null || vol.getKmsKeyId() != null) && !srcAndDestOnStorPool && !srcAndDestOnRBD && !srcStoragePoolVO.getPoolType().equals(Storage.StoragePoolType.PowerFlex)) { throw new InvalidParameterValueException("Migration of encrypted volumes is unsupported"); } @@ -3995,6 +3997,16 @@ private boolean isSourceAndDestOnStorPool(StoragePoolVO storagePoolVO, StoragePo && destinationStoragePoolVo.getPoolType() == Storage.StoragePoolType.StorPool; } + private boolean isSourceOrDestNotOnRBD(StoragePoolVO storagePoolVO, StoragePoolVO destinationStoragePoolVo) { + return storagePoolVO.getPoolType() != Storage.StoragePoolType.RBD + || destinationStoragePoolVo.getPoolType() != Storage.StoragePoolType.RBD; + } + + private boolean isSourceAndDestOnRBD(StoragePoolVO storagePoolVO, StoragePoolVO destinationStoragePoolVo) { + return storagePoolVO.getPoolType() == Storage.StoragePoolType.RBD + && destinationStoragePoolVo.getPoolType() == Storage.StoragePoolType.RBD; + } + /** * Retrieves the new disk offering UUID that might be sent to replace the current one in the volume being migrated. * If no disk offering UUID is provided we return null. Otherwise, we perform the following checks. From 187ee3c3d440d4cb344d4ca057d9ce064ecd9557 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 7 Oct 2026 17:22:49 +0200 Subject: [PATCH 3/4] smoke tests: exercise online volume migration for KVM on RBD test_13_migrate_volume_and_change_offering previously always stopped the VM before migrating an attached volume on KVM. Now that VolumeApiServiceImpl allows KVM to live-migrate a volume between two RBD pools (VolumeApiServiceImpl.isSourceAndDestOnRBD), keep the VM running and pass livemigrate=true for that RBD-to-RBD case, same as already done for VMware/XenServer. Any other KVM source/destination combination keeps the existing stop/migrate/start behavior. --- test/integration/smoke/test_volumes.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/test/integration/smoke/test_volumes.py b/test/integration/smoke/test_volumes.py index ad1c359c27b5..3cae523b6ca8 100644 --- a/test/integration/smoke/test_volumes.py +++ b/test/integration/smoke/test_volumes.py @@ -1024,9 +1024,6 @@ def test_13_migrate_volume_and_change_offering(self): self.virtual_machine.attach_volume(self.apiclient, volume=volume) - if self.virtual_machine.hypervisor == "KVM": - self.virtual_machine.stop(self.apiclient) - pools = StoragePool.listForMigration( self.apiclient, id=volume.id @@ -1039,6 +1036,15 @@ def test_13_migrate_volume_and_change_offering(self): else: raise self.skipTest("Not enough storage pools found, skipping test") + # KVM can live migrate a volume of a running VM only between RBD pools; + # any other KVM source/destination combination needs the VM stopped first. + current_vol = Volume.list(self.apiclient, id=volume.id)[0] + source_pool = list_storage_pools(self.apiclient, id=current_vol.storageid)[0] + kvm_rbd_live_migration = source_pool.type == 'RBD' and pool.type == 'RBD' + + if self.virtual_machine.hypervisor == "KVM" and not kvm_rbd_live_migration: + self.virtual_machine.stop(self.apiclient) + if hasattr(pool, 'tags'): StoragePool.update(self.apiclient, id=pool.id, tags="") @@ -1046,6 +1052,8 @@ def test_13_migrate_volume_and_change_offering(self): livemigrate = False if self.virtual_machine.hypervisor.lower() == "vmware" or self.virtual_machine.hypervisor.lower() == 'xenserver': livemigrate = True + elif self.virtual_machine.hypervisor == "KVM" and kvm_rbd_live_migration: + livemigrate = True Volume.migrate( self.apiclient, @@ -1054,7 +1062,7 @@ def test_13_migrate_volume_and_change_offering(self): newdiskofferingid=large_offering.id, livemigrate=livemigrate ) - if self.virtual_machine.hypervisor == "KVM": + if self.virtual_machine.hypervisor == "KVM" and not kvm_rbd_live_migration: self.virtual_machine.start(self.apiclient) migrated_vol = Volume.list( self.apiclient, From b62125c052d9564b19d68e43a80182cb894e9c57 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Thu, 15 Oct 2026 21:22:21 +0200 Subject: [PATCH 4/4] KVM: live migrate volumes between RBD, NFS, SharedMountPoint and local pools Migrate a volume of a running Instance between an RBD pool and an NFS, SharedMountPoint or host-local pool, and between those file based pools, with a libvirt block copy on the host the Instance runs on, so that neither the Instance nor its host change. Migrate encrypted volumes between RBD and NFS, online and offline. Management server: - VolumeApiServiceImpl lets a KVM live migration through for these pool pairs, where it asked for migrateVirtualMachineWithVolumes before. A local pool has to be the one of the host the Instance runs on. - AncientDataMotionStrategy keeps the format of the volume right after the move: RAW on RBD, QCOW2 on a file based pool, and luks2 or luks as the encryption format, as the agent encrypts an RBD volume with librbd and any other volume with qemu. KVM agent: - LibvirtMigrateVolumeCommandWrapper builds the disk XML of the destination for the type of the destination pool, an RBD image or a qcow2 file, and copies the whole disk, so a volume cloned from a template does not depend on a backing file of the source pool. SHALLOW is only used from RBD to RBD. The source volume is removed after the pivot and the destination pool refreshed. - An encrypted volume gets its destination created with the encryption header first, and an offline copy goes through qemu-img with both secrets (LibvirtStorageAdaptor, KVMStorageProcessor). - LibvirtStorageVolumeXMLParser read the volume format with its arguments swapped and found none, so a migrated qcow2 file named like an RBD image was taken for a raw one and the Instance failed to start again. It reads the format of the volume now. - The ceph.conf which the copy of an encrypted volume writes gets its mon_host from RbdUtils.getRbdMonitors, which gives each monitor its port, leaves alone one which has it, and brackets a bare IPv6 address. Tests: - Unit tests for the pool pairs which are accepted, the local pool of another host, and the dispatch of the agent to the block copy. - test_volume_migration_rbd_nfs.py migrates ROOT and DATA volumes online, offline and detached, encrypted or not, and stops and starts the Instance after every move to check that it is still reachable. - test_volumes.py: the data disk of the encrypted volume migration test is found by its size. --- .../motion/AncientDataMotionStrategy.java | 17 + .../LibvirtStorageVolumeXMLParser.java | 2 +- .../LibvirtMigrateVolumeCommandWrapper.java | 355 ++++++++--- .../kvm/storage/KVMStorageProcessor.java | 3 +- .../kvm/storage/LibvirtStorageAdaptor.java | 114 +++- ...ibvirtMigrateVolumeCommandWrapperTest.java | 152 ++++- .../cloud/storage/VolumeApiServiceImpl.java | 48 +- .../storage/VolumeApiServiceImplTest.java | 97 +++ .../smoke/test_volume_migration_rbd_nfs.py | 598 ++++++++++++++++++ test/integration/smoke/test_volumes.py | 138 ++++ 10 files changed, 1436 insertions(+), 88 deletions(-) create mode 100644 test/integration/smoke/test_volume_migration_rbd_nfs.py diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java index 05ab5374af5b..104effd2ac7a 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java @@ -677,6 +677,23 @@ protected Answer migrateVolumeToPool(DataObject srcData, DataObject destData) { volumeVo.setLastPoolId(oldPoolId); if (destPool.getPoolType() == StoragePoolType.CLVM) { volumeVo.setFormat(ImageFormat.RAW); + } else if (destPool.getPoolType() == StoragePoolType.RBD) { + // RBD only ever stores raw images; keep the volume's format metadata consistent + // regardless of what format it had on the source pool (e.g. QCOW2 on NFS). + volumeVo.setFormat(ImageFormat.RAW); + } else if (srcPool.getPoolType() == StoragePoolType.RBD && volumeVo.getFormat() == ImageFormat.RAW + && (destPool.getPoolType() == StoragePoolType.NetworkFilesystem + || destPool.getPoolType() == StoragePoolType.SharedMountPoint + || destPool.getPoolType() == StoragePoolType.Filesystem)) { + // The KVM agent (LibvirtMigrateVolumeCommandWrapper) writes a real QCOW2 file when the + // destination is file-based; without updating this, the volume keeps looking RAW to + // LibvirtComputingResource, which then (mis)treats a plain qcow2 file on NFS as a raw + // block device the next time the Instance is started. + volumeVo.setFormat(ImageFormat.QCOW2); + } + if (volumeVo.getEncryptFormat() != null && (srcPool.getPoolType() == StoragePoolType.RBD) != (destPool.getPoolType() == StoragePoolType.RBD)) { + // The KVM agent encrypts an RBD volume natively with librbd (LUKS2), and a volume on any other pool with qemu (LUKS) + volumeVo.setEncryptFormat(destPool.getPoolType() == StoragePoolType.RBD ? "luks2" : "luks"); } if (ClvmPoolManager.isClvmPoolType(destPool.getPoolType())) { clvmPoolManager.setClvmLockHostId(volume.getId(), ep.getId()); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStorageVolumeXMLParser.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStorageVolumeXMLParser.java index 00126a07cd3a..6be955a5931e 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStorageVolumeXMLParser.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStorageVolumeXMLParser.java @@ -48,7 +48,7 @@ public LibvirtStorageVolumeDef parseStorageVolumeXML(String volXML) { String VolName = getTagValue("name", rootElement); Element target = (Element)rootElement.getElementsByTagName("target").item(0); - String format = getAttrValue("type", "format", target); + String format = getAttrValue("format", "type", target); Long capacity = Long.parseLong(getTagValue("capacity", rootElement)); return new LibvirtStorageVolumeDef(VolName, capacity, LibvirtStorageVolumeDef.VolumeFormat.getFormat(format), null, null); } catch (ParserConfigurationException e) { diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateVolumeCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateVolumeCommandWrapper.java index de3a07010ffc..2d56b72effda 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateVolumeCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateVolumeCommandWrapper.java @@ -30,6 +30,7 @@ import com.cloud.hypervisor.kvm.storage.KVMPhysicalDisk; import com.cloud.hypervisor.kvm.storage.KVMStoragePool; import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager; +import com.cloud.hypervisor.kvm.storage.LibvirtStorageAdaptor; import com.cloud.resource.CommandWrapper; import com.cloud.resource.ResourceWrapper; import com.cloud.storage.Storage; @@ -46,6 +47,7 @@ import org.apache.cloudstack.storage.datastore.util.ScaleIOUtil; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; import org.apache.cloudstack.storage.to.VolumeObjectTO; +import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat; import org.apache.cloudstack.utils.security.ParserUtils; import org.apache.commons.lang3.ArrayUtils; import org.apache.commons.lang3.StringUtils; @@ -81,10 +83,17 @@ public Answer execute(final MigrateVolumeCommand command, final LibvirtComputing VolumeObjectTO srcVolumeObjectTO = (VolumeObjectTO)command.getSrcData(); PrimaryDataStoreTO srcPrimaryDataStore = (PrimaryDataStoreTO)srcVolumeObjectTO.getDataStore(); + VolumeObjectTO destVolumeObjectTO = (VolumeObjectTO)command.getDestData(); + PrimaryDataStoreTO destPrimaryDataStore = (PrimaryDataStoreTO)destVolumeObjectTO.getDataStore(); + MigrateVolumeAnswer answer; if (Objects.equals(Storage.StoragePoolType.PowerFlex, srcPrimaryDataStore.getPoolType())) { answer = migratePowerFlexVolume(command, libvirtComputingResource); - } else if (Objects.equals(Storage.StoragePoolType.RBD, srcPrimaryDataStore.getPoolType())) { + } else if (Objects.equals(Storage.StoragePoolType.RBD, srcPrimaryDataStore.getPoolType()) + || Objects.equals(Storage.StoragePoolType.RBD, destPrimaryDataStore.getPoolType()) + || (isFileBasedPool(srcPrimaryDataStore) && isFileBasedPool(destPrimaryDataStore))) { + // RBD, NFS, SharedMountPoint and local volumes are live migrated in place with a libvirt block copy, as both pools are + // reachable from the host the Instance is running on (the command is sent to that host). answer = migrateRBDVolume(command, libvirtComputingResource); } else { answer = migrateRegularVolume(command, libvirtComputingResource); @@ -93,6 +102,12 @@ public Answer execute(final MigrateVolumeCommand command, final LibvirtComputing return answer; } + private static boolean isFileBasedPool(PrimaryDataStoreTO dataStore) { + return Objects.equals(Storage.StoragePoolType.NetworkFilesystem, dataStore.getPoolType()) + || Objects.equals(Storage.StoragePoolType.SharedMountPoint, dataStore.getPoolType()) + || Objects.equals(Storage.StoragePoolType.Filesystem, dataStore.getPoolType()); + } + protected MigrateVolumeAnswer migratePowerFlexVolume(final MigrateVolumeCommand command, final LibvirtComputingResource libvirtComputingResource) { // Source Details @@ -387,11 +402,9 @@ protected MigrateVolumeAnswer migrateRBDVolume(final MigrateVolumeCommand comman VolumeObjectTO srcVolumeObjectTO = (VolumeObjectTO)command.getSrcData(); PrimaryDataStoreTO sourcePool = (PrimaryDataStoreTO)srcVolumeObjectTO.getDataStore(); - final String sourceFilePath = String.format("%s/%s", sourcePool.getPath(), srcVolumeObjectTO.getPath()); VolumeObjectTO destVolumeObjectTO = (VolumeObjectTO)command.getDestData(); PrimaryDataStoreTO destPool = (PrimaryDataStoreTO)destVolumeObjectTO.getDataStore(); - final String destFilePath = String.format("%s/%s", destPool.getPath(), destVolumeObjectTO.getPath()); String destDiskLabel = null; @@ -401,6 +414,8 @@ protected MigrateVolumeAnswer migrateRBDVolume(final MigrateVolumeCommand comman Domain dm = null; MigrateVolumeAnswer answer = null; + boolean destVolumeCreated = false; + KVMStoragePool destKvmPool = null; try { final LibvirtUtilitiesHelper libvirtUtilitiesHelper = libvirtComputingResource.getLibvirtUtilitiesHelper(); Connect conn = libvirtUtilitiesHelper.getConnection(); @@ -415,28 +430,62 @@ protected MigrateVolumeAnswer migrateRBDVolume(final MigrateVolumeCommand comman } KVMStoragePool pool = storagePoolMgr.getStoragePool(destPool.getPoolType(), destPool.getUuid()); + destKvmPool = pool; pool.connectPhysicalDisk(destVolumeObjectTO.getPath(), null); + KVMStoragePool srcKvmPool = storagePoolMgr.getStoragePool(sourcePool.getPoolType(), sourcePool.getUuid()); + final String sourceFilePath = generateVolumePathOnHost(srcKvmPool, sourcePool, srcVolumeObjectTO); + final String destFilePath = generateVolumePathOnHost(pool, destPool, destVolumeObjectTO); + String srcSecretUUID = null; String destSecretUUID = null; if (ArrayUtils.isNotEmpty(destVolumeObjectTO.getPassphrase())) { srcSecretUUID = libvirtComputingResource.createLibvirtVolumeSecret(conn, srcVolumeObjectTO.getPath(), srcVolumeObjectTO.getPassphrase()); destSecretUUID = libvirtComputingResource.createLibvirtVolumeSecret(conn, destVolumeObjectTO.getPath(), destVolumeObjectTO.getPassphrase()); + if (Objects.equals(srcSecretUUID, destSecretUUID)) { + // the secret is named after the volume path and is shared by both disks: keep it for the Instance + srcSecretUUID = null; + } } - String diskdef = generateDestinationDiskXMLForRBD(dm, sourceFilePath, destFilePath, destSecretUUID, pool); + // A block copy between two RBD pools only has to copy the active layer, as the destination image is + // already provisioned. When NFS is involved the backing chain of the source differs from what the + // destination can reference, so the whole chain is flattened into the destination instead. + boolean srcAndDestOnRbd = Objects.equals(Storage.StoragePoolType.RBD, sourcePool.getPoolType()) + && Objects.equals(Storage.StoragePoolType.RBD, destPool.getPoolType()); + + String diskdef = generateDestinationDiskXMLForRBD(dm, sourceFilePath, destFilePath, destSecretUUID, pool, destPool.getPoolType(), + srcAndDestOnRbd); + if (diskdef == null) { + return new MigrateVolumeAnswer(command, false, + String.format("Migrate volume failed as the disk with path %s was not found in the XML domain of vm %s", sourceFilePath, vmName), null); + } destDiskLabel = generateDestinationDiskLabel(diskdef); TypedUlongParameter parameter = new TypedUlongParameter("bandwidth", 0); TypedParameter[] parameters = new TypedParameter[1]; parameters[0] = parameter; + int blockCopyFlags = srcAndDestOnRbd ? Domain.BlockCopyFlags.SHALLOW : 0; + if (ArrayUtils.isNotEmpty(destVolumeObjectTO.getPassphrase())) { + // libvirt can not create (and format) an encrypted mirror itself, so the destination volume is + // created with its encryption header up front and the block copy writes into it. + KVMPhysicalDisk srcDisk = srcKvmPool.getPhysicalDisk(srcVolumeObjectTO.getPath()); + long guestSize = srcDisk.getVirtualSize(); + if (Objects.equals(Storage.StoragePoolType.RBD, sourcePool.getPoolType())) { + guestSize -= LibvirtStorageAdaptor.LUKS2_HEADER_RESERVE_BYTES; + } + createEncryptedDestinationVolume(pool, destPool, destVolumeObjectTO, guestSize); + destVolumeCreated = true; + blockCopyFlags |= Domain.BlockCopyFlags.REUSE_EXT; + } + cancelHook = new VolumeMigrationCancelHook(dm, destDiskLabel); libvirtComputingResource.addDisconnectHook(cancelHook); libvirtComputingResource.createOrUpdateLogFileForCommand(command, Command.State.PROCESSING_IN_BACKEND); - dm.blockCopy(destDiskLabel, diskdef, parameters, Domain.BlockCopyFlags.SHALLOW); + dm.blockCopy(destDiskLabel, diskdef, parameters, blockCopyFlags); logger.info(String.format("Block copy has started for the volume %s : %s to %s", destDiskLabel, sourceFilePath, destFilePath)); answer = checkBlockJobStatus(command, dm, destDiskLabel, sourceFilePath, destFilePath, libvirtComputingResource, conn, srcSecretUUID); @@ -449,6 +498,17 @@ protected MigrateVolumeAnswer migrateRBDVolume(final MigrateVolumeCommand comman } if (answer != null && answer.getResult()) { answer = new MigrateVolumeAnswer(command, true, null, destVolumeObjectTO.getPath()); + + // Libvirt only detects the volume's real format (e.g. qcow2) if the destination pool + // already knew about it as such. Since the block copy wrote the file directly (not + // through libvirt's own volume-creation calls), any pool other than RBD needs an + // explicit refresh here; otherwise libvirt keeps whatever it inferred when the disk + // was first attached to this domain (or falls back to a filename-suffix guess), which + // for a volume carried over from RBD would wrongly be treated as a raw block device the + // next time the Instance is (re)started. + if (!Objects.equals(Storage.StoragePoolType.RBD, destPool.getPoolType())) { + pool.refresh(); + } } return answer; } catch (Exception e) { @@ -462,15 +522,21 @@ protected MigrateVolumeAnswer migrateRBDVolume(final MigrateVolumeCommand comman } } libvirtComputingResource.createOrUpdateLogFileForCommand(command, Command.State.FAILED); - return new MigrateVolumeAnswer(command, false, msg, null); + answer = new MigrateVolumeAnswer(command, false, msg, null); + return answer; } finally { if (cancelHook != null) { libvirtComputingResource.removeDisconnectHook(cancelHook); } + if (destVolumeCreated && (answer == null || !answer.getResult())) { + deleteDestinationVolume(destKvmPool, destPool, destVolumeObjectTO.getPath()); + } if (answer != null && answer.getResult()) { // Remove old volume after successful migration KVMStoragePool libvirtSourcePool = storagePoolMgr.getStoragePool(sourcePool.getPoolType(), sourcePool.getUuid()); - libvirtSourcePool.deletePhysicalDisk(srcVolumeObjectTO.getPath(), Storage.ImageFormat.RAW); + Storage.ImageFormat srcFormat = Objects.equals(Storage.StoragePoolType.RBD, sourcePool.getPoolType()) + ? Storage.ImageFormat.RAW : Storage.ImageFormat.QCOW2; + libvirtSourcePool.deletePhysicalDisk(srcVolumeObjectTO.getPath(), srcFormat); } if (dm != null) { try { @@ -482,10 +548,68 @@ protected MigrateVolumeAnswer migrateRBDVolume(final MigrateVolumeCommand comman } } - protected String generateDestinationDiskXMLForRBD(Domain dm, String srcVolumePath, String diskFilePath, String destSecretUUID, KVMStoragePool pool) throws LibvirtException, ParserConfigurationException, IOException, TransformerException, SAXException { + + /** + * Creates the (empty) destination volume of a live migration of an encrypted volume, with its encryption + * header, so that the block copy can write into it. An RBD volume gets a native librbd LUKS2 header, a volume + * on a file based pool is a qemu LUKS encrypted QCOW2 file. + * + * @param guestSize size of the disk as seen by the Instance + */ + protected void createEncryptedDestinationVolume(KVMStoragePool pool, PrimaryDataStoreTO destPool, VolumeObjectTO destVolumeObjectTO, long guestSize) { + String path = destVolumeObjectTO.getPath(); + // a previous failed attempt may have left the volume behind + deleteDestinationVolume(pool, destPool, path); + if (Objects.equals(Storage.StoragePoolType.RBD, destPool.getPoolType())) { + // the LUKS2 header is carved out of the image, so it has to be added to what the Instance can use + pool.createPhysicalDisk(path, PhysicalDiskFormat.RAW, Storage.ProvisioningType.THIN, + guestSize + LibvirtStorageAdaptor.LUKS2_HEADER_RESERVE_BYTES, destVolumeObjectTO.getPassphrase()); + } else { + pool.createPhysicalDisk(path, PhysicalDiskFormat.QCOW2, Storage.ProvisioningType.THIN, guestSize, destVolumeObjectTO.getPassphrase()); + } + } + + protected void deleteDestinationVolume(KVMStoragePool pool, PrimaryDataStoreTO destPool, String path) { + if (pool == null) { + return; + } + try { + pool.deletePhysicalDisk(path, Objects.equals(Storage.StoragePoolType.RBD, destPool.getPoolType()) + ? Storage.ImageFormat.RAW : Storage.ImageFormat.QCOW2); + } catch (Exception e) { + logger.debug("Could not remove the destination volume {} (it may not exist): {}", path, e.getMessage()); + } + } + + /** + * Returns the path of the volume as it is referenced in the XML domain of the Instance on this host. RBD volumes + * are addressed as "/", while file based volumes (e.g. NFS) are addressed by their path on the + * local mount point of the pool. + */ + protected String generateVolumePathOnHost(KVMStoragePool kvmStoragePool, PrimaryDataStoreTO dataStore, VolumeObjectTO volumeObjectTO) { + if (Objects.equals(Storage.StoragePoolType.RBD, dataStore.getPoolType())) { + return String.format("%s/%s", dataStore.getPath(), volumeObjectTO.getPath()); + } + return String.format("%s/%s", kvmStoragePool.getLocalPath(), volumeObjectTO.getPath()); + } + + /** + * Generates the XML of the destination disk used by the libvirt block copy. The source disk is located in the XML + * domain of the Instance by its current path, and is then rewritten to point at the destination pool. Both an RBD + * (network) and an NFS (file) destination are supported, independently of the type of the source disk. + */ + /** + * Build the mirror disk XML handed to {@code blockCopy} from the disk the volume currently uses. + * + * @param shallowBlockCopy whether the copy will run with {@code VIR_DOMAIN_BLOCK_COPY_SHALLOW}, which decides + * whether the source's backing chain may be carried over to the mirror. + */ + protected String generateDestinationDiskXMLForRBD(Domain dm, String srcVolumePath, String diskFilePath, String destSecretUUID, KVMStoragePool pool, + Storage.StoragePoolType destPoolType, boolean shallowBlockCopy) + throws LibvirtException, ParserConfigurationException, IOException, TransformerException, SAXException { final String domXml = dm.getXMLDesc(0); - DocumentBuilderFactory dbFactory = DocumentBuilderFactory.newInstance(); + DocumentBuilderFactory dbFactory = ParserUtils.getSaferDocumentBuilderFactory(); DocumentBuilder dBuilder = dbFactory.newDocumentBuilder(); Document doc = dBuilder.parse(new ByteArrayInputStream(domXml.getBytes("UTF-8"))); doc.getDocumentElement().normalize(); @@ -494,83 +618,166 @@ protected String generateDestinationDiskXMLForRBD(Domain dm, String srcVolumePat for (int i = 0; i < disks.getLength(); i++) { Node diskNode = disks.item(i); - Node diskTypeAttribute = diskNode.getAttributes().getNamedItem("type"); - if (diskTypeAttribute == null || !diskTypeAttribute.getNodeValue().equalsIgnoreCase(LibvirtVMDef.DiskDef.DiskType.NETWORK.toString())) { + if (!isDiskNodeOfVolume(diskNode, srcVolumePath)) { continue; } - NodeList diskChildNodes = diskNode.getChildNodes(); - boolean found = false; - for (int j = 0; j < diskChildNodes.getLength(); j++) { - Node diskChildNode = diskChildNodes.item(j); + logger.debug(String.format("Found disk node with source volume path %s, generating destination disk XML for %s on %s", srcVolumePath, diskFilePath, + destPoolType)); + updateDiskNodeForDestinationPool(doc, diskNode, diskFilePath, destSecretUUID, pool, destPoolType, shallowBlockCopy); - if ("source".equals(diskChildNode.getNodeName())) { - NamedNodeMap diskNodeAttributes = diskChildNode.getAttributes(); - Node diskNodeAttribute = diskNodeAttributes.getNamedItem("protocol"); - if (diskNodeAttribute == null || !diskNodeAttribute.getNodeValue().equalsIgnoreCase(LibvirtVMDef.DiskDef.DiskProtocol.RBD.toString())) { - logger.debug("Skipped disk node with protocol: " + diskNodeAttribute); - continue; - } - diskNodeAttribute = diskNodeAttributes.getNamedItem("name"); - if (diskNodeAttribute == null || !diskNodeAttribute.getNodeValue().contains(srcVolumePath)) { - logger.debug("Skipped disk node with name: " + diskNodeAttribute); - continue; - } + StringWriter diskSection = new StringWriter(); + Transformer xformer = TransformerFactory.newInstance().newTransformer(); + xformer.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "yes"); + xformer.transform(new DOMSource(diskNode), new StreamResult(diskSection)); - logger.debug(String.format("Found disk node with source RBD volume path %s: %s", srcVolumePath, diskNodeAttribute)); - diskNode.removeChild(diskChildNode); - Element newChildSourceNode = doc.createElement("source"); - newChildSourceNode.setAttribute("protocol", LibvirtVMDef.DiskDef.DiskProtocol.RBD.toString().toLowerCase()); - newChildSourceNode.setAttribute("name", diskFilePath); - for (String sourceHost : pool.getSourceHost().split(",")) { - Element newChildDiskNodeForHost = doc.createElement("host"); - newChildDiskNodeForHost.setAttribute("name", sourceHost); - if (pool.getSourcePort() != 0) { - newChildDiskNodeForHost.setAttribute("port", String.valueOf(pool.getSourcePort())); - } - newChildSourceNode.appendChild(newChildDiskNodeForHost); - } - diskNode.appendChild(newChildSourceNode); + return diskSection.toString(); + } + return null; + } + + /** + * True if the disk node is the one backing the volume with the given path, be it an RBD (network) or a file disk. + */ + protected boolean isDiskNodeOfVolume(Node diskNode, String srcVolumePath) { + Node diskTypeAttribute = diskNode.getAttributes().getNamedItem("type"); + if (diskTypeAttribute == null) { + return false; + } + String diskType = diskTypeAttribute.getNodeValue(); + boolean networkDisk = LibvirtVMDef.DiskDef.DiskType.NETWORK.toString().equalsIgnoreCase(diskType); + boolean fileDisk = LibvirtVMDef.DiskDef.DiskType.FILE.toString().equalsIgnoreCase(diskType); + if (!networkDisk && !fileDisk) { + return false; + } + + Node sourceNode = getChildNode(diskNode, "source"); + if (sourceNode == null) { + return false; + } + NamedNodeMap sourceAttributes = sourceNode.getAttributes(); + + if (networkDisk) { + Node protocolAttribute = sourceAttributes.getNamedItem("protocol"); + if (protocolAttribute == null || !protocolAttribute.getNodeValue().equalsIgnoreCase(LibvirtVMDef.DiskDef.DiskProtocol.RBD.toString())) { + logger.debug("Skipped disk node with protocol: " + protocolAttribute); + return false; + } + Node nameAttribute = sourceAttributes.getNamedItem("name"); + return nameAttribute != null && nameAttribute.getNodeValue().contains(srcVolumePath); + } + + Node fileAttribute = sourceAttributes.getNamedItem("file"); + return fileAttribute != null && fileAttribute.getNodeValue().contains(srcVolumePath); + } + + /** + * Rewrites the disk node so that it refers to the volume on the destination pool. The disk type, the source, the + * driver type and the Ceph authentication are all adjusted to what the destination pool requires. + */ + protected void updateDiskNodeForDestinationPool(Document doc, Node diskNode, String diskFilePath, String destSecretUUID, KVMStoragePool pool, + Storage.StoragePoolType destPoolType, boolean shallowBlockCopy) { + boolean destinationOnRbd = Objects.equals(Storage.StoragePoolType.RBD, destPoolType); + + Element diskElement = (Element)diskNode; + diskElement.setAttribute("type", destinationOnRbd ? LibvirtVMDef.DiskDef.DiskType.NETWORK.toString().toLowerCase() + : LibvirtVMDef.DiskDef.DiskType.FILE.toString().toLowerCase()); + + Node sourceNode = getChildNode(diskNode, "source"); + if (sourceNode != null) { + diskNode.removeChild(sourceNode); + } + diskNode.appendChild(destinationOnRbd ? createRbdSourceNode(doc, diskFilePath, pool) : createFileSourceNode(doc, diskFilePath)); + + // A disk cloned from a template keeps that template as its backing file, and libvirt reports the whole + // chain in the live domain XML. Only a shallow copy may carry that chain into the mirror: a full copy + // flattens every layer into the destination, so a there describes a chain the destination + // does not have. libvirt refuses that combination outright with "backingStore of mirror without + // VIR_DOMAIN_BLOCK_COPY_SHALLOW doesn't make sense", which is why the element is dropped here. + if (!shallowBlockCopy) { + Node backingStoreNode = getChildNode(diskNode, "backingStore"); + if (backingStoreNode != null) { + diskNode.removeChild(backingStoreNode); + } + } + + Node driverNode = getChildNode(diskNode, "driver"); + if (driverNode instanceof Element) { + // RBD volumes are raw, whereas volumes on a file based pool are qcow2 + ((Element)driverNode).setAttribute("type", destinationOnRbd ? "raw" : "qcow2"); + } - found = true; - break; + // The Ceph authentication of the source pool does not apply to the destination pool, so it is always replaced + Node authNode = getChildNode(diskNode, "auth"); + if (authNode != null) { + diskNode.removeChild(authNode); + } + if (destinationOnRbd && StringUtils.isNotBlank(pool.getAuthUserName())) { + diskNode.appendChild(createAuthNode(doc, pool)); + } + + if (StringUtils.isNotBlank(destSecretUUID)) { + Node encryptionNode = getChildNode(diskNode, "encryption"); + if (encryptionNode instanceof Element) { + // librbd encrypts an RBD image natively (LUKS2), a file is encrypted by qemu (LUKS) + Element encryptionElement = (Element)encryptionNode; + if (destinationOnRbd) { + encryptionElement.setAttribute("format", "luks2"); + encryptionElement.setAttribute("engine", "librbd"); + } else { + encryptionElement.setAttribute("format", "luks"); + encryptionElement.removeAttribute("engine"); } } - if (found) { - for (int j = 0; j < diskChildNodes.getLength(); j++) { - Node diskChildNode = diskChildNodes.item(j); - if ("auth".equals(diskChildNode.getNodeName())) { - NamedNodeMap diskNodeAttributes = diskChildNode.getAttributes(); - Node diskNodeAttribute = diskNodeAttributes.getNamedItem("username"); - if (diskNodeAttribute != null) { - diskNodeAttribute.setNodeValue(pool.getAuthUserName()); - } - for (int m = 0; m < diskChildNode.getChildNodes().getLength(); m++) { - Node authChild = diskChildNode.getChildNodes().item(m); - if ("secret".equals(authChild.getNodeName())) { - NamedNodeMap secretAttributes = authChild.getAttributes(); - Node uuidAttribute = secretAttributes.getNamedItem("uuid"); - uuidAttribute.setTextContent(pool.getAuthSecretUUID()); - } - } - } else if ("encryption".equals(diskChildNode.getNodeName())) { - for (int n = 0; n < diskChildNode.getChildNodes().getLength(); n++) { - Node encryptionChild = diskChildNode.getChildNodes().item(n); - if ("secret".equals(encryptionChild.getNodeName())) { - NamedNodeMap secretAttributes = encryptionChild.getAttributes(); - Node uuidAttribute = secretAttributes.getNamedItem("uuid"); - uuidAttribute.setTextContent(destSecretUUID); - } - } + if (encryptionNode != null) { + Node secretNode = getChildNode(encryptionNode, "secret"); + if (secretNode != null) { + Node uuidAttribute = secretNode.getAttributes().getNamedItem("uuid"); + if (uuidAttribute != null) { + uuidAttribute.setTextContent(destSecretUUID); } } + } + } + } - StringWriter diskSection = new StringWriter(); - Transformer xformer = TransformerFactory.newInstance().newTransformer(); - xformer.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "yes"); - xformer.transform(new DOMSource(diskNode), new StreamResult(diskSection)); + protected Element createRbdSourceNode(Document doc, String diskFilePath, KVMStoragePool pool) { + Element sourceNode = doc.createElement("source"); + sourceNode.setAttribute("protocol", LibvirtVMDef.DiskDef.DiskProtocol.RBD.toString().toLowerCase()); + sourceNode.setAttribute("name", diskFilePath); + for (String sourceHost : pool.getSourceHost().split(",")) { + Element hostNode = doc.createElement("host"); + hostNode.setAttribute("name", sourceHost); + if (pool.getSourcePort() != 0) { + hostNode.setAttribute("port", String.valueOf(pool.getSourcePort())); + } + sourceNode.appendChild(hostNode); + } + return sourceNode; + } + + protected Element createFileSourceNode(Document doc, String diskFilePath) { + Element sourceNode = doc.createElement("source"); + sourceNode.setAttribute("file", diskFilePath); + return sourceNode; + } + + protected Element createAuthNode(Document doc, KVMStoragePool pool) { + Element authNode = doc.createElement("auth"); + authNode.setAttribute("username", pool.getAuthUserName()); + Element secretNode = doc.createElement("secret"); + secretNode.setAttribute("type", "ceph"); + secretNode.setAttribute("uuid", pool.getAuthSecretUUID()); + authNode.appendChild(secretNode); + return authNode; + } - return diskSection.toString(); + protected Node getChildNode(Node parent, String nodeName) { + NodeList childNodes = parent.getChildNodes(); + for (int i = 0; i < childNodes.getLength(); i++) { + Node childNode = childNodes.item(i); + if (nodeName.equals(childNode.getNodeName())) { + return childNode; } } return null; 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..8b36fb1dfab5 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 @@ -3488,7 +3488,8 @@ public Answer copyVolumeFromPrimaryToPrimary(CopyCommand cmd) { newVol.setPath(path); ImageFormat newVolumeFormat = getFormat(newVolume.getFormat()); newVol.setFormat(newVolumeFormat); - newVol.setEncryptFormat(destVol.getEncryptFormat()); + // an encrypted volume copied to another kind of pool is encrypted differently (librbd LUKS2 on RBD, qemu LUKS otherwise) + newVol.setEncryptFormat(newVolume.getQemuEncryptFormat() != null ? newVolume.getQemuEncryptFormat().toString() : destVol.getEncryptFormat()); return new CopyCmdAnswer(newVol); } catch (final CloudRuntimeException e) { logger.debug("Failed to copyVolumeFromPrimaryToPrimary: ", e); 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 c26ff0155692..99a68c26728b 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 @@ -21,8 +21,12 @@ import java.io.File; import java.io.IOException; import java.nio.charset.Charset; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermissions; import java.util.ArrayList; import java.util.Arrays; +import java.util.EnumMap; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -45,6 +49,7 @@ import org.apache.cloudstack.utils.qemu.QemuObject; import org.apache.commons.codec.binary.Base64; import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.lang3.ArrayUtils; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -77,6 +82,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; @@ -98,10 +104,16 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { private int rbdOrder = 0; /* Order 0 means 4MB blocks (the default) */ /* Space reserved at the front of an encrypted RBD image for the LUKS2 header/keyslots so the usable (decrypted) size still matches the requested volume size. */ - private static final long LUKS2_HEADER_RESERVE_BYTES = 16L << 20; // 16 MiB + public static final long LUKS2_HEADER_RESERVE_BYTES = 16L << 20; // 16 MiB /* libvirt's VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS, not exposed as a constant by libvirt-java */ private static final int VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS = 2; + /* pool types an encrypted volume can be copied between, see copyEncryptedPhysicalDisk */ + private static final Set ENCRYPTED_COPY_POOL_TYPES = Set.of(StoragePoolType.RBD, StoragePoolType.NetworkFilesystem, + StoragePoolType.Filesystem, StoragePoolType.SharedMountPoint); + private static final String SRC_LUKS_SECRET_ID = "srcsec0"; + private static final String DEST_LUKS_SECRET_ID = "destsec0"; + private static final Set QEMU_IMG_MANAGED_POOL_TYPES = Set.of(StoragePoolType.NetworkFilesystem, StoragePoolType.Filesystem, StoragePoolType.SharedMountPoint); public LibvirtStorageAdaptor(StorageLayer storage) { @@ -1643,6 +1655,13 @@ public KVMPhysicalDisk copyPhysicalDisk(KVMPhysicalDisk disk, String name, KVMSt */ KVMStoragePool srcPool = disk.getPool(); + if (ArrayUtils.isNotEmpty(srcPassphrase) && ENCRYPTED_COPY_POOL_TYPES.contains(srcPool.getType()) + && ENCRYPTED_COPY_POOL_TYPES.contains(destPool.getType())) { + // the volume keeps its passphrase when it is copied to another pool + return copyEncryptedPhysicalDisk(disk, name, destPool, timeout, srcPassphrase, + ArrayUtils.isNotEmpty(dstPassphrase) ? dstPassphrase : srcPassphrase); + } + /* Linstor images are always stored as RAW, but Linstor uses qcow2 in DB, to support snapshots(backuped) as qcow2 files. */ PhysicalDiskFormat sourceFormat = srcPool.getType() != StoragePoolType.Linstor ? @@ -1797,6 +1816,99 @@ to support snapshots(backuped) as qcow2 files. */ return newDisk; } + /** + * Copies an encrypted volume to another pool. The data is read through the encryption of the source and written + * through the encryption of the destination, which is created up front: an RBD image gets a native librbd LUKS2 + * header, a volume on a file based pool is a qemu LUKS encrypted QCOW2 file. Copying the encrypted data as is + * would not do: the destination may need another kind of encryption than the source, and an encrypted RBD clone + * reads the data it inherits from its (plaintext) parent unencrypted. Any backing chain of the source (an RBD + * parent or a QCOW2 backing file) is flattened into the destination. + */ + private KVMPhysicalDisk copyEncryptedPhysicalDisk(KVMPhysicalDisk disk, String name, KVMStoragePool destPool, int timeout, + byte[] srcPassphrase, byte[] dstPassphrase) { + KVMStoragePool srcPool = disk.getPool(); + boolean srcOnRbd = srcPool.getType() == StoragePoolType.RBD; + boolean destOnRbd = destPool.getType() == StoragePoolType.RBD; + // the size of the disk as seen by the Instance: the LUKS2 header of an RBD image is carved out of the image + long guestSize = srcOnRbd ? disk.getVirtualSize() - LUKS2_HEADER_RESERVE_BYTES : disk.getVirtualSize(); + logger.debug("Copying encrypted volume {} from pool {} to {} in pool {}, with a size of {}", disk.getPath(), srcPool.getUuid(), + name, destPool.getUuid(), toHumanReadableSize(guestSize)); + + KVMPhysicalDisk newDisk = destPool.createPhysicalDisk(name, destOnRbd ? PhysicalDiskFormat.RAW : PhysicalDiskFormat.QCOW2, + Storage.ProvisioningType.THIN, destOnRbd ? guestSize + LUKS2_HEADER_RESERVE_BYTES : guestSize, dstPassphrase); + + List cephAuthFiles = new ArrayList<>(); + try (KeyFile srcKeyFile = new KeyFile(srcPassphrase); KeyFile destKeyFile = new KeyFile(dstPassphrase)) { + QemuImageOptions srcImageOpts = encryptedImageOptions(srcPool, disk.getName(), disk.getPath(), + disk.getFormat() == PhysicalDiskFormat.QCOW2 ? PhysicalDiskFormat.QCOW2 : PhysicalDiskFormat.LUKS, SRC_LUKS_SECRET_ID, cephAuthFiles); + srcImageOpts.setImageOptsFlag(true); + QemuImageOptions destImageOpts = encryptedImageOptions(destPool, name, newDisk.getPath(), PhysicalDiskFormat.QCOW2, + DEST_LUKS_SECRET_ID, cephAuthFiles); + + QemuImgFile srcFile = new QemuImgFile(disk.getPath(), srcOnRbd ? PhysicalDiskFormat.RAW : disk.getFormat()); + List secrets = List.of(createSecretObject(SRC_LUKS_SECRET_ID, srcKeyFile), createSecretObject(DEST_LUKS_SECRET_ID, destKeyFile)); + new QemuImg(timeout).convert(srcFile, null, null, null, secrets, srcImageOpts, destImageOpts, null, false, false, false, false, null, null); + } catch (IOException | QemuImgException | LibvirtException e) { + logger.error("Failed to copy encrypted volume {} to {} in pool {}: {}", disk.getPath(), name, destPool.getUuid(), e.getMessage()); + try { + destPool.deletePhysicalDisk(name, destOnRbd ? Storage.ImageFormat.RAW : Storage.ImageFormat.QCOW2); + } catch (Exception ex) { + logger.warn("Failed to remove {} from pool {} after the failed copy: {}", name, destPool.getUuid(), ex.getMessage()); + } + throw new CloudRuntimeException(String.format("Failed to copy encrypted volume %s to %s", disk.getPath(), name), e); + } finally { + for (Path file : cephAuthFiles) { + try { + Files.deleteIfExists(file); + } catch (IOException e) { + logger.warn("Failed to remove temporary file {}: {}", file, e.getMessage()); + } + } + } + + newDisk.setFormat(destOnRbd ? PhysicalDiskFormat.RAW : PhysicalDiskFormat.QCOW2); + newDisk.setVirtualSize(guestSize); + newDisk.setQemuEncryptFormat(destOnRbd ? QemuObject.EncryptFormat.LUKS2 : QemuObject.EncryptFormat.LUKS); + return newDisk; + } + + /** + * qemu image options addressing an encrypted volume: an RBD image (with librbd LUKS2 encryption, authenticated through + * a temporary ceph.conf and keyring that are added to {@code cephAuthFiles}) or a file of the given format. + */ + private QemuImageOptions encryptedImageOptions(KVMStoragePool pool, String rbdImage, String filePath, PhysicalDiskFormat fileFormat, + String secretId, List cephAuthFiles) throws IOException { + if (pool.getType() != StoragePoolType.RBD) { + return new QemuImageOptions(fileFormat, filePath, secretId); + } + Path keyring = Files.createTempFile("cs-ceph-", ".keyring", PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rw-------"))); + cephAuthFiles.add(keyring); + Files.writeString(keyring, "[client." + pool.getAuthUserName() + "]\n\tkey = " + pool.getAuthSecret() + "\n"); + Path conf = Files.createTempFile("cs-ceph-", ".conf", PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rw-------"))); + cephAuthFiles.add(conf); + String monHost = RbdUtils.getRbdMonitors(pool.getSourceHost(), pool.getSourcePort()); + Files.writeString(conf, "[global]\nmon_host = " + monHost + "\nkeyring = " + keyring + "\n"); + + Map options = new HashMap<>(); + options.put("driver", "rbd"); + options.put("pool", pool.getSourceDir()); + options.put("image", rbdImage); + options.put("conf", conf.toString()); + if (pool.getAuthUserName() != null) { + options.put("user", pool.getAuthUserName()); + } + options.put("encrypt.format", CryptSetup.LuksType.LUKS2.toString()); + options.put("encrypt.key-secret", secretId); + return new QemuImageOptions(options); + } + + private QemuObject createSecretObject(String secretId, KeyFile keyFile) { + Map params = new EnumMap<>(QemuObject.ObjectParameter.class); + params.put(QemuObject.ObjectParameter.ID, secretId); + params.put(QemuObject.ObjectParameter.FILE, keyFile.toString()); + return new QemuObject(QemuObject.ObjectType.SECRET, params); + } + @Override public boolean refresh(KVMStoragePool pool) { LibvirtStoragePool libvirtPool = (LibvirtStoragePool)pool; diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateVolumeCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateVolumeCommandWrapperTest.java index e3d35dfb4d1f..68f020213a77 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateVolumeCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateVolumeCommandWrapperTest.java @@ -228,6 +228,11 @@ public void testPowerFlexMigrateVolumeMethod() { Mockito.doReturn(srcPrimaryDataStore).when(srcVolumeObjectTO).getDataStore(); Mockito.doReturn(Storage.StoragePoolType.PowerFlex).when(srcPrimaryDataStore).getPoolType(); + VolumeObjectTO destVolumeObjectTO = Mockito.mock(VolumeObjectTO.class); + Mockito.doReturn(destVolumeObjectTO).when(command).getDestData(); + PrimaryDataStoreTO destPrimaryDataStore = Mockito.mock(PrimaryDataStoreTO.class); + Mockito.doReturn(destPrimaryDataStore).when(destVolumeObjectTO).getDataStore(); + MigrateVolumeAnswer powerFlexAnswer = Mockito.mock(MigrateVolumeAnswer.class); MigrateVolumeAnswer regularVolumeAnswer = Mockito.mock(MigrateVolumeAnswer.class); @@ -246,7 +251,13 @@ public void testRegularMigrateVolumeMethod() { PrimaryDataStoreTO srcPrimaryDataStore = Mockito.mock(PrimaryDataStoreTO.class); Mockito.doReturn(srcPrimaryDataStore).when(srcVolumeObjectTO).getDataStore(); - Mockito.doReturn(Storage.StoragePoolType.NetworkFilesystem).when(srcPrimaryDataStore).getPoolType(); + Mockito.doReturn(Storage.StoragePoolType.Iscsi).when(srcPrimaryDataStore).getPoolType(); + + VolumeObjectTO destVolumeObjectTO = Mockito.mock(VolumeObjectTO.class); + Mockito.doReturn(destVolumeObjectTO).when(command).getDestData(); + PrimaryDataStoreTO destPrimaryDataStore = Mockito.mock(PrimaryDataStoreTO.class); + Mockito.doReturn(destPrimaryDataStore).when(destVolumeObjectTO).getDataStore(); + Mockito.doReturn(Storage.StoragePoolType.Iscsi).when(destPrimaryDataStore).getPoolType(); MigrateVolumeAnswer powerFlexAnswer = Mockito.mock(MigrateVolumeAnswer.class); MigrateVolumeAnswer regularVolumeAnswer = Mockito.mock(MigrateVolumeAnswer.class); @@ -259,6 +270,54 @@ public void testRegularMigrateVolumeMethod() { Assert.assertFalse(answer.getResult()); } + private Answer executeMigrationBetween(Storage.StoragePoolType srcType, Storage.StoragePoolType destType, boolean blockCopy) { + VolumeObjectTO srcVolumeObjectTO = Mockito.mock(VolumeObjectTO.class); + Mockito.doReturn(srcVolumeObjectTO).when(command).getSrcData(); + PrimaryDataStoreTO srcPrimaryDataStore = Mockito.mock(PrimaryDataStoreTO.class); + Mockito.doReturn(srcPrimaryDataStore).when(srcVolumeObjectTO).getDataStore(); + Mockito.doReturn(srcType).when(srcPrimaryDataStore).getPoolType(); + + VolumeObjectTO destVolumeObjectTO = Mockito.mock(VolumeObjectTO.class); + Mockito.doReturn(destVolumeObjectTO).when(command).getDestData(); + PrimaryDataStoreTO destPrimaryDataStore = Mockito.mock(PrimaryDataStoreTO.class); + Mockito.doReturn(destPrimaryDataStore).when(destVolumeObjectTO).getDataStore(); + Mockito.doReturn(destType).when(destPrimaryDataStore).getPoolType(); + + MigrateVolumeAnswer blockCopyAnswer = Mockito.mock(MigrateVolumeAnswer.class); + MigrateVolumeAnswer regularVolumeAnswer = Mockito.mock(MigrateVolumeAnswer.class); + Mockito.doReturn(blockCopy).when(blockCopyAnswer).getResult(); + Mockito.doReturn(!blockCopy).when(regularVolumeAnswer).getResult(); + Mockito.doReturn(blockCopyAnswer).when(libvirtMigrateVolumeCommandWrapper).migrateRBDVolume(command, libvirtComputingResource); + Mockito.doReturn(regularVolumeAnswer).when(libvirtMigrateVolumeCommandWrapper).migrateRegularVolume(command, libvirtComputingResource); + + return libvirtMigrateVolumeCommandWrapper.execute(command, libvirtComputingResource); + } + + @Test + public void testBlockCopyMigrateVolumeMethodBetweenRbdAndFileBasedPools() { + for (Storage.StoragePoolType fileBased : new Storage.StoragePoolType[] {Storage.StoragePoolType.NetworkFilesystem, + Storage.StoragePoolType.SharedMountPoint, Storage.StoragePoolType.Filesystem}) { + Assert.assertTrue("RBD -> " + fileBased, executeMigrationBetween(Storage.StoragePoolType.RBD, fileBased, true).getResult()); + Assert.assertTrue(fileBased + " -> RBD", executeMigrationBetween(fileBased, Storage.StoragePoolType.RBD, true).getResult()); + } + } + + @Test + public void testBlockCopyMigrateVolumeMethodBetweenFileBasedPools() { + Storage.StoragePoolType[] fileBasedTypes = {Storage.StoragePoolType.NetworkFilesystem, Storage.StoragePoolType.SharedMountPoint, + Storage.StoragePoolType.Filesystem}; + for (Storage.StoragePoolType src : fileBasedTypes) { + for (Storage.StoragePoolType dest : fileBasedTypes) { + Assert.assertTrue(src + " -> " + dest, executeMigrationBetween(src, dest, true).getResult()); + } + } + } + + @Test + public void testRegularMigrateVolumeMethodFromFileBasedPoolToOtherPool() { + Assert.assertTrue(executeMigrationBetween(Storage.StoragePoolType.NetworkFilesystem, Storage.StoragePoolType.Iscsi, false).getResult()); + } + @Test public void testMigratePowerFlexVolume() throws LibvirtException, ParserConfigurationException, IOException, TransformerException, SAXException { VolumeObjectTO srcVolumeObjectTO = Mockito.mock(VolumeObjectTO.class); @@ -349,6 +408,97 @@ public void testMigratePowerFlexVolumeFailure() throws LibvirtException, ParserC Assert.assertFalse(migrateVolumeAnswer.getResult()); } + /** + * Domain XML of a running Instance whose ROOT disk was thin-cloned from a template on an NFS pool: the disk + * carries the template as its backing file, and libvirt reports the whole chain in the live XML. + */ + private static final String DOMAIN_XML_WITH_BACKING_CHAIN = + "\n" + + " i-2-428-VM\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + ""; + + private Domain domainWithBackingChain() throws LibvirtException { + Domain dm = Mockito.mock(Domain.class); + Mockito.when(dm.getXMLDesc(0)).thenReturn(DOMAIN_XML_WITH_BACKING_CHAIN); + return dm; + } + + private KVMStoragePool rbdDestinationPool() { + KVMStoragePool pool = Mockito.mock(KVMStoragePool.class); + Mockito.when(pool.getSourceHost()).thenReturn("10.0.33.44"); + Mockito.when(pool.getSourcePort()).thenReturn(0); + Mockito.when(pool.getAuthUserName()).thenReturn("admin"); + Mockito.when(pool.getAuthSecretUUID()).thenReturn("7684f6e5-fd91-3435-b04e-59ae0efb3914"); + return pool; + } + + /** + * A ROOT volume cloned from a template keeps that template as its backing file. Migrating it from NFS to Ceph is + * a full (non-shallow) block copy that flattens the chain into the destination, so the mirror XML must not carry + * a <backingStore> over - libvirt rejects it with "backingStore of mirror without + * VIR_DOMAIN_BLOCK_COPY_SHALLOW doesn't make sense" and the migration never starts. + */ + @Test + public void testGenerateDestinationDiskXMLForRBDDropsBackingStoreWhenCopyIsNotShallow() throws Exception { + String diskXml = libvirtMigrateVolumeCommandWrapper.generateDestinationDiskXMLForRBD( + domainWithBackingChain(), "5d468388-e71e-4169-805e-0398dbe7274a", + "cloudstack1/aa11bb22-cc33-dd44-ee55-ff6677889900", null, rbdDestinationPool(), + Storage.StoragePoolType.RBD, false); + + Assert.assertNotNull(diskXml); + Assert.assertFalse("the inherited backing chain must not reach a non-shallow mirror", + diskXml.contains("backingStore")); + Assert.assertTrue(diskXml.contains("protocol=\"rbd\"")); + Assert.assertTrue(diskXml.contains("cloudstack1/aa11bb22-cc33-dd44-ee55-ff6677889900")); + Assert.assertTrue("an RBD destination is raw", diskXml.contains("type=\"raw\"")); + } + + /** + * An RBD to RBD migration copies only the active layer, so the backing chain stays meaningful and is preserved. + */ + @Test + public void testGenerateDestinationDiskXMLForRBDKeepsBackingStoreWhenCopyIsShallow() throws Exception { + String diskXml = libvirtMigrateVolumeCommandWrapper.generateDestinationDiskXMLForRBD( + domainWithBackingChain(), "5d468388-e71e-4169-805e-0398dbe7274a", + "cloudstack1/aa11bb22-cc33-dd44-ee55-ff6677889900", null, rbdDestinationPool(), + Storage.StoragePoolType.RBD, true); + + Assert.assertNotNull(diskXml); + Assert.assertTrue("a shallow copy keeps the backing chain", diskXml.contains("backingStore")); + } + + /** + * The same applies migrating the other way: a Ceph to NFS copy is not shallow either, so the destination file + * disk must be free of the source's backing chain. + */ + @Test + public void testGenerateDestinationDiskXMLForFileDestinationDropsBackingStore() throws Exception { + KVMStoragePool pool = Mockito.mock(KVMStoragePool.class); + + String diskXml = libvirtMigrateVolumeCommandWrapper.generateDestinationDiskXMLForRBD( + domainWithBackingChain(), "5d468388-e71e-4169-805e-0398dbe7274a", + "/mnt/nfs-pool/aa11bb22-cc33-dd44-ee55-ff6677889900", null, pool, + Storage.StoragePoolType.NetworkFilesystem, false); + + Assert.assertNotNull(diskXml); + Assert.assertFalse(diskXml.contains("backingStore")); + Assert.assertTrue(diskXml.contains("/mnt/nfs-pool/aa11bb22-cc33-dd44-ee55-ff6677889900")); + Assert.assertTrue("a file destination is qcow2", diskXml.contains("type=\"qcow2\"")); + } + @Test public void testCheckBlockJobStatus() throws LibvirtException { Connect conn = Mockito.mock(Connect.class); diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 3d84dfcbca85..8da48b2a92bc 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -186,6 +186,7 @@ import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.SnapshotDao; +import com.cloud.storage.dao.StoragePoolHostDao; import com.cloud.storage.dao.StoragePoolTagsDao; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VolumeDao; @@ -305,6 +306,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic @Inject private PrimaryDataStoreDao _storagePoolDao; @Inject + private StoragePoolHostDao _storagePoolHostDao; + @Inject private ImageStoreDao imageStoreDao; @Inject private DiskOfferingDao _diskOfferingDao; @@ -431,6 +434,9 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic private static final Set STATES_VOLUME_CANNOT_BE_DESTROYED = new HashSet<>(Arrays.asList(Volume.State.Destroy, Volume.State.Expunging, Volume.State.Expunged, Volume.State.Allocated)); private static final long GiB_TO_BYTES = 1024 * 1024 * 1024; + // pool types whose volumes are files, on KVM + private static final Set KVM_FILE_BASED_POOL_TYPES = Set.of(Storage.StoragePoolType.NetworkFilesystem, Storage.StoragePoolType.SharedMountPoint, + Storage.StoragePoolType.Filesystem); private static final String CUSTOM_DISK_OFFERING_UNIQUE_NAME = "Cloud.com-Custom"; private static final List validAttachStates = Arrays.asList(Volume.State.Allocated, Volume.State.Ready, Volume.State.Uploaded); @@ -3755,7 +3761,6 @@ public Volume migrateVolume(MigrateVolumeCmd cmd) { boolean liveMigrateVolume = false; boolean srcAndDestOnStorPool = false; - boolean srcAndDestOnRBD = false; Long instanceId = vol.getInstanceId(); Long srcClusterId = null; VMInstanceVO vm = null; @@ -3796,13 +3801,13 @@ public Volume migrateVolume(MigrateVolumeCmd cmd) { if (liveMigrateVolume && HypervisorType.KVM.equals(host.getHypervisorType()) && !srcStoragePoolVO.getPoolType().equals(Storage.StoragePoolType.PowerFlex)) { StoragePoolVO destinationStoragePoolVo = _storagePoolDao.findById(storagePoolId); - if (isSourceOrDestNotOnStorPool(srcStoragePoolVO, destinationStoragePoolVo) && isSourceOrDestNotOnRBD(srcStoragePoolVO, destinationStoragePoolVo)) { + if (isSourceOrDestNotOnStorPool(srcStoragePoolVO, destinationStoragePoolVo) && !isVolumeMigrationBetweenRbdAndFileBased(srcStoragePoolVO.getPoolType(), destinationStoragePoolVo.getPoolType())) { throw new InvalidParameterValueException("KVM does not support volume live migration due to the limited possibility to refresh VM XML domain. " + "Therefore, to live migrate a volume between storage pools, one must migrate the VM to a different host as well to force the VM XML domain update. " + "Use 'migrateVirtualMachineWithVolumes' instead."); } + checkLocalPoolsOnHostOfRunningVm(vm, srcStoragePoolVO, destinationStoragePoolVo); srcAndDestOnStorPool = isSourceAndDestOnStorPool(srcStoragePoolVO, destinationStoragePoolVo); - srcAndDestOnRBD = isSourceAndDestOnRBD(srcStoragePoolVO, destinationStoragePoolVo); } } @@ -3816,7 +3821,11 @@ public Volume migrateVolume(MigrateVolumeCmd cmd) { } } - if ((vol.getPassphraseId() != null || vol.getKmsKeyId() != null) && !srcAndDestOnStorPool && !srcAndDestOnRBD && !srcStoragePoolVO.getPoolType().equals(Storage.StoragePoolType.PowerFlex)) { + // An encrypted (KVM) volume can be migrated between RBD and NFS pools, whether it is in use by a running Instance or not + StoragePoolVO destinationStoragePoolVO = _storagePoolDao.findById(storagePoolId); + boolean srcAndDestOnRbdOrNfs = destinationStoragePoolVO != null && isVolumeMigrationBetweenRbdAndFileBased(srcStoragePoolVO.getPoolType(), destinationStoragePoolVO.getPoolType()); + + if ((vol.getPassphraseId() != null || vol.getKmsKeyId() != null) && !srcAndDestOnStorPool && !srcAndDestOnRbdOrNfs && !srcStoragePoolVO.getPoolType().equals(Storage.StoragePoolType.PowerFlex)) { throw new InvalidParameterValueException("Migration of encrypted volumes is unsupported"); } @@ -3997,14 +4006,33 @@ private boolean isSourceAndDestOnStorPool(StoragePoolVO storagePoolVO, StoragePo && destinationStoragePoolVo.getPoolType() == Storage.StoragePoolType.StorPool; } - private boolean isSourceOrDestNotOnRBD(StoragePoolVO storagePoolVO, StoragePoolVO destinationStoragePoolVo) { - return storagePoolVO.getPoolType() != Storage.StoragePoolType.RBD - || destinationStoragePoolVo.getPoolType() != Storage.StoragePoolType.RBD; + /** + * A volume of a running Instance is live migrated in place on the host the Instance runs on, so a local (host scoped) + * pool involved in the migration must be the local pool of that very host. + */ + protected void checkLocalPoolsOnHostOfRunningVm(VMInstanceVO vm, StoragePoolVO srcPool, StoragePoolVO destPool) { + for (StoragePoolVO pool : new StoragePoolVO[] {srcPool, destPool}) { + if (pool == null || !Storage.StoragePoolType.Filesystem.equals(pool.getPoolType())) { + continue; + } + if (vm.getHostId() == null || _storagePoolHostDao.findByPoolHost(pool.getId(), vm.getHostId()) == null) { + throw new InvalidParameterValueException(String.format("The volume of a running Instance can only be migrated to or from the local storage pool %s " + + "of the host the Instance is running on", pool.getName())); + } + } + } + + /** + * True if KVM migrates (also encrypted) volumes between the pools, online or offline: between RBD pools and file based pools + * (NFS, SharedMountPoint or local), in any combination. For a running Instance a local pool has to be the one of the host + * the Instance runs on. + */ + protected static boolean isVolumeMigrationBetweenRbdAndFileBased(Storage.StoragePoolType srcType, Storage.StoragePoolType destType) { + return isRbdOrFileBased(srcType) && isRbdOrFileBased(destType); } - private boolean isSourceAndDestOnRBD(StoragePoolVO storagePoolVO, StoragePoolVO destinationStoragePoolVo) { - return storagePoolVO.getPoolType() == Storage.StoragePoolType.RBD - && destinationStoragePoolVo.getPoolType() == Storage.StoragePoolType.RBD; + private static boolean isRbdOrFileBased(Storage.StoragePoolType poolType) { + return Storage.StoragePoolType.RBD.equals(poolType) || KVM_FILE_BASED_POOL_TYPES.contains(poolType); } /** diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index 12258e2cc160..b80726837214 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -124,6 +124,7 @@ import com.cloud.storage.clvm.ClvmPoolManager; import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.SnapshotDao; +import com.cloud.storage.dao.StoragePoolHostDao; import com.cloud.storage.dao.StoragePoolTagsDao; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VolumeDao; @@ -142,6 +143,7 @@ import com.cloud.vm.DiskProfile; import com.cloud.vm.UserVmManager; import com.cloud.vm.UserVmVO; +import com.cloud.vm.VMInstanceVO; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachine.State; import com.cloud.vm.VirtualMachineManager; @@ -160,6 +162,8 @@ public class VolumeApiServiceImplTest { @InjectMocks private VolumeApiServiceImpl volumeApiServiceImpl = new VolumeApiServiceImpl(); @Mock + private StoragePoolHostDao storagePoolHostDaoMock; + @Mock private SnapshotManager snapshotManagerMock; @Mock private VolumeDataStoreDao volumeDataStoreDaoMock; @@ -2896,4 +2900,97 @@ public void testResizeVolumeInternal_VMware_VMRunning_ShouldThrowStateGuardError e.getMessage() != null && e.getMessage().contains("VM should be in")); } } + + private static final List FILE_BASED = Arrays.asList(Storage.StoragePoolType.NetworkFilesystem, Storage.StoragePoolType.SharedMountPoint, Storage.StoragePoolType.Filesystem); + + @Test + public void isVolumeMigrationBetweenRbdAndFileBasedBetweenRbdAndFileBasedPools() { + Assert.assertTrue(VolumeApiServiceImpl.isVolumeMigrationBetweenRbdAndFileBased(Storage.StoragePoolType.RBD, Storage.StoragePoolType.RBD)); + for (Storage.StoragePoolType fileBased : FILE_BASED) { + Assert.assertTrue("RBD -> " + fileBased, VolumeApiServiceImpl.isVolumeMigrationBetweenRbdAndFileBased(Storage.StoragePoolType.RBD, fileBased)); + Assert.assertTrue(fileBased + " -> RBD", VolumeApiServiceImpl.isVolumeMigrationBetweenRbdAndFileBased(fileBased, Storage.StoragePoolType.RBD)); + } + } + + @Test + public void isVolumeMigrationBetweenRbdAndFileBasedBetweenFileBasedPools() { + for (Storage.StoragePoolType src : FILE_BASED) { + for (Storage.StoragePoolType dest : FILE_BASED) { + Assert.assertTrue(src + " -> " + dest, VolumeApiServiceImpl.isVolumeMigrationBetweenRbdAndFileBased(src, dest)); + } + } + } + + @Test + public void isVolumeMigrationBetweenRbdAndFileBasedNotWithOtherPoolTypes() { + List supported = new ArrayList<>(FILE_BASED); + supported.add(Storage.StoragePoolType.RBD); + for (Storage.StoragePoolType other : Arrays.asList(Storage.StoragePoolType.StorPool, Storage.StoragePoolType.PowerFlex, + Storage.StoragePoolType.Iscsi, Storage.StoragePoolType.CLVM, Storage.StoragePoolType.Linstor)) { + Assert.assertFalse(other + " -> " + other, VolumeApiServiceImpl.isVolumeMigrationBetweenRbdAndFileBased(other, other)); + for (Storage.StoragePoolType type : supported) { + Assert.assertFalse(type + " -> " + other, VolumeApiServiceImpl.isVolumeMigrationBetweenRbdAndFileBased(type, other)); + Assert.assertFalse(other + " -> " + type, VolumeApiServiceImpl.isVolumeMigrationBetweenRbdAndFileBased(other, type)); + } + } + } + + private StoragePoolVO poolOfType(long id, Storage.StoragePoolType type) { + StoragePoolVO pool = mock(StoragePoolVO.class); + lenient().when(pool.getId()).thenReturn(id); + lenient().when(pool.getName()).thenReturn("pool-" + id); + lenient().when(pool.getPoolType()).thenReturn(type); + return pool; + } + + @Test + public void checkLocalPoolsOnHostOfRunningVmAcceptsLocalPoolOfTheHostOfTheInstance() { + VMInstanceVO vm = mock(VMInstanceVO.class); + when(vm.getHostId()).thenReturn(10L); + StoragePoolVO rbd = poolOfType(1L, Storage.StoragePoolType.RBD); + StoragePoolVO local = poolOfType(2L, Storage.StoragePoolType.Filesystem); + when(storagePoolHostDaoMock.findByPoolHost(2L, 10L)).thenReturn(mock(StoragePoolHostVO.class)); + + volumeApiServiceImpl.checkLocalPoolsOnHostOfRunningVm(vm, rbd, local); + volumeApiServiceImpl.checkLocalPoolsOnHostOfRunningVm(vm, local, rbd); + } + + @Test(expected = InvalidParameterValueException.class) + public void checkLocalPoolsOnHostOfRunningVmRejectsLocalPoolOfAnotherHost() { + VMInstanceVO vm = mock(VMInstanceVO.class); + when(vm.getHostId()).thenReturn(10L); + StoragePoolVO rbd = poolOfType(1L, Storage.StoragePoolType.RBD); + StoragePoolVO local = poolOfType(2L, Storage.StoragePoolType.Filesystem); + when(storagePoolHostDaoMock.findByPoolHost(2L, 10L)).thenReturn(null); + + volumeApiServiceImpl.checkLocalPoolsOnHostOfRunningVm(vm, rbd, local); + } + + @Test(expected = InvalidParameterValueException.class) + public void checkLocalPoolsOnHostOfRunningVmRejectsLocalSourcePoolOfAnotherHost() { + VMInstanceVO vm = mock(VMInstanceVO.class); + when(vm.getHostId()).thenReturn(10L); + StoragePoolVO local = poolOfType(2L, Storage.StoragePoolType.Filesystem); + StoragePoolVO rbd = poolOfType(1L, Storage.StoragePoolType.RBD); + when(storagePoolHostDaoMock.findByPoolHost(2L, 10L)).thenReturn(null); + + volumeApiServiceImpl.checkLocalPoolsOnHostOfRunningVm(vm, local, rbd); + } + + @Test(expected = InvalidParameterValueException.class) + public void checkLocalPoolsOnHostOfRunningVmRejectsLocalPoolWithoutHostOfTheInstance() { + VMInstanceVO vm = mock(VMInstanceVO.class); + when(vm.getHostId()).thenReturn(null); + + volumeApiServiceImpl.checkLocalPoolsOnHostOfRunningVm(vm, poolOfType(1L, Storage.StoragePoolType.RBD), poolOfType(2L, Storage.StoragePoolType.Filesystem)); + } + + @Test + public void checkLocalPoolsOnHostOfRunningVmIgnoresPoolsWhichAreNotLocal() { + VMInstanceVO vm = mock(VMInstanceVO.class); + + volumeApiServiceImpl.checkLocalPoolsOnHostOfRunningVm(vm, poolOfType(1L, Storage.StoragePoolType.RBD), poolOfType(2L, Storage.StoragePoolType.NetworkFilesystem)); + volumeApiServiceImpl.checkLocalPoolsOnHostOfRunningVm(vm, poolOfType(3L, Storage.StoragePoolType.SharedMountPoint), poolOfType(1L, Storage.StoragePoolType.RBD)); + Mockito.verifyNoInteractions(storagePoolHostDaoMock); + } } diff --git a/test/integration/smoke/test_volume_migration_rbd_nfs.py b/test/integration/smoke/test_volume_migration_rbd_nfs.py new file mode 100644 index 000000000000..90cf407a9198 --- /dev/null +++ b/test/integration/smoke/test_volume_migration_rbd_nfs.py @@ -0,0 +1,598 @@ +# 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. +""" BVT tests for volume migration between RBD (Ceph) and NFS primary +storage pools on KVM. + +Every combination below is covered, for plain volumes +(TestVolumeMigrationRbdNfs) as well as for encrypted volumes +(TestEncryptedVolumeMigrationRbdNfs): + +* source and destination pool: RBD -> NFS, NFS -> RBD, NFS -> NFS and + RBD -> RBD +* online (the Instance keeps running, migrateVolume with livemigrate) and + offline (the Instance is stopped, or the volume is not attached) +* ROOT and DATA volumes +* volumes with and without a backing chain: every test deploys its own + Instance on its source pool, so the first migration moves a ROOT volume + cloned from the template (a qcow2 backing file on NFS, an RBD clone on + Ceph); the DATA volume, and the ROOT volume once it was migrated, have no + chain. + +Besides the volume being on the expected pool, the data of each volume is +verified on the host after every migration, as the Instance sees it (that is, +decrypted): a pattern written to the DATA volume, and a checksum of the boot +area of the ROOT volume. For encrypted volumes the encryption the destination +pool requires is verified too: native librbd LUKS2 on RBD, qemu LUKS on NFS. + +Requirements: KVM, an RBD pool and the "nfs" primary storage of the test +data. The NFS -> NFS migrations also need the "nfs2" primary storage and the +RBD -> RBD migrations a second RBD pool; those are skipped otherwise. The +encrypted volume tests need a host with RBD volume encryption support. +""" + +import base64 +import hashlib +import re +import xml.etree.ElementTree as ET + +from marvin.cloudstackAPI import (deleteStoragePool, + detachVolume, + enableStorageMaintenance) +from marvin.cloudstackTestCase import cloudstackTestCase +from marvin.lib.base import (Account, + DiskOffering, + Host, + ServiceOffering, + StoragePool, + Template, + VirtualMachine, + Volume) +from marvin.lib.common import (get_domain, + get_pod, + get_zone, + list_clusters, + list_hosts, + list_storage_pools) +from marvin.lib.utils import random_gen +from marvin.sshClient import SshClient + +from nose.plugins.attrib import attr + +_multiprocess_shared_ = False + +# Runs on the host of an Instance to access one of its disks as the Instance sees it, i.e. decrypted: +# disk-io.sh write|read "" +# A write goes through the HMP qemu-io command of the disk device. A read goes through a read-only NBD +# export of the node of the disk, as the output of HMP qemu-io ends up (buffered) in the log of qemu. +DISK_IO_SCRIPT = r''' +dom=$1; dev=$2; mode=$3; cmd=$4 +qmp() { virsh qemu-monitor-command "$dom" "$1"; } +read qdev node <<<$(qmp '{"execute":"query-block"}' | python3 -c ' +import json, sys +for b in json.load(sys.stdin)["return"]: + q = b.get("qdev", "") + if sys.argv[1] in (q, b.get("device", "")) or q.startswith("/machine/peripheral/%s/" % sys.argv[1]): + print(q or b["device"], b["inserted"]["node-name"]) + break' "$dev") +[ -z "$node" ] && { echo "no block node found for disk $dev"; exit 1; } +if [ "$mode" = write ]; then + virsh qemu-monitor-command "$dom" --hmp "qemu-io -d $qdev \"$cmd\"" + exit $? +fi +sock=$(ls -d /var/lib/libvirt/qemu/domain-*-$dom | head -1)/volume-migration-test.sock +qmp "{\"execute\":\"nbd-server-start\",\"arguments\":{\"addr\":{\"type\":\"unix\",\"data\":{\"path\":\"$sock\"}}}}" >/dev/null +qmp "{\"execute\":\"block-export-add\",\"arguments\":{\"type\":\"nbd\",\"id\":\"volmig\",\"node-name\":\"$node\",\"writable\":false}}" >/dev/null +qemu-io -r -f raw "nbd+unix:///$node?socket=$sock" -c "$cmd" +rc=$? +qmp '{"execute":"block-export-del","arguments":{"id":"volmig"}}' >/dev/null +sleep 1 +qmp '{"execute":"nbd-server-stop"}' >/dev/null +exit $rc +''' +DISK_IO_SCRIPT_PATH = "/tmp/cs-volume-migration-test-disk-io.sh" + +DATA_PATTERN = "0xa5" +DATA_PATTERN_OFFSET = "1M" +DATA_PATTERN_LENGTH = "4M" +DATA_PATTERN_BYTES = 4 * 1024 * 1024 +# The start of a ROOT volume that the guest does not write to: the MBR and the gap in front of the first +# partition (which traditionally starts at sector 63) that holds the boot loader. Further on, the guest +# updates its file systems (e.g. the mount time in the superblock of /boot) while it runs. +ROOT_CHECKSUM_LENGTH = "31k" + + +class VolumeMigrationRbdNfsBase(cloudstackTestCase): + """Set up and helpers of the volume migration tests between RBD and NFS pools on KVM""" + + # whether the ROOT and DATA volumes of the Instances are encrypted + encrypted = False + + @classmethod + def setUpClass(cls): + testClient = super(VolumeMigrationRbdNfsBase, cls).getClsTestClient() + cls.apiclient = testClient.getApiClient() + cls.dbclient = testClient.getDbConnection() + cls.services = testClient.getParsedTestDataConfig() + cls._cleanup = [] + cls.created_pools = [] + cls.saved_pool_tags = {} + cls.unsupported_reason = None + + cls.domain = get_domain(cls.apiclient) + cls.zone = get_zone(cls.apiclient, testClient.getZoneForTests()) + cls.pod = get_pod(cls.apiclient, cls.zone.id) + cls.services['mode'] = cls.zone.networktype + cls.hypervisor = testClient.getHypervisorInfo() + + if cls.hypervisor.lower() != 'kvm': + cls.unsupported_reason = "requires KVM" + return + + rbd_pools = [pool for pool in list_storage_pools(cls.apiclient, zoneid=cls.zone.id) or [] + if pool.type == 'RBD' and pool.state == 'Up'] + if not rbd_pools: + cls.unsupported_reason = "requires an RBD storage pool" + return + cls.rbd_pool = rbd_pools[0] + cls.rbd_pool2 = rbd_pools[1] if len(rbd_pools) > 1 else None + + cls.cluster = None + for cluster in list_clusters(cls.apiclient, zoneid=cls.zone.id) or []: + if list_hosts(cls.apiclient, clusterid=cluster.id): + cls.cluster = cluster + break + if cls.cluster is None: + cls.unsupported_reason = "requires a cluster with hosts" + return + + cls.host_config = cls.config.__dict__["zones"][0].__dict__["pods"][0].__dict__["clusters"][0].__dict__["hosts"][0].__dict__ + + if cls.encrypted and not cls.host_with_rbd_encryption_support_exists(): + cls.unsupported_reason = "requires a host with RBD volume encryption support" + return + + cls.template = Template.register( + cls.apiclient, + cls.services["test_templates_cloud_init"][cls.hypervisor.lower()], + zoneid=cls.zone.id, + hypervisor=cls.hypervisor, + ) + cls.template.download(cls.apiclient) + + cls.services["domainid"] = cls.domain.id + cls.services["zoneid"] = cls.zone.id + cls.services["template"] = cls.template.id + + cls.account = Account.create( + cls.apiclient, + cls.services["account"], + domainid=cls.domain.id + ) + cls._cleanup.append(cls.account) + + # The offerings carry a storage tag of their own, which is put on the pool an Instance has to be + # deployed on, and on all the pools the volumes are migrated between. + cls.pool_tag = "volmig-" + random_gen(size=6).lower() + encryption_args = {"encryptroot": True} if cls.encrypted else {} + cls.service_offering = ServiceOffering.create( + cls.apiclient, + cls.services["service_offerings"]["tiny"], + tags=cls.pool_tag, + **encryption_args + ) + cls._cleanup.append(cls.service_offering) + + encryption_args = {"encrypt": True} if cls.encrypted else {} + cls.disk_offering = DiskOffering.create( + cls.apiclient, + cls.services["disk_offering"], + tags=cls.pool_tag, + **encryption_args + ) + cls._cleanup.append(cls.disk_offering) + + cls.nfs_pool = cls.create_nfs_pool(cls.services["nfs"]) + cls.nfs_pool2 = cls.create_nfs_pool(cls.services["nfs2"]) if "nfs2" in cls.services else None + + cls.pools = [pool for pool in (cls.rbd_pool, cls.rbd_pool2, cls.nfs_pool, cls.nfs_pool2) if pool is not None] + for pool in cls.pools: + cls.saved_pool_tags[pool.id] = getattr(pool, 'tags', "") or "" + + @classmethod + def create_nfs_pool(cls, services): + pool = StoragePool.create( + cls.apiclient, + services, + clusterid=cls.cluster.id, + zoneid=cls.zone.id, + podid=cls.pod.id + ) + cls.created_pools.append(pool) + return pool + + @classmethod + def host_with_rbd_encryption_support_exists(cls): + # librbd encryption support is only tracked in host_details, it is not part of the API response + hosts = Host.list(cls.apiclient, zoneid=cls.zone.id, type='Routing', hypervisor='KVM', state='Up') or [] + for host in hosts: + supported = cls.dbclient.execute( + "select hd.value from host_details hd join host h on h.id=hd.host_id " + "where h.uuid='%s' and hd.name='host.volume.encryption.rbd'" % host.id + ) + if supported and supported[0][0].lower() == 'true': + return True + return False + + @classmethod + def tag_pools(cls, pools): + """Put the storage tag of the offerings on the given pools (on top of their own tags) and remove it + from the other ones""" + for pool in cls.pools: + tags = [tag for tag in cls.saved_pool_tags[pool.id].split(",") if tag] + if pool in pools: + tags.append(cls.pool_tag) + StoragePool.update(cls.apiclient, id=pool.id, tags=",".join(tags)) + + @classmethod + def tearDownClass(cls): + try: + super(VolumeMigrationRbdNfsBase, cls).tearDownClass() + finally: + for pool in getattr(cls, 'pools', []): + try: + StoragePool.update(cls.apiclient, id=pool.id, tags=cls.saved_pool_tags[pool.id]) + except Exception as e: + cls.debug("Warning: failed to restore the tags of storage pool %s: %s" % (pool.id, e)) + for pool in cls.created_pools: + cls.delete_storage_pool(pool) + + @classmethod + def delete_storage_pool(cls, pool): + """Put the pool in maintenance and force delete it, so a volume left + behind by a failed test does not leak the pool into the next run.""" + try: + cmd = enableStorageMaintenance.enableStorageMaintenanceCmd() + cmd.id = pool.id + cls.apiclient.enableStorageMaintenance(cmd) + cmd = deleteStoragePool.deleteStoragePoolCmd() + cmd.id = pool.id + cmd.forced = True + cls.apiclient.deleteStoragePool(cmd) + except Exception as e: + cls.debug("Warning: failed to delete storage pool %s: %s" % (pool.id, e)) + + def setUp(self): + self.apiclient = self.testClient.getApiClient() + self.cleanup = [] + if self.unsupported_reason: + self.skipTest("Skipping test: %s" % self.unsupported_reason) + + def tearDown(self): + super(VolumeMigrationRbdNfsBase, self).tearDown() + + def require_pools(self, *pools): + if None in pools: + self.skipTest("Skipping test: requires a second NFS (\"nfs2\" in the test data) or RBD storage pool") + + # Instances and volumes + + def deploy_instance(self, pool): + """Deploy an Instance with a DATA volume, both volumes allocated on the given pool. The ROOT volume + is cloned from the template, so it has a backing chain (a qcow2 backing file or an RBD parent).""" + self.tag_pools([pool]) + try: + instance = VirtualMachine.create( + self.apiclient, + self.services, + accountid=self.account.name, + domainid=self.account.domainid, + serviceofferingid=self.service_offering.id, + mode=self.services["mode"] + ) + self.cleanup.append(instance) + data_volume = Volume.create( + self.apiclient, + self.services, + account=self.account.name, + domainid=self.account.domainid, + diskofferingid=self.disk_offering.id + ) + self.cleanup.append(data_volume) + instance.attach_volume(self.apiclient, data_volume) + finally: + self.tag_pools(self.pools) + + root_volume = Volume.list(self.apiclient, virtualmachineid=instance.id, type='ROOT', listall=True)[0] + for volume in (root_volume, data_volume): + self.assert_volume_on_pool(volume, pool) + # Destroying the Instance does not remove its DATA volume, which is to be detached first + self.cleanup.remove(data_volume) + self.cleanup.insert(0, _DetachedVolume(self.apiclient, data_volume)) + return instance, root_volume, data_volume + + def migrate_online(self, instance, volumes, pool): + for volume in volumes: + Volume.migrate(self.apiclient, volumeid=volume.id, storageid=pool.id, livemigrate=True) + self.assert_instance_state(instance, 'Running') + + def migrate_offline(self, instance, volumes, pool): + instance.stop(self.apiclient) + for volume in volumes: + Volume.migrate(self.apiclient, volumeid=volume.id, storageid=pool.id, livemigrate=False) + instance.start(self.apiclient) + + def migrate_detached(self, instance, volume, pool): + # the Instance is stopped, as not every disk bus (e.g. IDE) supports hot unplugging + instance.stop(self.apiclient) + instance.detach_volume(self.apiclient, volume) + Volume.migrate(self.apiclient, volumeid=volume.id, storageid=pool.id, livemigrate=False) + instance.attach_volume(self.apiclient, volume) + instance.start(self.apiclient) + + def run_migrations(self, source_pool, destination_pools, online): + """Deploy an Instance on source_pool and migrate its ROOT and DATA volumes along destination_pools, + verifying the volumes after every migration, and again after restarting the Instance.""" + instance, root_volume, data_volume = self.deploy_instance(source_pool) + disks = InstanceDisks(self, instance, root_volume, data_volume) + disks.mark() + + for pool in destination_pools: + if online: + self.migrate_online(instance, (root_volume, data_volume), pool) + else: + self.migrate_offline(instance, (root_volume, data_volume), pool) + self.verify_volumes(instance, disks, (root_volume, data_volume), pool) + # the volumes also have to be usable when the Instance is started again + instance.stop(self.apiclient) + instance.start(self.apiclient) + self.verify_volumes(instance, disks, (root_volume, data_volume), pool) + + # Checks + + def verify_volumes(self, instance, disks, volumes, pool): + for volume in volumes: + self.assert_volume_on_pool(volume, pool) + self.assert_instance_state(instance, 'Running') + disks.verify(volumes, pool) + + def assert_volume_on_pool(self, volume, pool): + migrated_volume = Volume.list(self.apiclient, id=volume.id, listall=True)[0] + self.assertEqual( + migrated_volume.storageid, + pool.id, + "Volume %s is on storage pool %s instead of %s" % (volume.id, migrated_volume.storage, pool.name) + ) + self.assertEqual(migrated_volume.state, 'Ready', "Volume %s is not Ready after the migration" % volume.id) + + def assert_instance_state(self, instance, state): + self.assertEqual( + VirtualMachine.list(self.apiclient, id=instance.id)[0].state, + state, + "Instance %s is not %s" % (instance.id, state) + ) + + # The tests + + def online_migration_from_rbd(self): + """Online migrate a ROOT volume with a backing chain (an RBD clone of the template) and a DATA volume + from RBD to NFS, then (without chain) NFS -> NFS, NFS -> RBD and RBD -> RBD""" + self.run_migrations(self.rbd_pool, [pool for pool in (self.nfs_pool, self.nfs_pool2, self.rbd_pool, self.rbd_pool2) + if pool is not None], online=True) + + def online_migration_from_nfs(self, destination_pool): + """Online migrate a ROOT volume with a backing chain (a qcow2 file backed by the template) and a DATA + volume from NFS to destination_pool""" + self.run_migrations(self.nfs_pool, [destination_pool], online=True) + + def offline_migration_from_rbd(self): + """Offline migrate a ROOT volume with a backing chain (an RBD clone of the template) and a DATA volume + from RBD to NFS, then (without chain) NFS -> NFS, NFS -> RBD and RBD -> RBD""" + self.run_migrations(self.rbd_pool, [pool for pool in (self.nfs_pool, self.nfs_pool2, self.rbd_pool, self.rbd_pool2) + if pool is not None], online=False) + + def offline_migration_from_nfs(self, destination_pool): + """Offline migrate a ROOT volume with a backing chain (a qcow2 file backed by the template) and a DATA + volume from NFS to destination_pool""" + self.run_migrations(self.nfs_pool, [destination_pool], online=False) + + def detached_volume_migration(self): + """Migrate a DATA volume that is not attached to an Instance between all the pools""" + instance, root_volume, data_volume = self.deploy_instance(self.rbd_pool) + disks = InstanceDisks(self, instance, root_volume, data_volume) + disks.mark() + for pool in [pool for pool in (self.nfs_pool, self.nfs_pool2, self.rbd_pool2, self.rbd_pool) if pool is not None]: + self.migrate_detached(instance, data_volume, pool) + self.verify_volumes(instance, disks, (data_volume,), pool) + + +class _DetachedVolume: + """Cleanup wrapper that detaches a DATA volume (if it still is attached) before deleting it""" + + def __init__(self, apiclient, volume): + self.apiclient = apiclient + self.volume = volume + + def delete(self, apiclient): + volumes = Volume.list(apiclient, id=self.volume.id, listall=True) + if not volumes: + return + if getattr(volumes[0], 'virtualmachineid', None): + cmd = detachVolume.detachVolumeCmd() + cmd.id = self.volume.id + apiclient.detachVolume(cmd) + self.volume.delete(apiclient) + + +class InstanceDisks: + """Verifies the data of the ROOT and DATA volumes of an Instance on its host, as the Instance sees them + (decrypted): a pattern written to the DATA volume, and a checksum of the boot area of the ROOT volume. For + encrypted volumes, the encryption of the disks in the XML domain of the Instance is verified as well.""" + + def __init__(self, test, instance, root_volume, data_volume): + self.test = test + self.instance = instance + self.root_volume = root_volume + self.data_volume = data_volume + self.root_checksum = None + + def _host_ssh(self): + vm = VirtualMachine.list(self.test.apiclient, id=self.instance.id)[0] + host = Host.list(self.test.apiclient, id=vm.hostid)[0] + ssh = SshClient(host=host.ipaddress, port=22, user=self.test.host_config['username'], + passwd=self.test.host_config['password']) + script = base64.b64encode(DISK_IO_SCRIPT.encode()).decode() + ssh.execute("echo %s | base64 -d > %s" % (script, DISK_IO_SCRIPT_PATH)) + return ssh, vm.instancename + + def _disk(self, ssh, instance_name, volume): + """The disk element of the volume in the XML domain; its serial is the start of the volume uuid""" + domain = ET.fromstring("\n".join(ssh.execute("virsh dumpxml %s" % instance_name))) + serial = volume.id.replace("-", "")[:20] + for disk in domain.findall("./devices/disk"): + if disk.findtext("serial") == serial: + return disk + self.test.fail("Disk of volume %s not found in the XML domain of %s" % (volume.id, instance_name)) + + def _disk_io(self, volume, mode, command): + ssh, instance_name = self._host_ssh() + alias = self._disk(ssh, instance_name, volume).find("alias").get("name") + return "\n".join(ssh.execute("bash %s %s %s %s '%s' 2>&1" % (DISK_IO_SCRIPT_PATH, instance_name, alias, mode, command))) + + def _root_checksum(self): + dump = [line for line in self._disk_io(self.root_volume, "read", "read -v 0 %s" % ROOT_CHECKSUM_LENGTH).splitlines() + if re.match(r"^[0-9a-f]{8}:", line)] + return hashlib.md5("\n".join(dump).encode()).hexdigest() if dump else None + + def _data_pattern_present(self): + out = self._disk_io(self.data_volume, "read", "read -P %s %s %s" % (DATA_PATTERN, DATA_PATTERN_OFFSET, DATA_PATTERN_LENGTH)) + return "verification failed" not in out and ("read %d/%d bytes" % (DATA_PATTERN_BYTES, DATA_PATTERN_BYTES)) in out + + def mark(self): + self._disk_io(self.data_volume, "write", "write -P %s %s %s" % (DATA_PATTERN, DATA_PATTERN_OFFSET, DATA_PATTERN_LENGTH)) + self.test.assertTrue(self._data_pattern_present(), "Failed to write the test pattern to volume %s" % self.data_volume.id) + self.root_checksum = self._root_checksum() + self.test.assertIsNotNone(self.root_checksum, "Failed to read volume %s" % self.root_volume.id) + + def verify(self, volumes, pool): + for volume in volumes: + if volume.id == self.data_volume.id: + self.test.assertTrue(self._data_pattern_present(), + "The data of volume %s is not intact after the migration to %s" % (volume.id, pool.name)) + else: + self.test.assertEqual(self._root_checksum(), self.root_checksum, + "The data of volume %s is not intact after the migration to %s" % (volume.id, pool.name)) + if self.test.encrypted: + self._verify_encryption(volume, pool) + + def _verify_encryption(self, volume, pool): + """An RBD volume is encrypted natively by librbd (LUKS2), a volume on NFS by qemu (LUKS)""" + ssh, instance_name = self._host_ssh() + disk = self._disk(ssh, instance_name, volume) + encryption = disk.find("encryption") + if encryption is None: + encryption = disk.find("source/encryption") + self.test.assertIsNotNone(encryption, "Volume %s is not encrypted after the migration to %s" % (volume.id, pool.name)) + if pool.type == 'RBD': + self.test.assertEqual(encryption.get("format"), "luks2") + self.test.assertEqual(encryption.get("engine"), "librbd") + else: + self.test.assertEqual(encryption.get("format"), "luks") + self.test.assertNotEqual(encryption.get("engine"), "librbd") + + +class TestVolumeMigrationRbdNfs(VolumeMigrationRbdNfsBase): + """Migration of plain volumes between RBD and NFS pools on KVM""" + + @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true") + def test_01_online_migration_from_rbd(self): + """Online migration: RBD (ROOT with chain) -> NFS -> NFS -> RBD -> RBD""" + self.online_migration_from_rbd() + + @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true") + def test_02_online_migration_from_nfs_to_rbd(self): + """Online migration: NFS (ROOT with chain) -> RBD""" + self.online_migration_from_nfs(self.rbd_pool) + + @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true") + def test_03_online_migration_from_nfs_to_nfs(self): + """Online migration: NFS (ROOT with chain) -> NFS""" + self.require_pools(self.nfs_pool2) + self.online_migration_from_nfs(self.nfs_pool2) + + @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true") + def test_04_offline_migration_from_rbd(self): + """Offline migration: RBD (ROOT with chain) -> NFS -> NFS -> RBD -> RBD""" + self.offline_migration_from_rbd() + + @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true") + def test_05_offline_migration_from_nfs_to_rbd(self): + """Offline migration: NFS (ROOT with chain) -> RBD""" + self.offline_migration_from_nfs(self.rbd_pool) + + @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true") + def test_06_offline_migration_from_nfs_to_nfs(self): + """Offline migration: NFS (ROOT with chain) -> NFS""" + self.require_pools(self.nfs_pool2) + self.offline_migration_from_nfs(self.nfs_pool2) + + @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true") + def test_07_detached_volume_migration(self): + """Migration of a detached DATA volume: RBD -> NFS -> NFS -> RBD -> RBD""" + self.detached_volume_migration() + + +class TestEncryptedVolumeMigrationRbdNfs(VolumeMigrationRbdNfsBase): + """Migration of encrypted volumes between RBD and NFS pools on KVM""" + + encrypted = True + + @attr(tags=["advanced", "advancedns", "smoke", "diskencrypt"], required_hardware="true") + def test_01_encrypted_online_migration_from_rbd(self): + """Online migration of encrypted volumes: RBD (ROOT with chain) -> NFS -> NFS -> RBD -> RBD""" + self.online_migration_from_rbd() + + @attr(tags=["advanced", "advancedns", "smoke", "diskencrypt"], required_hardware="true") + def test_02_encrypted_online_migration_from_nfs_to_rbd(self): + """Online migration of encrypted volumes: NFS (ROOT with chain) -> RBD""" + self.online_migration_from_nfs(self.rbd_pool) + + @attr(tags=["advanced", "advancedns", "smoke", "diskencrypt"], required_hardware="true") + def test_03_encrypted_online_migration_from_nfs_to_nfs(self): + """Online migration of encrypted volumes: NFS (ROOT with chain) -> NFS""" + self.require_pools(self.nfs_pool2) + self.online_migration_from_nfs(self.nfs_pool2) + + @attr(tags=["advanced", "advancedns", "smoke", "diskencrypt"], required_hardware="true") + def test_04_encrypted_offline_migration_from_rbd(self): + """Offline migration of encrypted volumes: RBD (ROOT with chain) -> NFS -> NFS -> RBD -> RBD""" + self.offline_migration_from_rbd() + + @attr(tags=["advanced", "advancedns", "smoke", "diskencrypt"], required_hardware="true") + def test_05_encrypted_offline_migration_from_nfs_to_rbd(self): + """Offline migration of encrypted volumes: NFS (ROOT with chain) -> RBD""" + self.offline_migration_from_nfs(self.rbd_pool) + + @attr(tags=["advanced", "advancedns", "smoke", "diskencrypt"], required_hardware="true") + def test_06_encrypted_offline_migration_from_nfs_to_nfs(self): + """Offline migration of encrypted volumes: NFS (ROOT with chain) -> NFS""" + self.require_pools(self.nfs_pool2) + self.offline_migration_from_nfs(self.nfs_pool2) + + @attr(tags=["advanced", "advancedns", "smoke", "diskencrypt"], required_hardware="true") + def test_07_encrypted_detached_volume_migration(self): + """Migration of a detached encrypted DATA volume: RBD -> NFS -> NFS -> RBD -> RBD""" + self.detached_volume_migration() diff --git a/test/integration/smoke/test_volumes.py b/test/integration/smoke/test_volumes.py index 3cae523b6ca8..c66f224d63b9 100644 --- a/test/integration/smoke/test_volumes.py +++ b/test/integration/smoke/test_volumes.py @@ -54,6 +54,9 @@ from nose.plugins.attrib import attr +# md5 of no data at all +EMPTY_MD5 = "d41d8cd98f00b204e9800998ecf8427e" + _multiprocess_shared_ = True @@ -1611,6 +1614,141 @@ def test_03_root_and_data_volume_encryption(self): self.assertEqual(ret[0], SUCCESS, "Check if promised disk size actually available") time.sleep(self.services["sleep"]) + @attr(tags=["advanced", "smoke", "diskencrypt"], required_hardware="true") + def test_04_encrypted_volume_migration_between_rbd_pools(self): + """Test migration of encrypted ROOT and DATA volumes between two RBD (Ceph) pools + + # Validate the following + # 1. Create VM using the service offering with encryptroot true and a data volume using + # the disk offering with encrypt true, and write data to the data volume in the guest + # 2. Migrate both volumes online (VM running) to the other RBD pool + # 3. Migrate both volumes offline (VM stopped) back to the first RBD pool + # 4. Migrate the data volume while it is detached to the other RBD pool + # 5. After every migration, verify the volumes are on the expected pool, still encrypted + # (natively by librbd), and that the guest reads back the data it wrote + """ + + if self.storage_pool_type != "RBD": + self.skipTest("Skipping test as it requires RBD storage pools") + rbd_pools = [pool for pool in list_storage_pools(self.apiclient, zoneid=self.zone.id) or [] + if pool.type == "RBD" and pool.state == "Up"] + if len(rbd_pools) < 2: + self.skipTest("Skipping test as it requires two RBD storage pools") + + virtual_machine = VirtualMachine.create( + self.apiclient, + self.services, + accountid=self.account.name, + domainid=self.account.domainid, + serviceofferingid=self.service_offering_encrypt.id, + mode=self.services["mode"] + ) + self.cleanup.append(virtual_machine) + + volume = Volume.create( + self.apiclient, + self.services, + zoneid=self.zone.id, + account=self.account.name, + domainid=self.account.domainid, + diskofferingid=self.disk_offering_encrypt.id + ) + self.cleanup.append(volume) + virtual_machine.attach_volume(self.apiclient, volume) + + root_volume = Volume.list(self.apiclient, virtualmachineid=virtual_machine.id, type='ROOT', listall=True)[0] + source_pool = [pool for pool in rbd_pools if pool.id == root_volume.storageid] + if not source_pool: + self.skipTest("Skipping test as the ROOT volume was not allocated on an RBD pool") + source_pool = source_pool[0] + other_pool = [pool for pool in rbd_pools if pool.id != source_pool.id][0] + # The VM is restarted after attaching the data volume, as not every guest detects a hot plugged disk + virtual_machine.stop(self.apiclient) + if Volume.list(self.apiclient, id=volume.id)[0].storageid != source_pool.id: + Volume.migrate(self.apiclient, volumeid=volume.id, storageid=source_pool.id, livemigrate=False) + virtual_machine.start(self.apiclient) + + def data_disk_device(ssh): + # The data volume is found in the guest by its size: the device name may change after a migration + # or a restart, and older guests have no /dev/disk/by-id links for virtio disks + listed = Volume.list(self.apiclient, id=volume.id)[0] + size = self.get_expected_guest_visible_size(listed, listed.size) + disks = [] + for _ in range(6): + disks = ssh.execute("/sbin/fdisk -l 2>/dev/null | grep '^Disk /dev/'") + for line in disks: + if " %s bytes" % size in line: + return line.split()[1].rstrip(":") + time.sleep(10) + self.fail("No disk of %s bytes (volume %s) found in the guest: %s" % (size, volume.id, disks)) + + def read_checksum(step): + ssh = virtual_machine.get_ssh_client(reconnect=True) + device = data_disk_device(ssh) + result = ssh.execute("dd if=%s bs=1M count=8 iflag=direct 2>/dev/null | md5sum" % device)[0].split()[0] + self.assertNotEqual(result, EMPTY_MD5, "%s: nothing could be read from %s" % (step, device)) + return result + + # Write random data to the raw data disk + ssh = virtual_machine.get_ssh_client(reconnect=True) + device = data_disk_device(ssh) + ssh.execute("dd if=/dev/urandom of=/tmp/encrypted-volume-data bs=1M count=8") + written = ssh.execute("dd if=/tmp/encrypted-volume-data of=%s bs=1M oflag=direct 2>&1 && sync" % device) + self.assertTrue(any("8388608 bytes" in line for line in written), "Failed to write to %s: %s" % (device, written)) + checksum = ssh.execute("md5sum < /tmp/encrypted-volume-data")[0].split()[0] + self.assertEqual(read_checksum("before migration"), checksum, "The data written to %s can not be read back" % device) + + def verify(pool, step): + for vol in (root_volume, volume): + migrated = Volume.list(self.apiclient, id=vol.id)[0] + self.assertEqual(migrated.storageid, pool.id, "%s: volume %s is not on pool %s" % (step, vol.id, pool.name)) + self.assertEqual(migrated.state, "Ready", "%s: volume %s is not Ready" % (step, vol.id)) + self.check_volume_encryption(virtual_machine, 2) + self.assertEqual(read_checksum(step), checksum, "%s: the data of the encrypted volume is not intact" % step) + + # Online: the VM keeps running while the volumes are copied with a libvirt block copy + for vol in (root_volume, volume): + Volume.migrate(self.apiclient, volumeid=vol.id, storageid=other_pool.id, livemigrate=True) + self.assertEqual(VirtualMachine.list(self.apiclient, id=virtual_machine.id)[0].state, "Running", + "VM should stay running during the online migration") + verify(other_pool, "online migration") + + # Offline: the volumes are copied through their encryption with qemu-img + virtual_machine.stop(self.apiclient) + for vol in (root_volume, volume): + Volume.migrate(self.apiclient, volumeid=vol.id, storageid=source_pool.id, livemigrate=False) + virtual_machine.start(self.apiclient) + verify(source_pool, "offline migration") + + # Offline, with the data volume detached (the VM is stopped, as not every disk bus supports hot unplugging) + virtual_machine.stop(self.apiclient) + virtual_machine.detach_volume(self.apiclient, volume) + Volume.migrate(self.apiclient, volumeid=volume.id, storageid=other_pool.id, livemigrate=False) + virtual_machine.attach_volume(self.apiclient, volume) + virtual_machine.start(self.apiclient) + migrated = Volume.list(self.apiclient, id=volume.id)[0] + self.assertEqual(migrated.storageid, other_pool.id, "Detached volume %s is not on pool %s" % (volume.id, other_pool.name)) + self.check_volume_encryption(virtual_machine, 2) + self.assertEqual(read_checksum("detached migration"), checksum, "detached migration: the data of the encrypted volume is not intact") + + virtual_machine.detach_volume(self.apiclient, volume) + + def get_expected_guest_visible_size(self, volume, vol_sz): + """Some pool types/encryption mechanisms reserve overhead out of the requested size, so the + guest sees fewer usable bytes than the volume's nominal size. Returns the byte count the + guest should actually report for the given volume. + """ + list_volume_pool_response = list_storage_pools(self.apiclient, id=volume.storageid) + volume_pool = list_volume_pool_response[0] + vol_sz = int(vol_sz) + if volume_pool.type.lower() == "powerflex": + vol_sz = vol_sz - (128 << 20) - ((vol_sz >> 30) * 200704) + elif volume_pool.type == "RBD" and getattr(volume, "encryptformat", None): + # RBD volumes are encrypted natively by librbd: the LUKS2 header/keyslots (16 MiB) are + # reserved out of the volume's own size, so the guest sees size - header. + vol_sz = vol_sz - (16 << 20) + return str(vol_sz) + def does_host_with_encryption_support_exists(self): hosts = Host.list( self.apiclient,