From b40abdebb8387297a9c0bd01cf59dda1386e2c6d Mon Sep 17 00:00:00 2001 From: Abhisar Sinha <63767682+abh1sar@users.noreply.github.com> Date: Mon, 14 Sep 2026 09:01:56 +0530 Subject: [PATCH 1/5] 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. --- .../api/command/admin/vm/ImportVmCmd.java | 2 +- .../command/admin/volume/ImportVolumeCmd.java | 2 +- .../LibvirtCheckVolumeCommandWrapper.java | 22 ++++++++++++++----- 3 files changed, 18 insertions(+), 8 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 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/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..6a861aed1799 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,13 +65,19 @@ 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()); + + if (!isRbd) { + try { + KVMPhysicalDisk.checkQcow2File(path); + } catch (final CloudRuntimeException e) { + return new CheckVolumeAnswer(command, false, "", 0, getVolumeDetails(pool, vol)); + } } - long size = KVMPhysicalDisk.getVirtualSizeFromFile(path); + // 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, getVolumeDetails(pool, vol)); } else { return new Answer(command, false, "Unsupported Storage Pool"); @@ -122,6 +129,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()); From c2ca38bdc1ab97a21f9464d700f7b69cd957e902 Mon Sep 17 00:00:00 2001 From: Abhisar Sinha <63767682+abh1sar@users.noreply.github.com> Date: Mon, 14 Sep 2026 09:03:40 +0530 Subject: [PATCH 2/5] 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. --- .../service/VolumeOrchestrationService.java | 6 ++- .../orchestration/VolumeOrchestrator.java | 14 +++++-- .../orchestration/VolumeOrchestratorTest.java | 2 +- .../VolumeImportUnmanageManagerImpl.java | 19 ++++++++- .../vm/UnmanagedVMsManagerImpl.java | 41 +++++++++++++++---- .../VolumeImportUnmanageManagerImplTest.java | 11 ++++- .../vm/UnmanagedVMsManagerImplTest.java | 35 +++++++++++++++- 7 files changed, 110 insertions(+), 18 deletions(-) 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/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..81b81864059a 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); } @@ -2968,12 +2992,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..55a0b3eac7d8 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,6 +65,7 @@ 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; @@ -952,13 +954,38 @@ 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 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 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 { 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 +999,14 @@ 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); 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 +1019,10 @@ 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)); } @Test From e8fa17e49f9e9db9d4782940589e19774544fd54 Mon Sep 17 00:00:00 2001 From: Abhisar Sinha <63767682+abh1sar@users.noreply.github.com> Date: Mon, 14 Sep 2026 10:45:10 +0530 Subject: [PATCH 3/5] 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 --- .../vm/UnmanagedVMsManagerImpl.java | 15 +++++++++- .../vm/UnmanagedVMsManagerImplTest.java | 28 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) 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 81b81864059a..585f00d80eb3 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -2671,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)) { @@ -2947,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); 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 55a0b3eac7d8..30bc769d5d4f 100644 --- a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java @@ -72,6 +72,7 @@ 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; @@ -105,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; @@ -959,6 +961,14 @@ public void importFromsharedStorageOnRbdPool() throws InsufficientServerCapacity 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)); @@ -973,6 +983,9 @@ public void testGetImageFormatFromVolumeDetails() { 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); @@ -980,6 +993,12 @@ private void importFromDisk(String source) throws InsufficientServerCapacityExce 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()); @@ -1000,6 +1019,8 @@ private void importFromDisk(String source, Storage.StoragePoolType poolType, Str 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); @@ -1023,6 +1044,13 @@ private void importFromDisk(String source, Storage.StoragePoolType poolType, Str // 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 From 8f49d1a14a967d30f1a63a43c45690debf22c217 Mon Sep 17 00:00:00 2001 From: Abhisar Sinha <63767682+abh1sar@users.noreply.github.com> Date: Mon, 14 Sep 2026 15:52:00 +0530 Subject: [PATCH 4/5] verify qemu is able to read the rbd image since check can not be done on a raw file --- .../resource/wrapper/LibvirtCheckVolumeCommandWrapper.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 6a861aed1799..659f6b9071a3 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 @@ -67,11 +67,16 @@ public Answer execute(final CheckVolumeCommand command, final LibvirtComputingRe final String path = vol.getPath(); 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, getVolumeDetails(pool, vol)); + return new CheckVolumeAnswer(command, false, "", 0, volumeDetails); } } From 0b898ad1ba2f2cf89f914a3801aa4aa716164a9f Mon Sep 17 00:00:00 2001 From: Abhisar Sinha <63767682+abh1sar@users.noreply.github.com> Date: Tue, 15 Sep 2026 13:16:05 +0530 Subject: [PATCH 5/5] use computed volumeDetails in success path as well. --- .../kvm/resource/wrapper/LibvirtCheckVolumeCommandWrapper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 659f6b9071a3..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 @@ -83,7 +83,7 @@ public Answer execute(final CheckVolumeCommand command, final LibvirtComputingRe // 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, getVolumeDetails(pool, vol)); + return new CheckVolumeAnswer(command, true, "", size, volumeDetails); } else { return new Answer(command, false, "Unsupported Storage Pool"); }