From d74bf10cb90356239aed3f772f3d1d489699dfd4 Mon Sep 17 00:00:00 2001 From: Brad House Date: Thu, 17 Sep 2026 03:16:05 +0000 Subject: [PATCH] kvm: look RBD volumes up through librbd instead of refreshing the pool getPhysicalDisk() asks libvirt for the volume. A volume that was just created by the management server is not in this host's libvirt pool cache, so the lookup misses and getVolume() falls back to refreshing the whole pool. Refreshing an RBD pool opens and stats every image in it, so the cost grows with the number of volumes in the pool and is paid on every VM start. On a pool holding 950 images that is 11.7 seconds added to each start, against 0.005 seconds to list the image names. Look RBD volumes up directly through librbd instead. Creating, cloning, resizing, copying and deleting RBD volumes in this class already use librbd directly; only the lookup went through libvirt. Because this runs on every VM start, the connection is handled more carefully than at the existing call sites: - the rados connection is shut down in a finally, so a client is not left for the finalizer. KVMStoragePoolManager.getPhysicalDisk retries a missing volume 100 times, so leaking here would mean 101 live librados clients for one absent volume. - the image is opened read only, since it is only stat'ed, and so cannot take the exclusive lock. - the image is closed through a helper that does not throw, so a failed close cannot discard a successful lookup or hide the original error. - the cephx key is only set when the pool has a user. librados aborts the process if it is handed a null value. Signed-off-by: Brad House --- .../kvm/storage/LibvirtStorageAdaptor.java | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index 059f4f8b67af..c949d0882479 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -659,6 +659,19 @@ public KVMStoragePool getStoragePool(String uuid, boolean refreshInfo) { public KVMPhysicalDisk getPhysicalDisk(String volumeUuid, KVMStoragePool pool) { LibvirtStoragePool libvirtPool = (LibvirtStoragePool)pool; + /* + * An RBD volume is looked up through librbd rather than through libvirt. + * A volume that was created by another host is not in the libvirt pool + * cache, so looking it up through libvirt misses and forces a refresh of + * the whole pool. Refreshing an RBD pool stats every image in it, so that + * cost grows with the number of volumes in the pool and is paid on every + * VM start. Every other RBD operation in this class already uses librbd + * directly. + */ + if (pool.getType() == StoragePoolType.RBD) { + return getRbdPhysicalDisk(volumeUuid, libvirtPool); + } + try { StorageVol vol = getVolume(libvirtPool.getPool(), volumeUuid); KVMPhysicalDisk disk; @@ -696,6 +709,74 @@ public KVMPhysicalDisk getPhysicalDisk(String volumeUuid, KVMStoragePool pool) { } } + /** + * Looks an RBD volume up directly through librbd. + * + * The size reported by librbd is used for both the size and the virtual size of + * the disk, matching what this class already does after converting an image into + * an RBD volume. + */ + private KVMPhysicalDisk getRbdPhysicalDisk(String volumeUuid, LibvirtStoragePool pool) { + Rados r = new Rados(pool.getAuthUserName()); + try { + r.confSet("mon_host", pool.getSourceHost() + ":" + pool.getSourcePort()); + /* + * The secret is null when the pool has no cephx user, and librados + * aborts the process rather than returning an error if it is handed a + * null value here. + */ + if (pool.getAuthUserName() != null) { + r.confSet("key", pool.getAuthSecret()); + } else { + r.confSet("auth_client_required", "none"); + } + r.confSet("client_mount_timeout", "30"); + r.connect(); + + IoCTX io = r.ioCtxCreate(pool.getSourceDir()); + try { + Rbd rbd = new Rbd(io); + // The image is only stat'ed, so it is opened read only and cannot take the exclusive lock. + RbdImage image = rbd.openReadOnly(volumeUuid); + try { + RbdImageInfo rbdInfo = image.stat(); + KVMPhysicalDisk disk = new KVMPhysicalDisk(pool.getSourceDir() + "/" + volumeUuid, volumeUuid, pool); + disk.setFormat(PhysicalDiskFormat.RAW); + disk.setSize(rbdInfo.size); + disk.setVirtualSize(rbdInfo.size); + return disk; + } finally { + closeRbdImage(rbd, image, volumeUuid); + } + } finally { + r.ioCtxDestroy(io); + } + } catch (RadosException e) { + logger.error("A Ceph RADOS operation failed (" + e.getReturnValue() + "). The error was: " + e.getMessage() + + " - " + ErrorCode.getErrorMessage(e.getReturnValue())); + throw new CloudRuntimeException(e.toString(), e); + } catch (RbdException e) { + logger.error("A Ceph RBD operation failed (" + e.getReturnValue() + "). The error was: " + e.getMessage() + + " - " + ErrorCode.getErrorMessage(e.getReturnValue())); + throw new CloudRuntimeException(e.toString(), e); + } finally { + r.shutDown(); + } + } + + /** + * Closes an RBD image without throwing, so that a failure to close cannot discard a + * successful result or hide the exception that is already on its way out. + */ + private void closeRbdImage(Rbd rbd, RbdImage image, String volumeUuid) { + try { + rbd.close(image); + } catch (RbdException e) { + logger.warn("Failed to close RBD image " + volumeUuid + " (" + e.getReturnValue() + "): " + + e.getMessage() + " - " + ErrorCode.getErrorMessage(e.getReturnValue())); + } + } + /** * adjust refcount */