This is an automated email from the ASF dual-hosted git repository. weizhouapache pushed a commit to branch 4.23.0-ceph in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 22427c6b4115962a6915727a2259010797525931 Author: Abhisar Sinha <[email protected]> AuthorDate: Sat Sep 19 14:14:17 2026 +0530 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. --- .../api/command/admin/vm/ImportVmCmd.java | 2 +- .../api/command/admin/volume/ImportVolumeCmd.java | 2 +- .../service/VolumeOrchestrationService.java | 6 ++- .../engine/orchestration/VolumeOrchestrator.java | 14 +++-- .../orchestration/VolumeOrchestratorTest.java | 2 +- .../wrapper/LibvirtCheckVolumeCommandWrapper.java | 29 +++++++--- .../volume/VolumeImportUnmanageManagerImpl.java | 19 ++++++- .../cloudstack/vm/UnmanagedVMsManagerImpl.java | 56 +++++++++++++++---- .../VolumeImportUnmanageManagerImplTest.java | 11 +++- .../cloudstack/vm/UnmanagedVMsManagerImplTest.java | 63 +++++++++++++++++++++- ui/public/locales/en.json | 8 +-- ui/src/views/tools/ManageInstances.vue | 4 +- 12 files changed, 182 insertions(+), 34 deletions(-) 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 db7dcc3fb44..a32b8dd604e 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 50f4b9c1fbe..db5961ac7f8 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 141596407fe..8614197347a 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 @@ public interface VolumeOrchestrationService { */ 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 2fad96ec1da..4f163b9a1c9 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 @@ -2626,7 +2626,8 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati @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(); } @@ -2665,7 +2666,9 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati 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); @@ -2677,7 +2680,8 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati @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) { @@ -2709,7 +2713,9 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati 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 d61caf16ae5..8336c5ae5ef 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 @@ -246,7 +246,7 @@ public class VolumeOrchestratorTest { 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 6788516df74..3a42f230f84 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<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 final class LibvirtCheckVolumeCommandWrapper extends CommandWrapper<Check 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 @@ public final class LibvirtCheckVolumeCommandWrapper extends CommandWrapper<Check 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 abc2e5ca225..4a69e32e77e 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 @@ public class VolumeImportUnmanageManagerImpl implements VolumeImportUnmanageServ 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 { 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 840a1de4596..76aa6bd4c31 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -797,14 +797,16 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { } 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"); @@ -813,22 +815,44 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { 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) { @@ -843,7 +867,7 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { } 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); } @@ -2674,6 +2698,13 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { 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)) { @@ -2950,7 +2981,13 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { 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); @@ -2995,12 +3032,13 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { 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); 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 f3ed13c3d6b..08bcaeaced3 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 class VolumeImportUnmanageManagerImplTest { 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 class VolumeImportUnmanageManagerImplTest { } } + @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<HostVO, String> 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 dbc05ec99f3..3640c75f3b1 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.anyMap; 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.PrimaryDataStoreDao; 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.ClusterDao; 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 class UnmanagedVMsManagerImplTest { 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 @@ public class UnmanagedVMsManagerImplTest { Map<Volume, StoragePool> 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<StoragePoolVO> storagePools = new ArrayList<>(); storagePools.add(storagePool); @@ -990,6 +1040,17 @@ public class UnmanagedVMsManagerImplTest { MockedConstruction<CheckedReservation> 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<DeploymentPlan> 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 diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index f57460efa48..f5d330971ae 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -887,8 +887,8 @@ "label.desc.db.stats": "Database Statistics", "label.desc.importexportinstancewizard": "Import and export Instances to/from an existing VMware or KVM Cluster.", "label.desc.import.ext.kvm.wizard": "Import Instance from remote KVM host", -"label.desc.import.local.kvm.wizard": "Import QCOW2 image from Local Storage", -"label.desc.import.shared.kvm.wizard": "Import QCOW2 image from Shared Storage", +"label.desc.import.local.kvm.wizard": "Import disk image from Local Storage", +"label.desc.import.shared.kvm.wizard": "Import disk image from Shared Storage", "label.desc.import.unmanage.volume": "Import and unmanage volume on Storage Pools", "label.desc.ingesttinstancewizard": "Ingest instances from an external KVM host", "label.desc.importmigratefromvmwarewizard": "Import instances from VMware into a KVM Cluster", @@ -3527,8 +3527,8 @@ "message.desc.created.ssh.key.pair": "Created a SSH key pair.", "message.desc.host": "Each Cluster must contain at least one host (computer) for guest Instances to run on. We will add the first host now. For a host to function in CloudStack, you must install hypervisor software on the host, assign an IP address to the host, and ensure the host is connected to the CloudStack management server.<br/><br/>Give the host's DNS or IP address, the user name (usually root) and password, and any labels you use to categorize hosts.", "message.desc.import.ext.kvm.wizard": "Import libvirt domain from External KVM Host not managed by CloudStack", -"message.desc.import.local.kvm.wizard": "Import QCOW2 image from Local Storage of selected KVM Host", -"message.desc.import.shared.kvm.wizard": "Import QCOW2 image from selected Primary Storage Pool", +"message.desc.import.local.kvm.wizard": "Import disk image from Local Storage of selected KVM Host", +"message.desc.import.shared.kvm.wizard": "Import disk image from selected Primary Storage Pool", "message.desc.import.unmanage.volume": "Please choose a storage pool that you want to import or unmanage volumes. The storage pool should be in Up status. <br>This feature only supports KVM.", "message.desc.importexportinstancewizard": "By choosing to manage an Instance, CloudStack takes over the orchestration of that Instance. Unmanaging an Instance removes CloudStack ability to manage it. In both cases, the Instance is left running and no changes are done to the VM on the hypervisor.<br><br>For KVM, managing a VM is an experimental feature.", "message.desc.importingestinstancewizard": "This feature only applies to libvirt based KVM instances. Only Stopped instances can be ingested", diff --git a/ui/src/views/tools/ManageInstances.vue b/ui/src/views/tools/ManageInstances.vue index 6f625961ea8..28166c233c5 100644 --- a/ui/src/views/tools/ManageInstances.vue +++ b/ui/src/views/tools/ManageInstances.vue @@ -612,7 +612,7 @@ export default { }, { name: 'local', - label: 'Import QCOW2 image from Local Storage', + label: 'Import disk image from Local Storage', sourceDestHypervisors: { kvm: 'kvm' }, @@ -621,7 +621,7 @@ export default { }, { name: 'shared', - label: 'Import QCOW2 image from Shared Storage', + label: 'Import disk image from Shared Storage', sourceDestHypervisors: { kvm: 'kvm' },
