From d5d054de71bd0ddaaf70ecd82f241c12219af503 Mon Sep 17 00:00:00 2001 From: catborise Date: Mon, 2 Nov 2026 13:51:20 +0300 Subject: [PATCH] perf(instances): the VM page lists ISO images without reading every pool The VM page refreshed every active storage pool to list the ISO images for its CD-ROM drives. A pool holding large disk images took up to seconds to refresh, most of the page's time, on every page load. The page now lists the ISO images libvirt already knows. Images uploaded through the panel are known at once. For an image copied to a pool by hand, the Removable media section has a Refresh ISO list button that refreshes the active pools and returns to the page. --- .../templates/instances/settings_tab.html | 8 ++++- instances/tests/test_disk_isolation.py | 8 +++++ instances/urls.py | 1 + instances/views.py | 8 +++++ vrtManager/instance.py | 12 +++++-- vrtManager/tests/test_iso_media.py | 34 +++++++++++-------- 6 files changed, 52 insertions(+), 19 deletions(-) diff --git a/instances/templates/instances/settings_tab.html b/instances/templates/instances/settings_tab.html index f749227d..72da6544 100644 --- a/instances/templates/instances/settings_tab.html +++ b/instances/templates/instances/settings_tab.html @@ -152,7 +152,13 @@

{% trans "Removable media" %}

