KVM: volume migration on Ceph RBD, and between RBD, NFS, SharedMountPoint, local and CLVM 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
     }

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StoragePoolInformation.java → api/src/main/java/com/cloud/storage/StoragePoolInformation.java

@@ -14,13 +14,15 @@
 // KIND, either express or implied.  See the License for the
 // specific language governing permissions and limitations
 // under the License.
-package com.cloud.hypervisor.kvm.storage;
-
-import com.cloud.storage.Storage;
+package com.cloud.storage;
 
 import java.util.Map;
 
-class StoragePoolInformation {
+/**
+ * What an agent needs to know to reach a primary storage pool: for a pool that is not (yet) defined on the host, e.g. a
+ * pool of another cluster to which an Instance is migrated.
+ */
+public class StoragePoolInformation {
     private String name;
     private String host;
     private int port;

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/MigrateCommand.java

@@ -27,6 +27,7 @@
 import com.cloud.agent.api.to.DpdkTO;
 import com.cloud.agent.api.to.VirtualMachineTO;
 import com.cloud.storage.Storage;
+import com.cloud.storage.StoragePoolInformation;
 
 public class MigrateCommand extends Command {
     private String vmName;
@@ -161,7 +162,7 @@ public void setClvmCrossPoolMigration(boolean clvmCrossPoolMigration) {
 
     public static class MigrateDiskInfo {
         public enum DiskType {
-            FILE, BLOCK;
+            FILE, BLOCK, NETWORK;
 
             @Override
             public String toString() {
@@ -179,7 +180,7 @@ public String toString() {
         }
 
         public enum Source {
-            FILE, DEV;
+            FILE, DEV, NETWORK;
 
             @Override
             public String toString() {
@@ -196,6 +197,7 @@ public String toString() {
         private boolean isSourceDiskOnStorageFileSystem;
         private Storage.StoragePoolType sourcePoolType;
         private Storage.StoragePoolType destPoolType;
+        private StoragePoolInformation destPoolInformation;
 
         public MigrateDiskInfo(final String serialNumber, final DiskType diskType, final DriverType driverType, final Source source, final String sourceText) {
             this.serialNumber = serialNumber;
@@ -260,6 +262,19 @@ public Storage.StoragePoolType getDestPoolType() {
         public void setDestPoolType(Storage.StoragePoolType destPoolType) {
             this.destPoolType = destPoolType;
         }
+
+        /**
+         * Describes the destination pool of a {@link DiskType#NETWORK} disk: the source host cannot look it up, as the
+         * pool may belong to a cluster it is not part of. The name is the UUID of the pool, the host holds the
+         * (comma separated) monitors and the user info only the user, so that no secret travels with the command.
+         */
+        public void setDestPoolInformation(StoragePoolInformation destPoolInformation) {
+            this.destPoolInformation = destPoolInformation;
+        }
+
+        public StoragePoolInformation getDestPoolInformation() {
+            return destPoolInformation;
+        }
     }
 
     @Override

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,7 +95,10 @@ public class AncientDataMotionStrategy implements DataMotionStrategy {
     private static final List<StoragePoolType> SUPPORTED_POOL_TYPES_TO_BYPASS_SECONDARY_STORE = Arrays.asList(
             StoragePoolType.NetworkFilesystem,
             StoragePoolType.Filesystem,
-            StoragePoolType.RBD
+            StoragePoolType.SharedMountPoint,
+            StoragePoolType.RBD,
+            StoragePoolType.CLVM,
+            StoragePoolType.CLVM_NG
     );
 
     @Inject
@@ -642,6 +645,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 +680,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/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java

@@ -44,6 +44,7 @@
 import com.cloud.exception.OperationTimedoutException;
 import com.cloud.host.Host;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.storage.clvm.ClvmPoolManager;
 import com.cloud.storage.DataStoreRole;
 import com.cloud.storage.ScopeType;
 import com.cloud.storage.Storage;
@@ -90,13 +91,35 @@ protected StrategyPriority internalCanHandle(Map<VolumeInfo, DataStore> volumeMa
         Set<VolumeInfo> volumeInfoSet = volumeMap.keySet();
         for (VolumeInfo volumeInfo : volumeInfoSet) {
             StoragePoolVO storagePoolVO = _storagePoolDao.findById(volumeInfo.getPoolId());
-            if (!supportStoragePoolType(storagePoolVO.getPoolType())) {
+            if (!supportStoragePoolType(storagePoolVO.getPoolType()) && !isRbdSourceSupported(storagePoolVO, volumeMap.get(volumeInfo))) {
                 return StrategyPriority.CANT_HANDLE;
             }
         }
         return StrategyPriority.HYPERVISOR;
     }
 
+    /**
+     * True if the volume lives on an RBD pool and goes to an RBD, NFS, SharedMountPoint, local or CLVM/CLVM_NG pool, which can be in
+     * another cluster. The Instance is live migrated with its disk copied over NBD to an image or file on the
+     * destination pool. A volume which stays on its
+     * pool is not copied (see {@link #shouldMigrateVolume}).
+     */
+    protected boolean isRbdSourceSupported(StoragePoolVO sourcePool, DataStore destDataStore) {
+        if (sourcePool == null || destDataStore == null || sourcePool.getPoolType() != StoragePoolType.RBD) {
+            return false;
+        }
+        StoragePoolVO destPool = _storagePoolDao.findById(destDataStore.getId());
+        return destPool != null && isDestinationOfRbdSource(destPool.getPoolType());
+    }
+
+    /**
+     * An RBD volume can be copied to an RBD, NFS, SharedMountPoint, local or CLVM/CLVM_NG pool.
+     */
+    protected boolean isDestinationOfRbdSource(StoragePoolType destPoolType) {
+        return destPoolType == StoragePoolType.RBD || ClvmPoolManager.isClvmPoolType(destPoolType)
+                || supportStoragePoolType(destPoolType, StoragePoolType.Filesystem);
+    }
+
     /**
      * Allow KVM live storage migration for non managed storage when:
      * - Source host and destination host are different, and are on the same cluster
@@ -173,6 +196,10 @@ protected String generateDestPath(Host destHost, StoragePoolVO destStoragePool,
             return String.format("/dev/%s/%s", vgName, destVolumeInfo.getUuid());
         }
 
+        if (destStoragePool.getPoolType() == StoragePoolType.RBD) {
+            return String.format("%s/%s", destStoragePool.getPath(), destVolumeInfo.getUuid());
+        }
+
         return new File(destStoragePool.getPath(), destVolumeInfo.getUuid()).getAbsolutePath();
     }
 
@@ -210,7 +237,9 @@ protected void setVolumePath(VolumeVO volume) {
      */
     @Override
     protected boolean shouldMigrateVolume(StoragePoolVO sourceStoragePool, Host destHost, StoragePoolVO destStoragePool) {
-        return supportStoragePoolType(sourceStoragePool.getPoolType());
+        return supportStoragePoolType(sourceStoragePool.getPoolType())
+                || (sourceStoragePool.getPoolType() == StoragePoolType.RBD && sourceStoragePool.getId() != destStoragePool.getId()
+                && isDestinationOfRbdSource(destStoragePool.getPoolType()));
     }
 
     /**