From 1d34b1ff148d9b191a7f1c58b3bdbeb01db51025 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Thu, 12 Nov 2026 12:13:18 +0100 Subject: [PATCH] kvm: resolve RBD disks of unmanaged instances without a mount lookup listUnmanagedInstances ran a substring mount lookup for every disk. For an RBD disk the pool name 'cloudstack' matched an unrelated NFS mount line, so the disk was reported on that NFS datastore and the import failed with 'Storage pool ... not found'. RBD disks now take the monitor host and the pool name from the libvirt source. Other disks, including Gluster network disks, keep the mount-based lookup. Add unit tests for the RBD and Gluster cases. --- ...rtGetUnmanagedInstancesCommandWrapper.java | 32 +++++++--- ...tUnmanagedInstancesCommandWrapperTest.java | 61 +++++++++++++++++++ 2 files changed, 83 insertions(+), 10 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapper.java index 60bc222594bb..a6e2187f9c8b 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapper.java @@ -191,7 +191,7 @@ private List getUnmanagedInstanceNics(LibvirtComputingR return nics; } - private List getUnmanagedInstanceDisks(List disksInfo, LibvirtComputingResource libvirtComputingResource, Connect conn, String domainName) { + protected List getUnmanagedInstanceDisks(List disksInfo, LibvirtComputingResource libvirtComputingResource, Connect conn, String domainName) { final ArrayList disks = new ArrayList<>(disksInfo.size()); int counter = 0; for (LibvirtVMDef.DiskDef diskDef : disksInfo) { @@ -215,18 +215,30 @@ private List getUnmanagedInstanceDisks(List sourceHostPath = getSourceHostPath(libvirtComputingResource, diskDef.getSourcePath()); - if (sourceHostPath != null) { - disk.setDatastoreHost(sourceHostPath.first()); - disk.setDatastorePath(sourceHostPath.second()); + if (diskDef.getDiskType() == LibvirtVMDef.DiskDef.DiskType.NETWORK + && diskDef.getDiskProtocol() == LibvirtVMDef.DiskDef.DiskProtocol.RBD) { + // RBD disks are not backed by a mounted filesystem, so a mount lookup would match an + // unrelated mount. Use the monitor host and the pool name from the source. Other network + // disks (Gluster) keep the mount lookup, as their pool path is the mounted volume. + String sourcePath = diskDef.getSourcePath(); + int poolEnd = sourcePath.indexOf("/"); + String poolName = poolEnd > 0 ? sourcePath.substring(0, poolEnd) : sourcePath; + disk.setDatastoreHost(diskDef.getSourceHost()); + disk.setDatastorePath(poolName); } else { - int pathEnd = diskDef.getSourcePath().lastIndexOf("/"); - if (pathEnd >= 0) { - disk.setDatastorePath(diskDef.getSourcePath().substring(0, pathEnd)); + Pair sourceHostPath = getSourceHostPath(libvirtComputingResource, diskDef.getSourcePath()); + if (sourceHostPath != null) { + disk.setDatastoreHost(sourceHostPath.first()); + disk.setDatastorePath(sourceHostPath.second()); } else { - disk.setDatastorePath(diskDef.getSourcePath()); + int pathEnd = diskDef.getSourcePath().lastIndexOf("/"); + if (pathEnd >= 0) { + disk.setDatastorePath(diskDef.getSourcePath().substring(0, pathEnd)); + } else { + disk.setDatastorePath(diskDef.getSourcePath()); + } + disk.setDatastoreHost(diskDef.getSourceHost()); } - disk.setDatastoreHost(diskDef.getSourceHost()); } disk.setDatastoreType(diskDef.getDiskType().toString()); diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapperTest.java index df40a3fe7d50..9a41deb4b49c 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapperTest.java @@ -16,14 +16,21 @@ // under the License. package com.cloud.hypervisor.kvm.resource.wrapper; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; import com.cloud.hypervisor.kvm.resource.LibvirtVMDef; +import com.cloud.utils.Pair; +import org.apache.cloudstack.vm.UnmanagedInstanceTO; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; +import org.libvirt.Connect; +import org.libvirt.Domain; +import org.libvirt.DomainBlockInfo; import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; +import java.util.List; import java.util.UUID; @RunWith(MockitoJUnitRunner.class) @@ -70,4 +77,58 @@ public void testGetDiskRelativePathFullPath() { Mockito.when(diskDef.getSourcePath()).thenReturn(imagePath); Assert.assertEquals(relativePath, wrapper.getDiskRelativePath(diskDef)); } + + @Test + public void testGlusterDiskStillUsesMountLookup() throws Exception { + LibvirtComputingResource libvirtComputingResource = Mockito.mock(LibvirtComputingResource.class); + Connect conn = Mockito.mock(Connect.class); + Domain domain = Mockito.mock(Domain.class); + String domainName = "i-2-40-VM"; + Mockito.when(conn.domainLookupByName(domainName)).thenReturn(domain); + Mockito.when(domain.blockInfo(Mockito.anyString())).thenReturn(Mockito.mock(DomainBlockInfo.class)); + Mockito.when(libvirtComputingResource.getSourceHostPath("vol1")) + .thenReturn(new Pair<>("10.0.0.5", "/vol1")); + + LibvirtVMDef.DiskDef glusterDisk = new LibvirtVMDef.DiskDef(); + glusterDisk.defNetworkBasedDisk("vol1/img.qcow2", "10.0.0.5", 24007, null, null, 0, + LibvirtVMDef.DiskDef.DiskBus.VIRTIO, LibvirtVMDef.DiskDef.DiskProtocol.GLUSTER, + LibvirtVMDef.DiskDef.DiskFmtType.QCOW2); + + List disks = wrapper.getUnmanagedInstanceDisks(List.of(glusterDisk), + libvirtComputingResource, conn, domainName); + + Assert.assertEquals(1, disks.size()); + Assert.assertEquals("10.0.0.5", disks.get(0).getDatastoreHost()); + Assert.assertEquals("/vol1", disks.get(0).getDatastorePath()); + Mockito.verify(libvirtComputingResource).getSourceHostPath("vol1"); + } + + @Test + public void testRbdDiskIsNotResolvedThroughMountLookup() throws Exception { + LibvirtComputingResource libvirtComputingResource = Mockito.mock(LibvirtComputingResource.class); + Connect conn = Mockito.mock(Connect.class); + Domain domain = Mockito.mock(Domain.class); + String domainName = "i-2-35-VM"; + Mockito.when(conn.domainLookupByName(domainName)).thenReturn(domain); + Mockito.when(domain.blockInfo(Mockito.anyString())).thenReturn(Mockito.mock(DomainBlockInfo.class)); + + LibvirtVMDef.DiskDef rbdDisk = new LibvirtVMDef.DiskDef(); + String imageName = "230e1224-f7b7-4dfc-97a7-3a0752636a31"; + rbdDisk.defNetworkBasedDisk("cloudstack/" + imageName, "192.168.55.201", 6789, null, null, 0, + LibvirtVMDef.DiskDef.DiskBus.VIRTIO, LibvirtVMDef.DiskDef.DiskProtocol.RBD, + LibvirtVMDef.DiskDef.DiskFmtType.RAW); + + List disks = wrapper.getUnmanagedInstanceDisks(List.of(rbdDisk), + libvirtComputingResource, conn, domainName); + + Assert.assertEquals(1, disks.size()); + UnmanagedInstanceTO.Disk disk = disks.get(0); + Assert.assertEquals("192.168.55.201", disk.getDatastoreHost()); + Assert.assertEquals("cloudstack", disk.getDatastorePath()); + Assert.assertEquals("cloudstack", disk.getDatastoreName()); + Assert.assertEquals("network", disk.getDatastoreType()); + Assert.assertEquals("cloudstack/" + imageName, disk.getImagePath()); + Assert.assertEquals(imageName, disk.getFileBaseName()); + Mockito.verify(libvirtComputingResource, Mockito.never()).getSourceHostPath(Mockito.anyString()); + } }