- {% include 'add_instance_cdrom.html' %} +
+ {# the pools are read again: an ISO image copied to one by hand is listed then #} +
{% csrf_token %} + +
+ {% include 'add_instance_cdrom.html' %} +
{% for cd in instance.media %}
diff --git a/instances/tests/test_disk_isolation.py b/instances/tests/test_disk_isolation.py index d5bbf559..9ebefe28 100644 --- a/instances/tests/test_disk_isolation.py +++ b/instances/tests/test_disk_isolation.py @@ -23,6 +23,7 @@ DISK_ENDPOINTS = [ ("detach_cdrom", ["hda"]), ("mount_iso", []), ("unmount_iso", []), + ("refresh_iso", []), ] VM_DISK = { @@ -86,6 +87,13 @@ class DiskViewsTenantIsolationTestCase(TestCase): mock_storage.assert_not_called() mock_create.assert_not_called() + def test_refresh_iso_reads_the_pools_again_and_returns_to_the_disks_tab(self): + self.client.force_login(self.superuser) + with patch("instances.models.wvmInstance") as mock_wvm: + res = self._post("refresh_iso", {}) + self.assertRedirects(res, self.referer + "#disks", fetch_redirect_response=False) + mock_wvm.return_value.refresh_iso_pools.assert_called_once_with() + def test_delete_vol_deletes_the_volume_attached_to_dev_not_the_posted_one(self): self.client.force_login(self.superuser) with patch("instances.models.wvmInstance") as mock_wvm, patch( diff --git a/instances/urls.py b/instances/urls.py index f07fdf72..ffd35068 100755 --- a/instances/urls.py +++ b/instances/urls.py @@ -40,6 +40,7 @@ urlpatterns = [ path("/detach_cdrom//", views.detach_cdrom, name="detach_cdrom"), path("/unmount_iso/", views.unmount_iso, name="unmount_iso"), path("/mount_iso/", views.mount_iso, name="mount_iso"), + path("/refresh_iso/", views.refresh_iso, name="refresh_iso"), path("/snapshot/", views.snapshot, name="snapshot"), path("/delete_snapshot/", views.delete_snapshot, name="delete_snapshot"), path("/revert_snapshot/", views.revert_snapshot, name="revert_snapshot"), diff --git a/instances/views.py b/instances/views.py index 800f6dd8..1a61be11 100755 --- a/instances/views.py +++ b/instances/views.py @@ -1685,6 +1685,14 @@ def mount_iso(request, pk): return redirect(same_origin_referer(request, reverse("instances:instance", args=[pk])) + "#disks") +@require_POST +@superuser_only +def refresh_iso(request, pk): + instance = get_instance(request.user, pk, perm_type="change") + instance.proxy.refresh_iso_pools() + return _back(request, pk, "disks") + + @require_POST def snapshot(request, pk): return _snapshot_job(request, pk, "snapshot.create", diff --git a/vrtManager/instance.py b/vrtManager/instance.py index 8d366984..7ef0bab1 100644 --- a/vrtManager/instance.py +++ b/vrtManager/instance.py @@ -1713,12 +1713,18 @@ class wvmInstance(wvmConnect): if active: self.instance.blockResize(disk["path"], disk["size_new"], VIR_DOMAIN_BLOCK_RESIZE_BYTES) + def refresh_iso_pools(self): + """Refresh the active pools, so that an image copied to one by hand + is listed by get_iso_media.""" + for pool in self.wvm.listAllStoragePools(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE): + with contextlib.suppress(libvirtError): # stopped or removed meanwhile + pool.refresh(0) + def get_iso_media(self): + """The .iso volumes libvirt knows in the active pools; one copied in + by hand is known after refresh_iso_pools.""" iso = [] for pool in self.wvm.listAllStoragePools(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE): - # refreshed so that an image copied to the pool by hand shows up - with contextlib.suppress(libvirtError): - pool.refresh(0) try: names = pool.listVolumes() except libvirtError: # stopped or removed meanwhile diff --git a/vrtManager/tests/test_iso_media.py b/vrtManager/tests/test_iso_media.py index 4677f3b6..5556cab4 100644 --- a/vrtManager/tests/test_iso_media.py +++ b/vrtManager/tests/test_iso_media.py @@ -69,31 +69,35 @@ def pool(volumes, refresh_error=False, list_error=False): class IsoListTestCase(unittest.TestCase): - """The CD-ROM choices are the .iso volumes of the active pools, refreshed - so that an image copied in by hand shows up.""" + """The CD-ROM choices are the .iso volumes libvirt knows in the active + pools; the pools are read again only when asked.""" - def iso(self, *pools): + def wvm(self, *pools): inst = wvmInstance.__new__(wvmInstance) inst.wvm = MagicMock() inst.wvm.listAllStoragePools.return_value = list(pools) - return inst, inst.get_iso_media() + return inst - def test_iso_volumes_of_the_active_pools(self): + def test_iso_volumes_of_the_active_pools_without_a_refresh(self): a, b = pool(["a.iso", "disk.qcow2"]), pool(["B.ISO"]) - inst, iso = self.iso(a, b) - self.assertEqual(iso, ["a.iso", "B.ISO"]) + inst = self.wvm(a, b) + self.assertEqual(inst.get_iso_media(), ["a.iso", "B.ISO"]) + inst.wvm.listAllStoragePools.assert_called_once_with(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE) + a.refresh.assert_not_called() + b.refresh.assert_not_called() + + def test_a_pool_stopped_meanwhile_is_skipped(self): + inst = self.wvm(pool([], list_error=True), pool(["b.iso"])) + self.assertEqual(inst.get_iso_media(), ["b.iso"]) + + def test_refresh_reads_every_active_pool_again(self): + a, b = pool([], refresh_error=True), pool([]) + inst = self.wvm(a, b) + inst.refresh_iso_pools() inst.wvm.listAllStoragePools.assert_called_once_with(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE) a.refresh.assert_called_once_with(0) b.refresh.assert_called_once_with(0) - def test_a_failed_refresh_still_lists_the_pool(self): - _, iso = self.iso(pool(["a.iso"], refresh_error=True)) - self.assertEqual(iso, ["a.iso"]) - - def test_a_pool_stopped_meanwhile_is_skipped(self): - _, iso = self.iso(pool([], list_error=True), pool(["b.iso"])) - self.assertEqual(iso, ["b.iso"]) - if __name__ == "__main__": unittest.main()