From b89672169fe98bcc37f7081633b1cb07d6e57c93 Mon Sep 17 00:00:00 2001 From: catborise Date: Mon, 2 Nov 2026 16:03:44 +0300 Subject: [PATCH] fix(instances): Refresh ISO list says which pools it could not read A pool whose refresh failed, one busy with a volume copy for example, was skipped without a word, so a hand-copied image stayed missing with no reason given. The other pools are still refreshed; each failure is shown with libvirt's error. The API comment no longer says listing the ISO images refreshes pools. --- instances/api/serializers.py | 3 +-- instances/tests/test_disk_isolation.py | 9 +++++++++ instances/views.py | 3 ++- vrtManager/instance.py | 9 +++++++-- vrtManager/tests/test_iso_media.py | 12 ++++++++++-- 5 files changed, 29 insertions(+), 7 deletions(-) diff --git a/instances/api/serializers.py b/instances/api/serializers.py index 1a4b72c0..8e3ede47 100644 --- a/instances/api/serializers.py +++ b/instances/api/serializers.py @@ -12,8 +12,7 @@ class InstanceSerializer(serializers.ModelSerializer): class InstanceDetailsSerializer(serializers.ModelSerializer): def get_fields(self): fields = super().get_fields() - # the host's ISO images are a superuser's, as on the VM page; reading - # them refreshes every pool of the host + # the host's ISO images are a superuser's, as on the VM page request = self.context.get("request") if not (request and request.user.is_superuser): del fields["media_iso"] diff --git a/instances/tests/test_disk_isolation.py b/instances/tests/test_disk_isolation.py index 9ebefe28..7027adc7 100644 --- a/instances/tests/test_disk_isolation.py +++ b/instances/tests/test_disk_isolation.py @@ -6,6 +6,7 @@ views must not act on volumes or option values taken blindly from the client. from unittest.mock import patch from django.contrib.auth import get_user_model +from django.contrib.messages import get_messages from django.test import TestCase from django.urls import reverse @@ -90,10 +91,18 @@ class DiskViewsTenantIsolationTestCase(TestCase): 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: + mock_wvm.return_value.refresh_iso_pools.return_value = {} 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_refresh_iso_reports_the_pools_that_failed(self): + self.client.force_login(self.superuser) + with patch("instances.models.wvmInstance") as mock_wvm: + mock_wvm.return_value.refresh_iso_pools.return_value = {"busy": "pool is busy"} + res = self._post("refresh_iso", {}) + self.assertEqual([str(m) for m in get_messages(res.wsgi_request)], ["Pool busy was not read again: pool is busy"]) + 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/views.py b/instances/views.py index 1a61be11..13dddc79 100755 --- a/instances/views.py +++ b/instances/views.py @@ -1689,7 +1689,8 @@ def mount_iso(request, pk): @superuser_only def refresh_iso(request, pk): instance = get_instance(request.user, pk, perm_type="change") - instance.proxy.refresh_iso_pools() + for pool, err in instance.proxy.refresh_iso_pools().items(): + messages.error(request, _("Pool %(pool)s was not read again: %(err)s") % {"pool": pool, "err": err}) return _back(request, pk, "disks") diff --git a/vrtManager/instance.py b/vrtManager/instance.py index 7ef0bab1..5f24c71f 100644 --- a/vrtManager/instance.py +++ b/vrtManager/instance.py @@ -1715,10 +1715,15 @@ class wvmInstance(wvmConnect): def refresh_iso_pools(self): """Refresh the active pools, so that an image copied to one by hand - is listed by get_iso_media.""" + is listed by get_iso_media. A pool that fails does not stop the + others; returns the errors, by pool name.""" + failed = {} for pool in self.wvm.listAllStoragePools(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE): - with contextlib.suppress(libvirtError): # stopped or removed meanwhile + try: pool.refresh(0) + except libvirtError as err: + failed[pool.name()] = err + return failed def get_iso_media(self): """The .iso volumes libvirt knows in the active pools; one copied in diff --git a/vrtManager/tests/test_iso_media.py b/vrtManager/tests/test_iso_media.py index 5556cab4..ba52d87c 100644 --- a/vrtManager/tests/test_iso_media.py +++ b/vrtManager/tests/test_iso_media.py @@ -91,13 +91,21 @@ class IsoListTestCase(unittest.TestCase): self.assertEqual(inst.get_iso_media(), ["b.iso"]) def test_refresh_reads_every_active_pool_again(self): - a, b = pool([], refresh_error=True), pool([]) + a, b = pool([]), pool([]) inst = self.wvm(a, b) - inst.refresh_iso_pools() + self.assertEqual(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_is_returned_and_the_others_still_run(self): + a, b = pool([], refresh_error=True), pool([]) + a.name.return_value = "busy" + failed = self.wvm(a, b).refresh_iso_pools() + self.assertEqual(list(failed), ["busy"]) + self.assertIsInstance(failed["busy"], libvirtError) + b.refresh.assert_called_once_with(0) + if __name__ == "__main__": unittest.main()