From f720a9a3e37f8b82ebc53d0f1141ad2b6bc4f7c9 Mon Sep 17 00:00:00 2001 From: andrijapanicsb Date: Sat, 6 Jun 2026 22:35:41 +0200 Subject: [PATCH 1/2] KVM: allow importVm to adopt an existing RBD (Ceph) root volume importVm with importsource=shared on KVM failed to adopt an existing ROOT volume located on an RBD (Ceph) primary storage pool, returning "Disk not found or is invalid" (jobresultcode 530). Two changes fix this: 1. LibvirtCheckVolumeCommandWrapper only whitelisted file-based pools (Filesystem, NetworkFilesystem, SharedMountPoint) and inspected the volume with direct file reads (checkQcow2File / getVirtualSizeFromFile), which do not work on RBD. RBD is now supported: the volume is inspected through the RBD URI via qemu-img (the same approach already used by LibvirtGetVolumesOnStorageCommandWrapper, which backs listVolumesForImport), the QCOW2 check is skipped for raw RBD images, the virtual size is taken from the pool-reported disk, and the encrypted/backing-file/locked details are still collected so the management server import validations keep working. 2. VolumeOrchestrator.importVolume() and updateImportedVolume() hardcoded the KVM volume format to QCOW2. RBD-backed volumes are raw, so the format is now derived from the storage pool type (RBD -> RAW) through a single shared helper, applied at both import call sites. Unit tests: new LibvirtCheckVolumeCommandWrapperTest and additional VolumeOrchestratorTest cases for the format helper. --- .../orchestration/VolumeOrchestrator.java | 17 +- .../orchestration/VolumeOrchestratorTest.java | 22 ++ .../LibvirtCheckVolumeCommandWrapper.java | 25 ++- .../LibvirtCheckVolumeCommandWrapperTest.java | 189 ++++++++++++++++++ 4 files changed, 250 insertions(+), 3 deletions(-) create mode 100644 plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVolumeCommandWrapperTest.java 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..eeb67108bbd5 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 @@ -1165,6 +1165,19 @@ private ImageFormat getSupportedImageFormatForCluster(HypervisorType hyperType) } } + /** + * Returns the image format for a volume taking the underlying storage pool type into account. + * On KVM the default cluster format is QCOW2, but block-based pools such as RBD (Ceph) store + * volumes as RAW images. This is relevant when importing/adopting an existing volume so the + * recorded format matches what is actually on the pool. + */ + protected ImageFormat getSupportedImageFormatForCluster(HypervisorType hyperType, Storage.StoragePoolType poolType) { + if (hyperType == HypervisorType.KVM && poolType == Storage.StoragePoolType.RBD) { + return ImageFormat.RAW; + } + return getSupportedImageFormatForCluster(hyperType); + } + private boolean isSupportedImageFormatForCluster(VolumeInfo volume, HypervisorType rootDiskHyperType) { ImageFormat volumeFormat = volume.getFormat(); if (rootDiskHyperType == HypervisorType.Hyperv) { @@ -2367,7 +2380,7 @@ public DiskProfile importVolume(Type type, String name, DiskOffering offering, L vol.setDisplayVolume(userVm.isDisplayVm()); } - vol.setFormat(getSupportedImageFormatForCluster(hypervisorType)); + vol.setFormat(getSupportedImageFormatForCluster(hypervisorType, poolType)); vol.setPoolId(poolId); vol.setPoolType(poolType); vol.setPath(path); @@ -2411,7 +2424,7 @@ public DiskProfile updateImportedVolume(Type type, DiskOffering offering, Virtua vol.setDisplayVolume(userVm.isDisplayVm()); } - vol.setFormat(getSupportedImageFormatForCluster(vm.getHypervisorType())); + vol.setFormat(getSupportedImageFormatForCluster(vm.getHypervisorType(), poolType)); 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..8fe51f116359 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 @@ -173,6 +173,28 @@ public void testCheckAndUpdateVolumeAccountResourceCountLessSize() { runCheckAndUpdateVolumeAccountResourceCountTest(20L, 10L); } + @Test + public void testGetSupportedImageFormatForClusterKvmRbdIsRaw() { + Assert.assertEquals(Storage.ImageFormat.RAW, + volumeOrchestrator.getSupportedImageFormatForCluster(Hypervisor.HypervisorType.KVM, Storage.StoragePoolType.RBD)); + } + + @Test + public void testGetSupportedImageFormatForClusterKvmNonRbdIsQcow2() { + Assert.assertEquals(Storage.ImageFormat.QCOW2, + volumeOrchestrator.getSupportedImageFormatForCluster(Hypervisor.HypervisorType.KVM, Storage.StoragePoolType.NetworkFilesystem)); + Assert.assertEquals(Storage.ImageFormat.QCOW2, + volumeOrchestrator.getSupportedImageFormatForCluster(Hypervisor.HypervisorType.KVM, Storage.StoragePoolType.Filesystem)); + } + + @Test + public void testGetSupportedImageFormatForClusterNonKvmIgnoresPoolType() { + Assert.assertEquals(Storage.ImageFormat.VHD, + volumeOrchestrator.getSupportedImageFormatForCluster(Hypervisor.HypervisorType.XenServer, Storage.StoragePoolType.RBD)); + Assert.assertEquals(Storage.ImageFormat.OVA, + volumeOrchestrator.getSupportedImageFormatForCluster(Hypervisor.HypervisorType.VMware, Storage.StoragePoolType.RBD)); + } + @Test public void testGrantVolumeAccessToHostIfNeededDriverNoNeed() { PrimaryDataStore store = Mockito.mock(PrimaryDataStore.class); 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..62248c1d9c9b 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) { @@ -63,6 +64,9 @@ public Answer execute(final CheckVolumeCommand command, final LibvirtComputingRe try { if (STORAGE_POOL_TYPES_SUPPORTED.contains(storageFilerTO.getType())) { final KVMPhysicalDisk vol = pool.getPhysicalDisk(srcFile); + if (Storage.StoragePoolType.RBD.equals(storageFilerTO.getType())) { + return checkRbdVolume(command, pool, vol); + } final String path = vol.getPath(); try { KVMPhysicalDisk.checkQcow2File(path); @@ -81,6 +85,21 @@ public Answer execute(final CheckVolumeCommand command, final LibvirtComputingRe } } + /** + * RBD (Ceph) volumes are raw images that cannot be inspected through direct file reads + * (checkQcow2File / getVirtualSizeFromFile operate on a local path). Instead the volume is + * inspected through the RBD URI via qemu-img (see {@link #getDiskFileInfo}). The virtual size + * is taken from the disk reported by the storage pool, and the encrypted/backing-file/locked + * details are still collected so the management server can apply its import validations. + */ + private Answer checkRbdVolume(final CheckVolumeCommand command, final KVMStoragePool pool, final KVMPhysicalDisk disk) { + Map volumeDetails = getVolumeDetails(pool, disk); + if (volumeDetails == null) { + return new CheckVolumeAnswer(command, false, "", 0, null); + } + return new CheckVolumeAnswer(command, true, "", disk.getVirtualSize(), volumeDetails); + } + private Map getVolumeDetails(KVMStoragePool pool, KVMPhysicalDisk disk) { Map info = getDiskFileInfo(pool, disk, true); if (MapUtils.isEmpty(info)) { @@ -122,6 +141,10 @@ 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())) { + String rbdDestFile = KVMPhysicalDisk.RBDStringBuilder(pool, disk.getPath()); + qemuFile = new QemuImgFile(rbdDestFile, 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/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVolumeCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVolumeCommandWrapperTest.java new file mode 100644 index 000000000000..7d5a775452be --- /dev/null +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVolumeCommandWrapperTest.java @@ -0,0 +1,189 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package com.cloud.hypervisor.kvm.resource.wrapper; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.CheckVolumeAnswer; +import com.cloud.agent.api.CheckVolumeCommand; +import com.cloud.agent.api.to.StorageFilerTO; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.hypervisor.kvm.storage.KVMPhysicalDisk; +import com.cloud.hypervisor.kvm.storage.KVMStoragePool; +import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager; +import com.cloud.storage.Storage; +import org.apache.cloudstack.storage.volume.VolumeOnStorageTO; +import org.apache.cloudstack.utils.qemu.QemuImg; +import org.apache.cloudstack.utils.qemu.QemuImgFile; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.MockedConstruction; +import org.mockito.Mockito; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; + +import java.util.HashMap; +import java.util.Map; + +@RunWith(MockitoJUnitRunner.class) +public class LibvirtCheckVolumeCommandWrapperTest { + + @Mock + LibvirtComputingResource libvirtComputingResource; + @Mock + KVMStoragePoolManager storagePoolMgr; + @Mock + KVMStoragePool storagePool; + @Mock + StorageFilerTO storageFilerTO; + @Mock + KVMPhysicalDisk disk; + + @Spy + LibvirtCheckVolumeCommandWrapper wrapper = new LibvirtCheckVolumeCommandWrapper(); + + MockedConstruction qemuImg; + + private final String poolUuid = "pool-uuid"; + private final String srcFile = "rbd-volume-uuid"; + private final long virtualSize = 21474836480L; // 20 GiB + + private Map qemuInfo; + + @Before + public void setUp() { + qemuInfo = new HashMap<>(); + qemuInfo.put(QemuImg.FILE_FORMAT, "raw"); + Mockito.when(libvirtComputingResource.getStoragePoolMgr()).thenReturn(storagePoolMgr); + } + + @After + public void tearDown() { + if (qemuImg != null) { + qemuImg.close(); + } + } + + private void mockRbdPool() { + Mockito.when(storageFilerTO.getType()).thenReturn(Storage.StoragePoolType.RBD); + Mockito.when(storageFilerTO.getUuid()).thenReturn(poolUuid); + Mockito.when(storagePoolMgr.getStoragePool(Storage.StoragePoolType.RBD, poolUuid)).thenReturn(storagePool); + Mockito.when(storagePool.getType()).thenReturn(Storage.StoragePoolType.RBD); + Mockito.when(storagePool.getPhysicalDisk(srcFile)).thenReturn(disk); + Mockito.when(disk.getPath()).thenReturn(srcFile); + Mockito.when(disk.getFormat()).thenReturn(QemuImg.PhysicalDiskFormat.RAW); + // values consumed by KVMPhysicalDisk.RBDStringBuilder when building the RBD URI + Mockito.when(storagePool.getSourceHost()).thenReturn("10.0.0.10"); + Mockito.when(storagePool.getSourcePort()).thenReturn(6789); + Mockito.when(storagePool.getAuthUserName()).thenReturn(null); + Mockito.when(storagePool.getDetails()).thenReturn(null); + } + + private CheckVolumeCommand buildCommand() { + CheckVolumeCommand command = new CheckVolumeCommand(); + command.setSrcFile(srcFile); + command.setStorageFilerTO(storageFilerTO); + return command; + } + + @Test + public void testRbdVolumeReturnsSuccessWithVirtualSizeAndDetails() { + mockRbdPool(); + Mockito.when(disk.getVirtualSize()).thenReturn(virtualSize); + qemuImg = Mockito.mockConstruction(QemuImg.class, (mock, context) -> + Mockito.when(mock.info(Mockito.any(QemuImgFile.class), Mockito.anyBoolean())).thenReturn(qemuInfo)); + + Answer answer = wrapper.execute(buildCommand(), libvirtComputingResource); + + Assert.assertTrue(answer instanceof CheckVolumeAnswer); + Assert.assertTrue(answer.getResult()); + CheckVolumeAnswer checkVolumeAnswer = (CheckVolumeAnswer) answer; + Assert.assertEquals(virtualSize, checkVolumeAnswer.getSize()); + Map details = checkVolumeAnswer.getVolumeDetails(); + Assert.assertNotNull(details); + Assert.assertEquals("raw", details.get(VolumeOnStorageTO.Detail.FILE_FORMAT)); + Assert.assertEquals("false", details.get(VolumeOnStorageTO.Detail.IS_LOCKED)); + } + + @Test + public void testRbdVolumeInspectedThroughRbdUri() throws Exception { + mockRbdPool(); + Mockito.when(disk.getVirtualSize()).thenReturn(virtualSize); + qemuImg = Mockito.mockConstruction(QemuImg.class, (mock, context) -> + Mockito.when(mock.info(Mockito.any(QemuImgFile.class), Mockito.anyBoolean())).thenReturn(qemuInfo)); + + wrapper.execute(buildCommand(), libvirtComputingResource); + + ArgumentCaptor fileCaptor = ArgumentCaptor.forClass(QemuImgFile.class); + Mockito.verify(qemuImg.constructed().get(0), Mockito.atLeastOnce()) + .info(fileCaptor.capture(), Mockito.anyBoolean()); + Assert.assertTrue("qemu-img should be pointed at the RBD URI", + fileCaptor.getValue().getFileName().startsWith("rbd:" + srcFile)); + } + + @Test + public void testRbdVolumeReportsLockedWhenInfoProbeFails() { + mockRbdPool(); + Mockito.when(disk.getVirtualSize()).thenReturn(virtualSize); + qemuImg = Mockito.mockConstruction(QemuImg.class, (mock, context) -> { + // secure=true uses force-share and succeeds; secure=false is the lock probe + Mockito.when(mock.info(Mockito.any(QemuImgFile.class), Mockito.eq(true))).thenReturn(qemuInfo); + Mockito.when(mock.info(Mockito.any(QemuImgFile.class), Mockito.eq(false))).thenReturn(null); + }); + + Answer answer = wrapper.execute(buildCommand(), libvirtComputingResource); + + Assert.assertTrue(answer instanceof CheckVolumeAnswer); + Assert.assertTrue(answer.getResult()); + Map details = ((CheckVolumeAnswer) answer).getVolumeDetails(); + Assert.assertEquals("true", details.get(VolumeOnStorageTO.Detail.IS_LOCKED)); + } + + @Test + public void testRbdVolumeWithoutInfoReturnsInvalid() { + mockRbdPool(); + qemuImg = Mockito.mockConstruction(QemuImg.class, (mock, context) -> + Mockito.when(mock.info(Mockito.any(QemuImgFile.class), Mockito.anyBoolean())).thenReturn(null)); + + Answer answer = wrapper.execute(buildCommand(), libvirtComputingResource); + + Assert.assertTrue(answer instanceof CheckVolumeAnswer); + Assert.assertFalse(answer.getResult()); + CheckVolumeAnswer checkVolumeAnswer = (CheckVolumeAnswer) answer; + Assert.assertEquals(0, checkVolumeAnswer.getSize()); + Assert.assertNull(checkVolumeAnswer.getVolumeDetails()); + } + + @Test + public void testUnsupportedStoragePoolReturnsPlainAnswer() { + Mockito.when(storageFilerTO.getType()).thenReturn(Storage.StoragePoolType.PowerFlex); + Mockito.when(storageFilerTO.getUuid()).thenReturn(poolUuid); + + Answer answer = wrapper.execute(buildCommand(), libvirtComputingResource); + + Assert.assertFalse(answer instanceof CheckVolumeAnswer); + Assert.assertFalse(answer.getResult()); + Assert.assertEquals("Unsupported Storage Pool", answer.getDetails()); + } +} From 8827182476963e22b105a784c8c6b9b760b0bfb2 Mon Sep 17 00:00:00 2001 From: andrijapanicsb Date: Sun, 7 Jun 2026 01:06:52 +0200 Subject: [PATCH 2/2] KVM: add unit test for RBD format on the import/manage-volume flow importVolume (managing a previously-unmanaged volume) routes through VolumeOrchestrator.importVolume() and therefore also benefits from the pool-type-aware format fix (RBD -> RAW). Add a VolumeImportUnmanageManagerImplTest case asserting the RBD pool type and KVM hypervisor are forwarded to the orchestrator. --- .../VolumeImportUnmanageManagerImplTest.java | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) 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..a45ebeb68a0f 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 @@ -290,6 +290,55 @@ public void testImportVolumeAllGood() throws ResourceAllocationException { } } + @Test + public void testImportVolumeForwardsRbdPoolTypeToOrchestrator() throws ResourceAllocationException { + ImportVolumeCmd cmd = mock(ImportVolumeCmd.class); + when(cmd.getPath()).thenReturn(path); + when(cmd.getStorageId()).thenReturn(poolId); + when(cmd.getDiskOfferingId()).thenReturn(diskOfferingId); + when(volumeDao.findByPoolIdAndPath(poolId, path)).thenReturn(null); + when(templatePoolDao.findByPoolPath(poolId, path)).thenReturn(null); + + // Import a KVM volume residing on an RBD (Ceph) pool + when(storagePoolVO.getPoolType()).thenReturn(Storage.StoragePoolType.RBD); + + VolumeOnStorageTO volumeOnStorageTO = new VolumeOnStorageTO(Hypervisor.HypervisorType.KVM, path, name, fullPath, + "raw", size, virtualSize); + List volumesOnStorageTO = new ArrayList<>(); + volumesOnStorageTO.add(volumeOnStorageTO); + doReturn(volumesOnStorageTO).when(volumeImportUnmanageManager).listVolumesForImportInternal(storagePoolVO, path, null); + + doNothing().when(volumeImportUnmanageManager).checkIfVolumeIsLocked(volumeOnStorageTO); + doNothing().when(volumeImportUnmanageManager).checkIfVolumeIsEncrypted(volumeOnStorageTO); + doNothing().when(volumeImportUnmanageManager).checkIfVolumeHasBackingFile(volumeOnStorageTO); + + DiskOfferingVO diskOffering = mock(DiskOfferingVO.class); + when(diskOffering.isCustomized()).thenReturn(true); + doReturn(diskOffering).when(volumeImportUnmanageManager).getOrCreateDiskOffering(account, diskOfferingId, zoneId, isLocal); + 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()); + when(diskProfile.getVolumeId()).thenReturn(volumeId); + when(volumeDao.findById(volumeId)).thenReturn(volumeVO); + + doNothing().when(resourceLimitService).incrementResourceCount(accountId, Resource.ResourceType.volume); + doNothing().when(resourceLimitService).incrementResourceCount(accountId, Resource.ResourceType.primary_storage, virtualSize); + + VolumeResponse response = mock(VolumeResponse.class); + doReturn(response).when(responseGenerator).createVolumeResponse(ResponseObject.ResponseView.Full, volumeVO); + try (MockedStatic ignored = Mockito.mockStatic(UsageEventUtils.class); + MockedStatic ignoredtoo = Mockito.mockStatic(ActionEventUtils.class)) { + volumeImportUnmanageManager.importVolume(cmd); + } + + // The RBD pool type and KVM hypervisor are forwarded to the orchestrator, which is what makes the + // imported volume be recorded with RAW format (see VolumeOrchestrator.getSupportedImageFormatForCluster). + verify(volumeManager).importVolume(eq(Volume.Type.DATADISK), anyString(), any(), eq(virtualSize), isNull(), isNull(), + anyLong(), eq(Hypervisor.HypervisorType.KVM), isNull(), isNull(), any(), isNull(), anyLong(), + eq(Storage.StoragePoolType.RBD), anyString(), isNull()); + } + @Test public void testListVolumesForImportInternal() { Pair hostAndLocalPath = mock(Pair.class);