importVM improvements: RBD support, volume format, and cluster selection (#14162)

* kvm: allow importVm importsource=shared from an RBD pool

The CheckVolumeCommand wrapper on the KVM agent only accepted file based
pools, so importing a root disk straight from Ceph failed on the agent
with "Unsupported Storage Pool" and surfaced as "Disk not found or is
invalid" on the management server. Add RBD to the supported pool types,
take the virtual size from the disk libvirt resolved (qemu-img cannot
open a bare RBD image name), skip the QCOW2 header check for raw RBD
images, and build the rbd: URI when running qemu-img info, the same way
LibvirtGetVolumesOnStorageCommandWrapper already does for
listVolumesForImport.

* kvm: record the real image format on an imported volume

importVolume and updateImportedVolume both stamped the cluster default
format for the hypervisor, so a volume imported from an RBD pool was
recorded as QCOW2 while a natively deployed volume on the same pool is
RAW. This affected both entry points: importVm importsource=shared for a
root disk, and importVolume for a data disk.

Pass the format the hypervisor reported for the existing image, from the
check answer for a root disk and from the volume listed on the pool for
a data disk, and fall back to the hypervisor default only when no format
is reported. This also corrects a raw image imported from a file based
pool.

* vm import: plan the instance inside the pod and cluster of the requested pool

importKVMInstanceFromDiskImage planned with the pod and cluster unset, so
the planner was free to pick any host in the zone by capacity. When it
picked a host in a cluster that cannot see the pool the caller named, the
volume check ran against whichever pool that cluster does have, and the
import failed with "Disk not found or is invalid" although the image was
fine.

Take the pod and cluster from the pool the caller passed, the same way
importVolume already derives its host from the pool's scope

* verify qemu is able to read the rbd image since check can not be done on a raw file

* use computed volumeDetails in success path as well.

* ui: do not call the imported disk a QCOW2 image

The import wizard for local and shared storage described the disk as a
QCOW2 image. An image on an RBD pool is raw, and a raw image on a file
based pool can be imported as well, so the wording is wrong for both.
Call it a disk image instead.

Only the English strings are changed; the other locales come from
Transifex.

Author: abh1sar

api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportVmCmd.java

@@ -92,7 +92,7 @@ public class ImportVmCmd extends ImportUnmanagedInstanceCmd {
 
     @Parameter(name = ApiConstants.DISK_PATH,
             type = CommandType.STRING,
-            description = "path of the disk image")
+            description = "path of the disk image. It is the file name on file based storage pools (NFS, Local, SharedMountPoint), and the image name on RBD storage pools")
     private String diskPath;
 
     @Parameter(name = ApiConstants.IMPORT_SOURCE,

api/src/main/java/org/apache/cloudstack/api/command/admin/volume/ImportVolumeCmd.java

@@ -63,7 +63,7 @@ public class ImportVolumeCmd extends BaseAsyncCmd {
     @Parameter(name = ApiConstants.PATH,
             type = BaseCmd.CommandType.STRING,
             required = true,
-            description = "the path of the volume")
+            description = "the path of the volume. It is the file name on file based storage pools (NFS, Local, SharedMountPoint), and the image name on RBD storage pools")
     private String path;
 
     @Parameter(name = ApiConstants.NAME,

engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java

@@ -183,10 +183,12 @@ List<DiskProfile> allocateTemplatedVolumes(Type type, String name, DiskOffering
      */
     DiskProfile importVolume(Type type, String name, DiskOffering offering, Long sizeInBytes, Long minIops, Long maxIops,
                              Long zoneId, HypervisorType hypervisorType, VirtualMachine vm, VirtualMachineTemplate template,
-                             Account owner, Long deviceId, Long poolId, Storage.StoragePoolType poolType, String path, String chainInfo);
+                             Account owner, Long deviceId, Long poolId, Storage.StoragePoolType poolType, String path, String chainInfo,
+                             Storage.ImageFormat format);
 
     DiskProfile updateImportedVolume(Type type, DiskOffering offering, VirtualMachine vm, VirtualMachineTemplate template,
-                                     Long deviceId, Long poolId, Storage.StoragePoolType poolType, String path, String chainInfo, DiskProfile diskProfile);
+                                     Long deviceId, Long poolId, Storage.StoragePoolType poolType, String path, String chainInfo, DiskProfile diskProfile,
+                                     Storage.ImageFormat format);
 
     /**
      * Unmanage VM volumes

engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java

@@ -2328,7 +2328,8 @@ public void updateVolumeDiskChain(long volumeId, String path, String chainInfo,
     @Override
     public DiskProfile importVolume(Type type, String name, DiskOffering offering, Long sizeInBytes, Long minIops, Long maxIops,
                                     Long zoneId, HypervisorType hypervisorType, VirtualMachine vm, VirtualMachineTemplate template, Account owner,
-                                    Long deviceId, Long poolId, Storage.StoragePoolType poolType, String path, String chainInfo) {
+                                    Long deviceId, Long poolId, Storage.StoragePoolType poolType, String path, String chainInfo,
+                                    ImageFormat format) {
         if (sizeInBytes == null) {
             sizeInBytes = offering.getDiskSize();
         }
@@ -2367,7 +2368,9 @@ public DiskProfile importVolume(Type type, String name, DiskOffering offering, L
             vol.setDisplayVolume(userVm.isDisplayVm());
         }
 
-        vol.setFormat(getSupportedImageFormatForCluster(hypervisorType));
+        // The format the hypervisor actually reported for the existing image wins; pools such as RBD
+        // hold raw images even though QCOW2 is the cluster default for KVM.
+        vol.setFormat(format != null ? format : getSupportedImageFormatForCluster(hypervisorType));
         vol.setPoolId(poolId);
         vol.setPoolType(poolType);
         vol.setPath(path);
@@ -2379,7 +2382,8 @@ public DiskProfile importVolume(Type type, String name, DiskOffering offering, L
 
     @Override
     public DiskProfile updateImportedVolume(Type type, DiskOffering offering, VirtualMachine vm, VirtualMachineTemplate template,
-                                    Long deviceId, Long poolId, Storage.StoragePoolType poolType, String path, String chainInfo, DiskProfile diskProfile) {
+                                    Long deviceId, Long poolId, Storage.StoragePoolType poolType, String path, String chainInfo, DiskProfile diskProfile,
+                                    ImageFormat format) {
 
         VolumeVO vol = _volsDao.findById(diskProfile.getVolumeId());
         if (vm != null) {
@@ -2411,7 +2415,9 @@ public DiskProfile updateImportedVolume(Type type, DiskOffering offering, Virtua
             vol.setDisplayVolume(userVm.isDisplayVm());
         }
 
-        vol.setFormat(getSupportedImageFormatForCluster(vm.getHypervisorType()));
+        // The format the hypervisor actually reported for the existing image wins; pools such as RBD
+        // hold raw images even though QCOW2 is the cluster default for KVM.
+        vol.setFormat(format != null ? format : getSupportedImageFormatForCluster(vm.getHypervisorType()));
         vol.setPoolId(poolId);
         vol.setPoolType(poolType);
         vol.setPath(path);

engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java

@@ -241,7 +241,7 @@ public void testImportVolume() {
 
         volumeOrchestrator.importVolume(volumeType, name, diskOffering, sizeInBytes, null, null,
                 zoneId, hypervisorType, null, null, owner,
-                deviceId, poolId, Storage.StoragePoolType.NetworkFilesystem, path, chainInfo);
+                deviceId, poolId, Storage.StoragePoolType.NetworkFilesystem, path, chainInfo, null);
 
         VolumeVO volume = volumeVOMockedConstructionConstruction.constructed().get(0);
         Mockito.verify(volume, Mockito.never()).setInstanceId(Mockito.anyLong());

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

@@ -50,7 +50,8 @@ public final class LibvirtCheckVolumeCommandWrapper extends CommandWrapper<Check
     private static final List<Storage.StoragePoolType> STORAGE_POOL_TYPES_SUPPORTED = Arrays.asList(
             Storage.StoragePoolType.Filesystem,
             Storage.StoragePoolType.NetworkFilesystem,
-            Storage.StoragePoolType.SharedMountPoint);
+            Storage.StoragePoolType.SharedMountPoint,
+            Storage.StoragePoolType.RBD);
 
     @Override
     public Answer execute(final CheckVolumeCommand command, final LibvirtComputingResource libvirtComputingResource) {
@@ -64,14 +65,25 @@ public Answer execute(final CheckVolumeCommand command, final LibvirtComputingRe
             if (STORAGE_POOL_TYPES_SUPPORTED.contains(storageFilerTO.getType())) {
                 final KVMPhysicalDisk vol = pool.getPhysicalDisk(srcFile);
                 final String path = vol.getPath();
-                try {
-                    KVMPhysicalDisk.checkQcow2File(path);
-                } catch (final CloudRuntimeException e) {
-                    return new CheckVolumeAnswer(command, false, "", 0, getVolumeDetails(pool, vol));
+                final boolean isRbd = Storage.StoragePoolType.RBD.equals(storageFilerTO.getType());
+
+                Map<VolumeOnStorageTO.Detail, String> volumeDetails = getVolumeDetails(pool, vol);
+                if (MapUtils.isEmpty(volumeDetails)) {
+                    return new Answer(command, false, "Unable to read the volume on the storage pool");
+                }
+
+                if (!isRbd) {
+                    try {
+                        KVMPhysicalDisk.checkQcow2File(path);
+                    } catch (final CloudRuntimeException e) {
+                        return new CheckVolumeAnswer(command, false, "", 0, volumeDetails);
+                    }
                 }
 
-                long size = KVMPhysicalDisk.getVirtualSizeFromFile(path);
-                return new CheckVolumeAnswer(command, true, "", size, getVolumeDetails(pool, vol));
+                // Images on RBD are raw and the path is an image name that qemu-img cannot open
+                // without the rbd: URI, so take the size libvirt already reported for the volume.
+                long size = isRbd ? vol.getVirtualSize() : KVMPhysicalDisk.getVirtualSizeFromFile(path);
+                return new CheckVolumeAnswer(command, true, "", size, volumeDetails);
             } else {
                 return new Answer(command, false, "Unsupported Storage Pool");
             }
@@ -122,6 +134,9 @@ private Map<String, String> getDiskFileInfo(KVMStoragePool pool, KVMPhysicalDisk
         try {
             QemuImg qemu = new QemuImg(0);
             QemuImgFile qemuFile = new QemuImgFile(disk.getPath(), disk.getFormat());
+            if (Storage.StoragePoolType.RBD.equals(pool.getType())) {
+                qemuFile = new QemuImgFile(KVMPhysicalDisk.RBDStringBuilder(pool, disk.getPath()), disk.getFormat());
+            }
             return qemu.info(qemuFile, secure);
         } catch (QemuImgException | LibvirtException ex) {
             logger.error("Failed to get info of disk file: " + ex.getMessage());

server/src/main/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImpl.java

@@ -461,10 +461,27 @@ private VolumeVO importVolumeInternal(VolumeOnStorageTO volume, DiskOfferingVO d
                                           Account owner, StoragePoolVO pool, String volumeName) {
         DiskProfile diskProfile = volumeManager.importVolume(Volume.Type.DATADISK, volumeName, diskOffering,
                 volume.getVirtualSize(), null, null, pool.getDataCenterId(), volume.getHypervisorType(), null, null,
-                owner, null, pool.getId(), pool.getPoolType(), volume.getPath(), null);
+                owner, null, pool.getId(), pool.getPoolType(), volume.getPath(), null, getImageFormat(volume.getFormat()));
         return volumeDao.findById(diskProfile.getVolumeId());
     }
 
+    /**
+     * Maps the format the hypervisor reported for the volume on the pool onto an image format, so that
+     * the imported volume records what is actually on the pool (raw on RBD, qcow2 on file based pools)
+     * instead of the cluster default for the hypervisor. Returns null when the format is not recognised.
+     */
+    protected Storage.ImageFormat getImageFormat(String format) {
+        if (StringUtils.isBlank(format)) {
+            return null;
+        }
+        try {
+            return Storage.ImageFormat.valueOf(format.toUpperCase());
+        } catch (IllegalArgumentException e) {
+            logger.warn("Unrecognised image format {} reported for the volume being imported, falling back to the hypervisor default", format);
+            return null;
+        }
+    }
+
     protected void checkResourceLimitForImportVolume(Account owner, VolumeOnStorageTO volume, DiskOfferingVO diskOffering, List<Reserver> reservations) {
         Long volumeSize = volume.getVirtualSize();
         try {

server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java

@@ -796,14 +796,16 @@ private Pair<DiskProfile, StoragePool> importExternalDisk(UnmanagedInstanceTO.Di
         }
         diskProfile.setSize(copyRemoteVolumeAnswer.getSize());
         DiskProfile profile = volumeManager.updateImportedVolume(type, diskOffering, vm, template, deviceId,
-                storagePool.getId(), storagePool.getPoolType(), copyRemoteVolumeAnswer.getFilename(), chainInfo, diskProfile);
+                storagePool.getId(), storagePool.getPoolType(), copyRemoteVolumeAnswer.getFilename(), chainInfo, diskProfile,
+                getImageFormatFromVolumeDetails(copyRemoteVolumeAnswer.getVolumeDetails()));
 
         return new Pair<>(profile, storagePool);
     }
 
     private Pair<DiskProfile, StoragePool> importKVMLocalDisk(VirtualMachine vm, DiskOffering diskOffering,
                                                               Volume.Type type, VirtualMachineTemplate template,
-                                                              Long deviceId, Long hostId, String diskPath, DiskProfile diskProfile) {
+                                                              Long deviceId, Long hostId, String diskPath, DiskProfile diskProfile,
+                                                              Storage.ImageFormat format) {
         List<StoragePoolVO> storagePools = primaryDataStoreDao.findLocalStoragePoolsByHostAndTags(hostId, null);
         if(storagePools.size() < 1) {
             throw new CloudRuntimeException("Local Storage not found for host");
@@ -812,22 +814,44 @@ private Pair<DiskProfile, StoragePool> importKVMLocalDisk(VirtualMachine vm, Dis
         StoragePool storagePool = storagePools.get(0);
 
         DiskProfile profile = volumeManager.updateImportedVolume(type, diskOffering, vm, template, deviceId,
-                storagePool.getId(), storagePool.getPoolType(), diskPath, null, diskProfile);
+                storagePool.getId(), storagePool.getPoolType(), diskPath, null, diskProfile, format);
 
         return new Pair<>(profile, storagePool);
     }
 
     private Pair<DiskProfile, StoragePool> importKVMSharedDisk(VirtualMachine vm, DiskOffering diskOffering,
                                                               Volume.Type type, VirtualMachineTemplate template,
-                                                              Long deviceId, Long poolId, String diskPath, DiskProfile diskProfile) {
+                                                              Long deviceId, Long poolId, String diskPath, DiskProfile diskProfile,
+                                                              Storage.ImageFormat format) {
         StoragePool storagePool = primaryDataStoreDao.findById(poolId);
 
         DiskProfile profile = volumeManager.updateImportedVolume(type, diskOffering, vm, template, deviceId,
-                poolId, storagePool.getPoolType(), diskPath, null, diskProfile);
+                poolId, storagePool.getPoolType(), diskPath, null, diskProfile, format);
 
         return new Pair<>(profile, storagePool);
     }
 
+    /**
+     * Reads the image format the hypervisor reported for an existing volume, so that the imported
+     * volume records what is actually on the pool (raw on RBD, qcow2 on file based pools) instead of
+     * the cluster default for the hypervisor. Returns null when the agent did not report a format.
+     */
+    protected Storage.ImageFormat getImageFormatFromVolumeDetails(Map<VolumeOnStorageTO.Detail, String> volumeDetails) {
+        if (MapUtils.isEmpty(volumeDetails)) {
+            return null;
+        }
+        String fileFormat = volumeDetails.get(VolumeOnStorageTO.Detail.FILE_FORMAT);
+        if (StringUtils.isBlank(fileFormat)) {
+            return null;
+        }
+        try {
+            return Storage.ImageFormat.valueOf(fileFormat.toUpperCase());
+        } catch (IllegalArgumentException e) {
+            logger.warn("Unrecognised image format {} reported for the volume being imported, falling back to the hypervisor default", fileFormat);
+            return null;
+        }
+    }
+
     private Pair<DiskProfile, StoragePool> importDisk(UnmanagedInstanceTO.Disk disk, VirtualMachine vm, Cluster cluster, DiskOffering diskOffering,
                                                       Volume.Type type, String name, Long diskSize, Long minIops, Long maxIops, VirtualMachineTemplate template,
                                                       Account owner, Long deviceId) {
@@ -842,7 +866,7 @@ private Pair<DiskProfile, StoragePool> importDisk(UnmanagedInstanceTO.Disk disk,
         }
         StoragePool storagePool = getStoragePool(disk, zone, cluster, diskOffering);
         DiskProfile profile = volumeManager.importVolume(type, name, diskOffering, diskSize,
-                minIops, maxIops, vm.getDataCenterId(), vm.getHypervisorType(), vm, template, owner, deviceId, storagePool.getId(), storagePool.getPoolType(), path, chainInfo);
+                minIops, maxIops, vm.getDataCenterId(), vm.getHypervisorType(), vm, template, owner, deviceId, storagePool.getId(), storagePool.getPoolType(), path, chainInfo, null);
 
         return new Pair<DiskProfile, StoragePool>(profile, storagePool);
     }
@@ -2647,6 +2671,13 @@ private UserVmResponse importKvmInstance(ImportVmCmd cmd) {
                 throw new InvalidParameterValueException("Disk image is already in use");
             }
 
+            // A host the planner is pinned to must be able to see the pool, otherwise the volume check
+            // runs on a host that cannot reach the image and reports it as missing.
+            if (ImportSource.SHARED == importSource && hostId != null && storagePoolHostDao.findByPoolHost(poolId, hostId) == null) {
+                throw new InvalidParameterValueException(String.format(
+                        "Specified host does not have access to the storage pool: %s", storagePool.getUuid()));
+            }
+
             DiskOffering diskOffering = diskOfferingDao.findById(serviceOffering.getDiskOfferingId());
 
             if (diskOffering != null && !storagePoolSupportsDiskOffering(storagePool, diskOffering)) {
@@ -2923,7 +2954,13 @@ private UserVm importKvmVirtualMachineFromDisk(final ImportSource importSource,
             ServiceOfferingVO dummyOffering = serviceOfferingDao.findById(userVm.getId(), serviceOffering.getId());
             profile.setServiceOffering(dummyOffering);
             DeploymentPlanner.ExcludeList excludeList = new DeploymentPlanner.ExcludeList();
-            final DataCenterDeployment plan = new DataCenterDeployment(zone.getId(), null, null, hostId, poolId, null);
+            // Confine the plan to the pod and cluster of the pool the caller asked for. Otherwise the
+            // planner is free to pick a host in another cluster that cannot see the pool, and the volume
+            // check then runs against whichever pool that cluster does have. Both are null for a zone
+            // wide pool, which every host can see.
+            StoragePoolVO importStoragePool = primaryDataStoreDao.findById(poolId);
+            final DataCenterDeployment plan = new DataCenterDeployment(zone.getId(), importStoragePool.getPodId(),
+                    importStoragePool.getClusterId(), hostId, poolId, null);
             DeployDestination dest = null;
             try {
                 dest = deploymentPlanningManager.planDeployment(profile, plan, excludeList, null);
@@ -2968,12 +3005,13 @@ private UserVm importKvmVirtualMachineFromDisk(final ImportSource importSource,
             List<Pair<DiskProfile, StoragePool>> diskProfileStoragePoolList = new ArrayList<>();
             try {
                 long deviceId = 1L;
+                Storage.ImageFormat diskFormat = getImageFormatFromVolumeDetails(checkVolumeAnswer.getVolumeDetails());
                 if(ImportSource.SHARED == importSource) {
                     diskProfileStoragePoolList.add(importKVMSharedDisk(userVm, diskOffering, Volume.Type.ROOT,
-                            template, deviceId, poolId, diskPath, diskProfile));
+                            template, deviceId, poolId, diskPath, diskProfile, diskFormat));
                 } else if(ImportSource.LOCAL == importSource) {
                     diskProfileStoragePoolList.add(importKVMLocalDisk(userVm, diskOffering, Volume.Type.ROOT,
-                            template, deviceId, hostId, diskPath, diskProfile));
+                            template, deviceId, hostId, diskPath, diskProfile, diskFormat));
                 }
             } catch (Exception e) {
                 logger.error(String.format("Failed to import volumes while importing vm: %s", instanceName), e);