kvm: look RBD volumes up through librbd instead of refreshing the whole pool - #14188
bhouse-nexthop wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.22 #14188 +/- ##
=========================================
Coverage 17.93% 17.94%
- Complexity 16142 16148 +6
=========================================
Files 5928 5928
Lines 535205 535247 +42
Branches 65501 65503 +2
=========================================
+ Hits 95989 96028 +39
- Misses 428286 428290 +4
+ Partials 10930 10929 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
89d9f50 to
a3a511b
Compare
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 <bhouse@nexthop.ai>
a3a511b to
d74bf10
Compare
|
@blueorangutan package |
|
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19299 |
|
@blueorangutan test |
|
@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-17030)
|
This branch has not been deployed
3 participants
Description
Starting a VM whose root disk is on RBD primary storage refreshes the entire libvirt storage pool. The cost grows with the number of volumes in the Ceph pool, and it is paid on every single VM start.
Why it happens
LibvirtStorageAdaptor.getPhysicalDisk()asks libvirt for the volume. The volume was just created by the management server, so this host's libvirt has never seen it and the lookup misses.getVolume()then refreshes the whole pool:The existing comment in
getVolume()already describes the case: "This can happen when a volume has just been created on a different host and since then the libvirt storage pool has not been refreshed." For a cluster that creates VMs continuously, that is every start.Measured cost
Ceph pool holding 950 RBD images, measured on the hypervisor:
Running four of these at once does not slow them down (9.9 - 11.1 s each), so this is not Ceph contention. It is 950 serial round trips.
Agent-side
StartCommandon a host in a 12-node cluster running short-lived VMs:StartCommandgetPhysicalDiskThe stall is 0 when the volume happens to be cached, which is what points at the refresh rather than at fixed work.
Fix
Look RBD volumes up directly through librbd and skip libvirt entirely.
This makes
getPhysicalDisk()consistent with the rest of the class. Creating, cloning, resizing, copying and deleting RBD volumes inLibvirtStorageAdaptoralready useRados/Rbddirectly. Only the lookup went through libvirt.StoragePoolType.RBDonly. Every other pool type is untouched.<ceph pool>/<volume uuid>is the same format the class already builds increatePhysicalDisk,createDiskFromTemplateandcreateDiskFromTemplateOnRBD.rbd stat, matching what the class already does after converting an image into an RBD volume.What this does not fix
getStorageStatsstill callspool.refresh()explicitly, so the full walk of the pool still happens on the storage stats poll. That is a periodic background poll rather than something on the VM start path, so it is left alone here.Connection handling
Because this runs on every VM start rather than occasionally, the rados connection is handled more carefully than at the existing call sites in this class:
shutDown()in afinallyKVMStoragePoolManager.getPhysicalDiskretries a missing volume 100 times with a 3 s sleep, so leaking the client would mean 101 live librados clients for one absent volumeopenReadOnly()rather thanopen()Rbd.close()throws, and in afinallythat would discard a successful lookup or hide the exception already on its way outrados_conf_set("key", NULL)is called. This is pre-existing at the other call sites in this class and is guarded only on the new oneTypes of changes
How Has This Been Tested?
rbd_liston its own is 0.005 s; opening and stating all 950 images serially, which is what the refresh does, is 11.67 s.On the one non-obvious point, the librbd lookup reports
rbd statsize as both size and virtual size, where the libvirt path reportedallocationandcapacityseparately. These are the same number for RBD: libvirt derives allocation asobj_size * num_objs. Checked against libvirt 10.6.0 on three RBD volumes of different sizes:Consumers of those two values (
LibvirtGetVolumeStatsCommandWrapper, the template and volume sizes reported back byKVMStorageProcessor, and the snapshot pre-check invalidateAvailableSizeOnPoolToTakeVolumeSnapshot) therefore see no change.