diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportVmCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportVmCmd.java index db7dcc3fb44f..a32b8dd604e7 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportVmCmd.java +++ b/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, diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/ImportVolumeCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/ImportVolumeCmd.java index 57c3ee586d35..9f29c0e9a6df 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/ImportVolumeCmd.java +++ b/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, diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java index 8686e4dd3a5a..fc9245b89704 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java @@ -183,10 +183,12 @@ List 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 diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java index 933accbda524..da827b17ff34 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java +++ b/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); diff --git a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java b/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java index b4a26c17e2e5..259dfaa6b1fa 100644 --- a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java +++ b/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()); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVolumeCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVolumeCommandWrapper.java index 6788516df741..3a42f230f845 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVolumeCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVolumeCommandWrapper.java @@ -50,7 +50,8 @@ public final class LibvirtCheckVolumeCommandWrapper extends CommandWrapper 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 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 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()); diff --git a/server/src/main/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImpl.java b/server/src/main/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImpl.java index abc2e5ca2255..4a69e32e77e4 100644 --- a/server/src/main/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImpl.java +++ b/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 reservations) { Long volumeSize = volume.getVirtualSize(); try { diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index 846eab599fd1..585f00d80eb3 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -796,14 +796,16 @@ private Pair 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 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 storagePools = primaryDataStoreDao.findLocalStoragePoolsByHostAndTags(hostId, null); if(storagePools.size() < 1) { throw new CloudRuntimeException("Local Storage not found for host"); @@ -812,22 +814,44 @@ private Pair 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 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 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 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 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(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> 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); diff --git a/server/src/test/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImplTest.java index f3ed13c3d6ba..08bcaeaced3c 100644 --- a/server/src/test/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImplTest.java @@ -274,7 +274,7 @@ public void testImportVolumeAllGood() throws ResourceAllocationException { doNothing().when(volumeApiService).validateCustomDiskOfferingSizeRange(anyLong()); doReturn(true).when(volumeApiService).doesStoragePoolSupportDiskOffering(any(), any()); doReturn(diskProfile).when(volumeManager).importVolume(any(), anyString(), any(), eq(virtualSize), isNull(), isNull(), anyLong(), - any(), isNull(), isNull(), any(), isNull(), anyLong(), any(), anyString(), isNull()); + any(), isNull(), isNull(), any(), isNull(), anyLong(), any(), anyString(), isNull(), eq(Storage.ImageFormat.QCOW2)); when(diskProfile.getVolumeId()).thenReturn(volumeId); when(volumeDao.findById(volumeId)).thenReturn(volumeVO); @@ -290,6 +290,15 @@ public void testImportVolumeAllGood() throws ResourceAllocationException { } } + @Test + public void testGetImageFormat() { + Assert.assertNull(volumeImportUnmanageManager.getImageFormat(null)); + Assert.assertNull(volumeImportUnmanageManager.getImageFormat("")); + Assert.assertNull(volumeImportUnmanageManager.getImageFormat("not-a-format")); + Assert.assertEquals(Storage.ImageFormat.RAW, volumeImportUnmanageManager.getImageFormat("raw")); + Assert.assertEquals(Storage.ImageFormat.QCOW2, volumeImportUnmanageManager.getImageFormat("qcow2")); + } + @Test public void testListVolumesForImportInternal() { Pair hostAndLocalPath = mock(Pair.class); diff --git a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java index bee6c4ad257f..30bc769d5d4f 100644 --- a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java @@ -25,6 +25,7 @@ import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -64,12 +65,14 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.jetbrains.annotations.NotNull; import org.junit.After; +import org.apache.cloudstack.storage.volume.VolumeOnStorageTO; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.BDDMockito; import org.mockito.InjectMocks; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.MockedConstruction; import org.mockito.MockedStatic; @@ -103,6 +106,7 @@ import com.cloud.dc.dao.DataCenterDao; import com.cloud.dc.dao.VmwareDatacenterDao; import com.cloud.deploy.DeployDestination; +import com.cloud.deploy.DeploymentPlan; import com.cloud.deploy.DeploymentPlanningManager; import com.cloud.event.ActionEventUtils; import com.cloud.event.UsageEventUtils; @@ -952,13 +956,55 @@ public void importFromsharedStorage() throws InsufficientServerCapacityException importFromDisk("shared"); } + @Test + public void importFromsharedStorageOnRbdPool() throws InsufficientServerCapacityException { + importFromDisk("shared", Storage.StoragePoolType.RBD, "cloudstack-image", "raw", Storage.ImageFormat.RAW); + } + + @Test + public void importFromsharedStorageOnZoneWidePool() throws InsufficientServerCapacityException { + // A zone wide pool carries no pod or cluster, so the plan must stay unconstrained and the + // planner is free to pick any host in the zone, exactly as it did before the pool was consulted. + importFromDisk("shared", Storage.StoragePoolType.NetworkFilesystem, "/mnt/pool/zonewide.qcow2", + "qcow2", Storage.ImageFormat.QCOW2, null, null); + } + + @Test + public void testGetImageFormatFromVolumeDetails() { + Assert.assertNull(unmanagedVMsManager.getImageFormatFromVolumeDetails(null)); + Assert.assertNull(unmanagedVMsManager.getImageFormatFromVolumeDetails(new HashMap<>())); + Assert.assertNull(unmanagedVMsManager.getImageFormatFromVolumeDetails( + Map.of(VolumeOnStorageTO.Detail.IS_LOCKED, "false"))); + Assert.assertNull(unmanagedVMsManager.getImageFormatFromVolumeDetails( + Map.of(VolumeOnStorageTO.Detail.FILE_FORMAT, "not-a-format"))); + Assert.assertEquals(Storage.ImageFormat.RAW, unmanagedVMsManager.getImageFormatFromVolumeDetails( + Map.of(VolumeOnStorageTO.Detail.FILE_FORMAT, "raw"))); + Assert.assertEquals(Storage.ImageFormat.QCOW2, unmanagedVMsManager.getImageFormatFromVolumeDetails( + Map.of(VolumeOnStorageTO.Detail.FILE_FORMAT, "qcow2"))); + } + + private static final Long POOL_POD_ID = 11L; + private static final Long POOL_CLUSTER_ID = 22L; + private void importFromDisk(String source) throws InsufficientServerCapacityException { + importFromDisk(source, Storage.StoragePoolType.NetworkFilesystem, "/var/lib/libvirt/images/test.qcow2", + "qcow2", Storage.ImageFormat.QCOW2); + } + + private void importFromDisk(String source, Storage.StoragePoolType poolType, String diskPath, + String reportedFileFormat, Storage.ImageFormat expectedFormat) throws InsufficientServerCapacityException { + importFromDisk(source, poolType, diskPath, reportedFileFormat, expectedFormat, POOL_POD_ID, POOL_CLUSTER_ID); + } + + private void importFromDisk(String source, Storage.StoragePoolType poolType, String diskPath, + String reportedFileFormat, Storage.ImageFormat expectedFormat, + Long poolPodId, Long poolClusterId) throws InsufficientServerCapacityException { String vmname = "testVm"; ImportVmCmd cmd = Mockito.mock(ImportVmCmd.class); when(cmd.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM.toString()); when(cmd.getName()).thenReturn(vmname); when(cmd.getImportSource()).thenReturn(source); - when(cmd.getDiskPath()).thenReturn("/var/lib/libvirt/images/test.qcow2"); + when(cmd.getDiskPath()).thenReturn(diskPath); when(cmd.getDomainId()).thenReturn(null); HostVO host = Mockito.mock(HostVO.class); when(hostDao.findById(anyLong())).thenReturn(host); @@ -972,12 +1018,16 @@ private void importFromDisk(String source) throws InsufficientServerCapacityExce Map storage = new HashMap<>(); VolumeVO volume = Mockito.mock(VolumeVO.class); StoragePoolVO storagePool = Mockito.mock(StoragePoolVO.class); + lenient().when(storagePool.getPoolType()).thenReturn(poolType); + lenient().when(storagePool.getPodId()).thenReturn(poolPodId); + lenient().when(storagePool.getClusterId()).thenReturn(poolClusterId); storage.put(volume, storagePool); when(mockDest.getStorageForDisks()).thenReturn(storage); when(mockDest.getHost()).thenReturn(host); when(volumeDao.findById(anyLong())).thenReturn(volume); CheckVolumeAnswer answer = Mockito.mock(CheckVolumeAnswer.class); when(answer.getResult()).thenReturn(true); + when(answer.getVolumeDetails()).thenReturn(Map.of(VolumeOnStorageTO.Detail.FILE_FORMAT, reportedFileFormat)); when(agentManager.easySend(anyLong(), any(CheckVolumeCommand.class))).thenReturn(answer); List storagePools = new ArrayList<>(); storagePools.add(storagePool); @@ -990,6 +1040,17 @@ private void importFromDisk(String source) throws InsufficientServerCapacityExce MockedConstruction mockCheckedReservation = Mockito.mockConstruction(CheckedReservation.class)) { unmanagedVMsManager.importVm(cmd); } + // the imported volume must record the format the agent reported for the image on the pool, + // not the hypervisor default, so an RBD image is stored as RAW + verify(volumeManager).updateImportedVolume(any(), any(), any(), any(), anyLong(), anyLong(), Mockito.eq(poolType), + Mockito.eq(diskPath), Mockito.isNull(), any(), Mockito.eq(expectedFormat)); + + // the plan must be confined to the pod and cluster of the pool the caller asked for, so that the + // volume check cannot land on a host in another cluster that has no access to that pool + ArgumentCaptor planCaptor = ArgumentCaptor.forClass(DeploymentPlan.class); + verify(deploymentPlanningManager).planDeployment(any(), planCaptor.capture(), any(), any()); + Assert.assertEquals(poolPodId, planCaptor.getValue().getPodId()); + Assert.assertEquals(poolClusterId, planCaptor.getValue().getClusterId()); } @Test