KVM: volume migration on Ceph RBD, and between RBD, NFS, SharedMountPoint and local pools

api/src/main/java/com/cloud/storage/Storage.java

@@ -20,6 +20,7 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Set;
 
 public class Storage {
     public static enum ImageFormat {
@@ -226,6 +227,47 @@ public static List<StoragePoolType> getNonSharedStoragePoolTypes() {
 
     }
 
+    // pool types whose volumes are files, on KVM
+    public static final Set<StoragePoolType> KVM_FILE_BASED_POOL_TYPES = Set.of(StoragePoolType.NetworkFilesystem, StoragePoolType.SharedMountPoint,
+            StoragePoolType.Filesystem);
+
+    /**
+     * True if a volume is migrated between two pools of the same type.
+     */
+    public static boolean arePoolsSameType(StoragePoolType srcType, StoragePoolType destType) {
+        return srcType != null && srcType.equals(destType);
+    }
+
+    /**
+     * True if at least one of the pools is file based (NFS, SharedMountPoint or local) on KVM, whether it is the
+     * source or the destination, and whether the other one is file based as well.
+     */
+    public static boolean isAnyPoolFileBased(StoragePoolType srcType, StoragePoolType destType) {
+        return KVM_FILE_BASED_POOL_TYPES.contains(srcType) || KVM_FILE_BASED_POOL_TYPES.contains(destType);
+    }
+
+    /**
+     * True if KVM copies a volume in place with a libvirt block copy: between two RBD pools, or between an RBD pool and
+     * a file based pool (NFS, SharedMountPoint or local). Both pools are then reachable from the host the Instance runs
+     * on, so the volume of a running Instance can be copied without moving the Instance.
+     */
+    public static boolean isCopiedInPlaceOnKvm(StoragePoolType srcType, StoragePoolType destType) {
+        if (!StoragePoolType.RBD.equals(srcType) && !StoragePoolType.RBD.equals(destType)) {
+            return false;
+        }
+        return arePoolsSameType(srcType, destType) || isAnyPoolFileBased(srcType, destType);
+    }
+
+    /**
+     * True if a volume is migrated between two file based pools (NFS, SharedMountPoint or local) on KVM. Such a volume
+     * is copied with qemu-img, which reads and writes an encrypted volume with its passphrase, so it is not limited to
+     * the pools a libvirt block copy can reach. It is the way a volume is moved when it is not in use by a running
+     * Instance.
+     */
+    public static boolean areBothPoolsFileBased(StoragePoolType srcType, StoragePoolType destType) {
+        return KVM_FILE_BASED_POOL_TYPES.contains(srcType) && KVM_FILE_BASED_POOL_TYPES.contains(destType);
+    }
+
     public static enum StorageResourceType {
         STORAGE_POOL, STORAGE_HOST, SECONDARY_STORAGE, LOCAL_SECONDARY_STORAGE
     }

api/src/test/java/com/cloud/storage/StorageTest.java

@@ -83,4 +83,82 @@ public void equalityTest() {
         Assert.assertTrue(t1.equals(StoragePoolType.NetworkFilesystem));
         Assert.assertFalse(t1.equals(StoragePoolType.EXT));
     }
+
+    @Test
+    public void arePoolsSameTypeForTwoPoolsOfThatType() {
+        for (StoragePoolType poolType : StoragePoolType.values()) {
+            Assert.assertTrue(poolType + " -> " + poolType, Storage.arePoolsSameType(poolType, poolType));
+            for (StoragePoolType other : StoragePoolType.values()) {
+                if (other.equals(poolType)) {
+                    continue;
+                }
+                Assert.assertFalse(poolType + " -> " + other, Storage.arePoolsSameType(poolType, other));
+            }
+        }
+    }
+
+    @Test
+    public void isAnyPoolFileBasedForOneFileBasedPool() {
+        for (StoragePoolType fileBased : Storage.KVM_FILE_BASED_POOL_TYPES) {
+            Assert.assertTrue("RBD -> " + fileBased, Storage.isAnyPoolFileBased(StoragePoolType.RBD, fileBased));
+            Assert.assertTrue(fileBased + " -> RBD", Storage.isAnyPoolFileBased(fileBased, StoragePoolType.RBD));
+            for (StoragePoolType otherFileBased : Storage.KVM_FILE_BASED_POOL_TYPES) {
+                Assert.assertTrue(fileBased + " -> " + otherFileBased, Storage.isAnyPoolFileBased(fileBased, otherFileBased));
+            }
+        }
+        Assert.assertFalse("RBD -> RBD", Storage.isAnyPoolFileBased(StoragePoolType.RBD, StoragePoolType.RBD));
+    }
+
+    @Test
+    public void isCopiedInPlaceOnKvmForRbdAndForRbdAndFileBasedPools() {
+        Assert.assertTrue("RBD -> RBD", Storage.isCopiedInPlaceOnKvm(StoragePoolType.RBD, StoragePoolType.RBD));
+        for (StoragePoolType fileBased : Storage.KVM_FILE_BASED_POOL_TYPES) {
+            Assert.assertTrue("RBD -> " + fileBased, Storage.isCopiedInPlaceOnKvm(StoragePoolType.RBD, fileBased));
+            Assert.assertTrue(fileBased + " -> RBD", Storage.isCopiedInPlaceOnKvm(fileBased, StoragePoolType.RBD));
+        }
+    }
+
+    @Test
+    public void isCopiedInPlaceOnKvmNotBetweenTwoFileBasedPools() {
+        for (StoragePoolType srcType : Storage.KVM_FILE_BASED_POOL_TYPES) {
+            for (StoragePoolType destType : Storage.KVM_FILE_BASED_POOL_TYPES) {
+                Assert.assertFalse(srcType + " -> " + destType, Storage.isCopiedInPlaceOnKvm(srcType, destType));
+            }
+        }
+    }
+
+    @Test
+    public void isCopiedInPlaceOnKvmNotWithOtherPoolTypes() {
+        for (StoragePoolType other : StoragePoolType.values()) {
+            if (StoragePoolType.RBD.equals(other) || Storage.KVM_FILE_BASED_POOL_TYPES.contains(other)) {
+                continue;
+            }
+            Assert.assertFalse(other + " -> " + other, Storage.isCopiedInPlaceOnKvm(other, other));
+            Assert.assertFalse("RBD -> " + other, Storage.isCopiedInPlaceOnKvm(StoragePoolType.RBD, other));
+            Assert.assertFalse(other + " -> RBD", Storage.isCopiedInPlaceOnKvm(other, StoragePoolType.RBD));
+        }
+    }
+
+    @Test
+    public void areBothPoolsFileBasedBetweenTwoFileBasedPools() {
+        for (StoragePoolType srcType : Storage.KVM_FILE_BASED_POOL_TYPES) {
+            for (StoragePoolType destType : Storage.KVM_FILE_BASED_POOL_TYPES) {
+                Assert.assertTrue(srcType + " -> " + destType, Storage.areBothPoolsFileBased(srcType, destType));
+            }
+        }
+    }
+
+    @Test
+    public void areBothPoolsFileBasedNotWithOtherPoolTypes() {
+        for (StoragePoolType other : StoragePoolType.values()) {
+            if (Storage.KVM_FILE_BASED_POOL_TYPES.contains(other)) {
+                continue;
+            }
+            Assert.assertFalse(other + " -> " + other, Storage.areBothPoolsFileBased(other, other));
+            for (StoragePoolType fileBased : Storage.KVM_FILE_BASED_POOL_TYPES) {
+                Assert.assertFalse(fileBased + " -> " + other, Storage.areBothPoolsFileBased(fileBased, other));
+                Assert.assertFalse(other + " -> " + fileBased, Storage.areBothPoolsFileBased(other, fileBased));
+            }
+        }
+    }
 }

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<String, String> details) {
         srcDetails = details;
     }

engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java

@@ -95,6 +95,7 @@ public class AncientDataMotionStrategy implements DataMotionStrategy {
     private static final List<StoragePoolType> SUPPORTED_POOL_TYPES_TO_BYPASS_SECONDARY_STORE = Arrays.asList(
             StoragePoolType.NetworkFilesystem,
             StoragePoolType.Filesystem,
+            StoragePoolType.SharedMountPoint,
             StoragePoolType.RBD
     );
 
@@ -642,6 +643,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);
@@ -675,6 +678,21 @@ 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
+                    && Storage.KVM_FILE_BASED_POOL_TYPES.contains(destPool.getPoolType())) {
+                // 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());

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();

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java

@@ -1424,7 +1424,7 @@ public boolean configure(final String name, final Map<String, Object> params) th
 
             if (this.bridgeType == BridgeType.OPENVSWITCH) {
                 if (conn.getLibVirVersion() < 10 * 1000 + 0) {
-                    throw new ConfigurationException("Libvirt version 0.10.0 required for openvswitch support, but version " + conn.getLibVirVersion() + " detected");
+                    throw new ConfigurationException("Libvirt version 0.10.0 required for openvswitch support, but version " + prettyVersion(conn.getLibVirVersion()) + " detected");
                 }
             }
         } catch (final LibvirtException e) {
@@ -6104,7 +6104,7 @@ public Pair<Double, Double> getNicStats(final String nicName) {
         }
     }
 
-    private String prettyVersion(final long version) {
+    public static String prettyVersion(final long version) {
         final long major = version / 1000000;
         final long minor = version % 1000000 / 1000;
         final long release = version % 1000000 % 1000;