From befd1747df0c46761887212a8883a9bc92c530df Mon Sep 17 00:00:00 2001 From: catborise Date: Wed, 28 Oct 2026 17:03:51 +0300 Subject: [PATCH] perf(storages): a pool's volumes are read with one libvirt call each wvmStorage.update_volumes looked each volume up by name three times, read its info twice and its XML once: six round trips per volume to the host. It now lists the pool's volumes as objects (listAllVolumes) and reads each one's XML only (volume_details): name, capacity, allocation and format are all in it, and libvirt refreshes a volume for its XML as it does for info(). A pool of N volumes takes 2 + N calls instead of 2 + 6N; 100 volumes over SSH on a test host read in about 110 ms instead of 720 ms, with the same values. - A volume deleted between the listing and its read is left out; any other error is raised. Volumes come sorted by name. - get_volume_details (the API's one volume) reads it the same way. - Only a refused refresh (a clone running in the pool) is tolerated, not any exception. - The pool page reads its volumes and space only to render itself; its actions, which all redirect, no longer read every volume first. - get_volume_size and get_volume_allocation go: nothing else used them. --- storages/views.py | 37 +++++++++---------- vrtManager/storage.py | 55 +++++++++++++++-------------- vrtManager/tests/test_host_reads.py | 55 ++++++++++++++++++++++++++++- 3 files changed, 102 insertions(+), 45 deletions(-) diff --git a/storages/views.py b/storages/views.py index 56f1eeb8..0eb657a9 100644 --- a/storages/views.py +++ b/storages/views.py @@ -136,24 +136,6 @@ def storage(request, compute_id, pool): storages = conn.get_storages() state = conn.is_active() - # first: it refreshes the pool, and libvirt measures a pool's space only then - volumes = conn.update_volumes() if state else None - for volume in volumes or []: - volume["iso"] = is_iso(volume["name"]) - try: - size, free = conn.get_size() - status = conn.get_status() - used = size - free - if state and size: # an iSCSI pool without LUNs has no size - percent = (used * 100) // size - else: - percent = 0 - except libvirtError: # both read the pool's info: the page says it could not be read - size = free = used = percent = status = None - - path = conn.get_target_path() or conn.get_source_name() # an RBD pool: its Ceph pool - type = conn.get_type() - autostart = conn.get_autostart() if request.method == "POST": subject = f"Storage pool {pool}" @@ -222,6 +204,25 @@ def storage(request, compute_id, pool): messages.error(request, msg_err.as_text()) return redirect(reverse("storage", args=[compute.id, pool])) + # read only for the page: every action above redirects + # first: it refreshes the pool, and libvirt measures a pool's space only then + volumes = conn.update_volumes() if state else None + for volume in volumes or []: + volume["iso"] = is_iso(volume["name"]) + try: + size, free = conn.get_size() + status = conn.get_status() + used = size - free + if state and size: # an iSCSI pool without LUNs has no size + percent = (used * 100) // size + else: + percent = 0 + except libvirtError: # both read the pool's info: the page says it could not be read + size = free = used = percent = status = None + + path = conn.get_target_path() or conn.get_source_name() # an RBD pool: its Ceph pool + type = conn.get_type() + autostart = conn.get_autostart() job_resource = claims.pool_key(compute.id, pool) conn.close() diff --git a/vrtManager/storage.py b/vrtManager/storage.py index b8905a70..b56dab21 100644 --- a/vrtManager/storage.py +++ b/vrtManager/storage.py @@ -5,6 +5,20 @@ from vrtManager import util from vrtManager.connection import wvmConnect +def volume_details(vol): + """A volume's name, capacity (size), allocation and format, from its XML: + one libvirt call, which refreshes the volume as info() does. Logical and + iSCSI volumes have no format (None).""" + def read(doc): + return { + "name": util.get_xpath(doc, "/volume/name"), + "size": int(util.get_xpath(doc, "/volume/capacity")), + "allocation": int(util.get_xpath(doc, "/volume/allocation")), + "type": util.get_xpath(doc, "/volume/target/format/@type"), + } + return util.get_xml_path(vol.XMLDesc(0), func=read) + + class wvmStorages(wvmConnect): def get_storages_info(self): """Each pool's state, autostart, type, path (an RBD pool's Ceph pool), @@ -255,14 +269,6 @@ class wvmStorage(wvmConnect): def get_volume(self, name): return self.pool.storageVolLookupByName(name) - def get_volume_size(self, name): - vol = self.get_volume(name) - return vol.info()[1] - - def get_volume_allocation(self, name): - vol = self.get_volume(name) - return vol.info()[2] - def _vol_XMLDesc(self, name): vol = self.get_volume(name) return vol.XMLDesc(0) @@ -286,28 +292,25 @@ class wvmStorage(wvmConnect): return self.pool.refresh(0) def get_volume_details(self, volname): - with contextlib.suppress(Exception): + """The volume's details (volume_details), the pool refreshed first.""" + with contextlib.suppress(libvirt.libvirtError): # refused while a volume is cloned in the pool self.refresh() - return { - "name": volname, - "size": self.get_volume_size(volname), - "allocation": self.get_volume_allocation(volname), - "type": self.get_volume_format_type(volname), - } + return volume_details(self.get_volume(volname)) def update_volumes(self): - with contextlib.suppress(Exception): + """Every volume's details (volume_details) by name, the pool refreshed + first: two calls and one per volume, where a lookup, info() and the + XML of each took six. A volume deleted meanwhile is left out.""" + with contextlib.suppress(libvirt.libvirtError): # refused while a volume is cloned in the pool self.refresh() - vols = self.get_volumes() - return [ - { - "name": volname, - "size": self.get_volume_size(volname), - "allocation": self.get_volume_allocation(volname), - "type": self.get_volume_format_type(volname), - } - for volname in vols - ] + volumes = [] + for vol in self.pool.listAllVolumes(0): + try: + volumes.append(volume_details(vol)) + except libvirt.libvirtError as err: + if err.get_error_code() != libvirt.VIR_ERR_NO_STORAGE_VOL: + raise + return sorted(volumes, key=lambda volume: volume["name"]) def create_volume( self, diff --git a/vrtManager/tests/test_host_reads.py b/vrtManager/tests/test_host_reads.py index c0f35bb0..8efd1c7d 100644 --- a/vrtManager/tests/test_host_reads.py +++ b/vrtManager/tests/test_host_reads.py @@ -18,6 +18,7 @@ from libvirt import ( VIR_DOMAIN_PAUSED, VIR_DOMAIN_RUNNING, VIR_DOMAIN_SHUTOFF, + VIR_ERR_NO_STORAGE_VOL, VIR_ERR_NO_SUPPORT, VIR_ERR_OPERATION_DENIED, VIR_ERR_XML_ERROR, @@ -25,7 +26,7 @@ from libvirt import ( ) from vrtManager.connection import wvmConnect from vrtManager.hostdetails import wvmHostDetails -from vrtManager.storage import wvmStorages +from vrtManager.storage import wvmStorage, wvmStorages CAPABILITIES = """x86_64 hvm/usr/bin/qemu-kvm @@ -217,3 +218,55 @@ class StoragesInfoTestCase(unittest.TestCase): conn.get_storage = lambda name: pool conn.get_storages_info() pool.refresh.assert_not_called() + + +def volume(name, capacity, allocation, fmt=None, error=None): + target = f"" if fmt else "" + xml = (f"{name}{capacity}" + f"{allocation}{target}") + return MagicMock(**{"XMLDesc.side_effect": error} if error else {"XMLDesc.return_value": xml}) + + +def gone(code): + err = libvirtError("no storage vol") + err.get_error_code = lambda: code + return err + + +class VolumesTestCase(unittest.TestCase): + """A pool's volumes are read with one call each, their XML, after one listing.""" + + def storage(self, volumes): + stg = wvmStorage.__new__(wvmStorage) # no libvirt connection: only the pool's calls + stg.pool = MagicMock(**{"listAllVolumes.return_value": volumes}) + return stg + + def test_each_volume_is_one_call(self): + vols = [volume("b.qcow2", 2048, 512, "qcow2"), volume("a-lv", 1024, 1024)] + stg = self.storage(vols) + self.assertEqual(stg.update_volumes(), [ + {"name": "a-lv", "size": 1024, "allocation": 1024, "type": None}, # a logical volume has no format + {"name": "b.qcow2", "size": 2048, "allocation": 512, "type": "qcow2"}, + ]) + self.assertEqual([c[0] for c in stg.pool.mock_calls], ["refresh", "listAllVolumes"]) + for vol in vols: + self.assertEqual([c[0] for c in vol.mock_calls], ["XMLDesc"]) + + def test_a_volume_deleted_meanwhile_is_left_out(self): + stg = self.storage([volume("a.img", 1, 1, "raw"), volume("b.img", 1, 1, error=gone(VIR_ERR_NO_STORAGE_VOL))]) + self.assertEqual([v["name"] for v in stg.update_volumes()], ["a.img"]) + # any other failure is not hidden + stg = self.storage([volume("c.img", 1, 1, error=gone(VIR_ERR_OPERATION_DENIED))]) + with self.assertRaises(libvirtError): + stg.update_volumes() + + def test_a_refused_refresh_still_lists(self): + # libvirt refuses to refresh a pool while a volume is cloned in it + stg = self.storage([volume("a.img", 1, 1, "raw")]) + stg.pool.refresh.side_effect = libvirtError("pool has asynchronous jobs running") + self.assertEqual(len(stg.update_volumes()), 1) + + def test_one_volume(self): + stg = self.storage([]) + stg.pool.storageVolLookupByName.return_value = volume("a.img", 4096, 0, "raw") + self.assertEqual(stg.get_volume_details("a.img"), {"name": "a.img", "size": 4096, "allocation": 0, "type": "raw"})