From cf319a098e398f0a26836b569d793d1e6c31bc58 Mon Sep 17 00:00:00 2001 From: Brad House Date: Mon, 9 Nov 2026 06:26:27 +0000 Subject: [PATCH 1/2] kvm: refresh the RBD pool when resizing a volume libvirt has not cached LibvirtResizeVolumeCommandWrapper gets the disk with pool.getPhysicalDisk() and then looks the volume up through libvirt with storageVolLookupByPath() to resize it. Before #14188, getPhysicalDisk() went through LibvirtStorageAdaptor.getVolume(), which refreshes the libvirt pool when the volume is not found, so the volume was in libvirt's cache by the time the wrapper looked it up. #14188 made getPhysicalDisk() stat RBD images through librbd, so nothing refreshes the pool any more. RBD volumes are created through librbd, not libvirt, so a new volume is not in libvirt's cache on a host until something refreshes the pool there, and resizing it fails with: Storage volume not found: no storage vol with matching path '/' When the lookup by path fails with VIR_ERR_NO_STORAGE_VOL on an RBD pool, refresh the pool and look the volume up once more. That is what getVolume() did for this path before #14188, and the refresh only happens when the volume is not cached. Other errors and other pool types are handled as before. Signed-off-by: Brad House --- .../LibvirtResizeVolumeCommandWrapper.java | 30 +++- .../LibvirtComputingResourceTest.java | 158 ++++++++++++++++++ 2 files changed, 187 insertions(+), 1 deletion(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtResizeVolumeCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtResizeVolumeCommandWrapper.java index f2af46d4cc8a..04cf35c205b7 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtResizeVolumeCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtResizeVolumeCommandWrapper.java @@ -36,6 +36,7 @@ import org.libvirt.Connect; import org.libvirt.Domain; import org.libvirt.DomainInfo; +import org.libvirt.Error.ErrorNumber; import org.libvirt.LibvirtException; import org.libvirt.StorageVol; @@ -120,7 +121,7 @@ public Answer execute(final ResizeVolumeCommand command, final LibvirtComputingR final LibvirtUtilitiesHelper libvirtUtilitiesHelper = libvirtComputingResource.getLibvirtUtilitiesHelper(); final Connect conn = libvirtUtilitiesHelper.getConnection(); - final StorageVol v = conn.storageVolLookupByPath(path); + final StorageVol v = lookupVolumeByPath(conn, pool, path); int flags = 0; if (conn.getLibVirVersion() > 1001000 && vol.getFormat() == PhysicalDiskFormat.RAW && pool.getType() != StoragePoolType.RBD) { @@ -181,6 +182,33 @@ public Answer execute(final ResizeVolumeCommand command, final LibvirtComputingR } } + /** + * Looks a volume up through libvirt by its path. + * + * An RBD volume is created through librbd, and KVMStoragePool.getPhysicalDisk() looks RBD + * volumes up through librbd too, so nothing may have made libvirt refresh the pool since the + * volume was created, and the volume may be missing from libvirt's cache of the pool on this + * host. For an RBD pool, a lookup that fails because the volume is not found refreshes the + * pool and looks the volume up once more. Any other failure, and any other pool type, is + * returned to the caller as before. + */ + private StorageVol lookupVolumeByPath(final Connect conn, final KVMStoragePool pool, final String path) throws LibvirtException { + try { + return conn.storageVolLookupByPath(path); + } catch (final LibvirtException e) { + if (pool.getType() != StoragePoolType.RBD || e.getError() == null + || e.getError().getCode() != ErrorNumber.VIR_ERR_NO_STORAGE_VOL) { + throw e; + } + logger.info("Volume " + path + " is not in libvirt's cache of RBD pool " + pool.getUuid() + " (" + e.getMessage() + + "). Refreshing the pool and looking the volume up again."); + if (!pool.refresh()) { + logger.warn("Failed to refresh RBD pool " + pool.getUuid() + ". Looking volume " + path + " up again anyway."); + } + return conn.storageVolLookupByPath(path); + } + } + private boolean isVmRunning(final String vmName, final LibvirtComputingResource libvirtComputingResource) { try { final LibvirtUtilitiesHelper libvirtUtilitiesHelper = libvirtComputingResource.getLibvirtUtilitiesHelper(); diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java index fa204f806261..1b5e1b5d8da2 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java @@ -92,6 +92,7 @@ import org.libvirt.DomainInfo; import org.libvirt.DomainInfo.DomainState; import org.libvirt.DomainInterfaceStats; +import org.libvirt.Error.ErrorNumber; import org.libvirt.LibvirtException; import org.libvirt.MemoryStatistic; import org.libvirt.NodeInfo; @@ -101,6 +102,7 @@ import org.libvirt.jna.virDomainMemoryStats; import org.mockito.ArgumentCaptor; import org.mockito.BDDMockito; +import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.MockedConstruction; import org.mockito.MockedStatic; @@ -175,6 +177,7 @@ import com.cloud.agent.api.storage.CreateCommand; import com.cloud.agent.api.storage.DestroyCommand; import com.cloud.agent.api.storage.PrimaryStorageDownloadCommand; +import com.cloud.agent.api.storage.ResizeVolumeAnswer; import com.cloud.agent.api.storage.ResizeVolumeCommand; import com.cloud.agent.api.to.DataStoreTO; import com.cloud.agent.api.to.DiskTO; @@ -4830,6 +4833,161 @@ public void testResizeVolumeCommand() { } } + private static final String RBD_RESIZE_PATH = "cloudstack/0d3c2bd6-cb39-4c8b-a1d9-0e7fbd3b8a5b"; + + private static LibvirtException libvirtException(final ErrorNumber code) { + final org.libvirt.Error error = Mockito.mock(org.libvirt.Error.class); + when(error.getCode()).thenReturn(code); + final LibvirtException e = Mockito.mock(LibvirtException.class); + when(e.getError()).thenReturn(error); + return e; + } + + /** + * Sets up a resize of a RAW volume at {@code path} on a pool of type {@code poolType}, up to + * the point where the wrapper looks the volume up through {@code conn}. + */ + private ResizeVolumeCommand prepareResizeLookup(final StoragePoolType poolType, final String path, final KVMStoragePool storagePool, + final KVMPhysicalDisk vol, final Connect conn) throws LibvirtException { + final StorageFilerTO pool = Mockito.mock(StorageFilerTO.class); + final KVMStoragePoolManager storagePoolMgr = Mockito.mock(KVMStoragePoolManager.class); + final LibvirtUtilitiesHelper libvirtUtilitiesHelper = Mockito.mock(LibvirtUtilitiesHelper.class); + + when(pool.getType()).thenReturn(poolType); + when(libvirtComputingResourceMock.getStoragePoolMgr()).thenReturn(storagePoolMgr); + when(storagePoolMgr.getStoragePool(pool.getType(), pool.getUuid())).thenReturn(storagePool); + when(storagePool.getPhysicalDisk(path)).thenReturn(vol); + when(storagePool.getType()).thenReturn(poolType); + when(vol.getPath()).thenReturn(path); + when(vol.getFormat()).thenReturn(PhysicalDiskFormat.RAW); + when(libvirtComputingResourceMock.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getConnection()).thenReturn(conn); + // Only reached when the resize gets as far as telling the VM about the new size. + Mockito.lenient().when(libvirtUtilitiesHelper.getConnectionByVmName("Test")).thenReturn(conn); + + return new ResizeVolumeCommand(path, pool, 100L, 200L, false, "Test", null); + } + + /** + * An RBD volume created through librbd is not in libvirt's cache of the pool until the pool is + * refreshed, so the first lookup by path misses. The resize must refresh the pool, find the + * volume and resize it, then tell the running VM about the new size. + */ + @Test + public void testResizeVolumeCommandRbdVolumeNotInLibvirtPoolCache() throws LibvirtException { + final KVMStoragePool storagePool = Mockito.mock(KVMStoragePool.class); + final KVMPhysicalDisk vol = Mockito.mock(KVMPhysicalDisk.class); + final Connect conn = Mockito.mock(Connect.class); + final StorageVol v = Mockito.mock(StorageVol.class); + final Domain vm = Mockito.mock(Domain.class); + final DomainInfo info = Mockito.mock(DomainInfo.class); + info.state = DomainInfo.DomainState.VIR_DOMAIN_RUNNING; + + final ResizeVolumeCommand command = prepareResizeLookup(StoragePoolType.RBD, RBD_RESIZE_PATH, storagePool, vol, conn); + final LibvirtException notFound = libvirtException(ErrorNumber.VIR_ERR_NO_STORAGE_VOL); + when(conn.storageVolLookupByPath(RBD_RESIZE_PATH)).thenThrow(notFound).thenReturn(v); + when(storagePool.refresh()).thenReturn(true); + when(conn.getLibVirVersion()).thenReturn(10010L); + when(conn.domainLookupByName("Test")).thenReturn(vm); + when(vm.getInfo()).thenReturn(info); + when(vol.getVirtualSize()).thenReturn(200L); + + // The resize script succeeds (a mocked Script.execute() returns null), so the answer comes from the size read back after the resize. + try (MockedConstruction