fix: a network, pool or secret a VM needs is not stopped or deleted

Stopping a network cut the running VMs on it off, deleting one or a
secret kept the VMs that name it from starting, and stopping a pool
took the disks of running VMs away, all without a word. Each is now
refused with the VMs (and, for a secret, the pools) that need it:
stopping while a running VM uses it, deleting while a VM's definition
names it. A disk given by its file path does not need its pool's
definition. The API's pool stop is refused the same way.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

networks/tests/test_network_in_use.py

@@ -0,0 +1,58 @@
+"""A network a VM needs is not stopped or deleted: stopping one a running VM
+is on would cut that VM off, deleting one a VM's definition names would keep
+that VM from starting. Runs on a private libvirt test driver: "test:///default"
+is shared by the whole process."""
+
+import tempfile
+from unittest.mock import patch
+
+import libvirt
+from computes.models import Compute
+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
+
+NODE = """<node>
+  <domain type='test'><name>net-user</name><memory>1024</memory><os><type>hvm</type></os>
+    <devices><interface type='network'><source network='lab'/></interface></devices></domain>
+  <network><name>lab</name><bridge name='virbr9'/><ip address='192.0.2.1' netmask='255.255.255.0'/></network>
+</node>"""
+
+
+class NetworkInUseTests(TestCase):
+    def setUp(self):
+        node = tempfile.NamedTemporaryFile("w", suffix=".xml")
+        self.addCleanup(node.close)
+        node.write(NODE)
+        node.flush()
+        self.conn = libvirt.open(f"test://{node.name}")
+        self.addCleanup(self.conn.close)
+        patcher = patch("vrtManager.connection.connection_manager.get_connection", return_value=self.conn)
+        patcher.start()
+        self.addCleanup(patcher.stop)
+        compute = Compute.objects.create(name="net-use", hostname="localhost", login="", password="", type=4)
+        self.client.force_login(get_user_model().objects.create_superuser("net-use", "nu@example.com", "pw"))
+        self.url = reverse("network", args=[compute.id, "lab"])
+
+    def post(self, action):
+        response = self.client.post(self.url, {action: ""}, HTTP_REFERER=f"http://testserver{self.url}")
+        return [str(m) for m in get_messages(response.wsgi_request)]
+
+    def test_a_network_a_running_vm_is_on_is_not_stopped(self):
+        messages = self.post("stop")
+        self.assertTrue(any("running VMs net-user" in m for m in messages), messages)
+        self.assertTrue(self.conn.networkLookupByName("lab").isActive())
+        self.conn.lookupByName("net-user").destroy()
+        self.post("stop")
+        self.assertFalse(self.conn.networkLookupByName("lab").isActive())
+
+    def test_a_network_a_vm_definition_names_is_not_deleted(self):
+        self.conn.lookupByName("net-user").destroy()
+        self.conn.networkLookupByName("lab").destroy()
+        messages = self.post("delete")
+        self.assertTrue(any("net-user" in m and "would no longer start" in m for m in messages), messages)
+        self.assertIn("lab", self.conn.listDefinedNetworks())
+        self.conn.lookupByName("net-user").undefine()
+        self.post("delete")
+        self.assertNotIn("lab", self.conn.listDefinedNetworks())

storages/tests/test_pool_in_use.py

@@ -0,0 +1,77 @@
+"""A pool a VM needs is not stopped or deleted: stopping one that holds a
+running VM's disk would take that disk away, deleting one a VM's definition
+names (a disk of type volume) would keep that VM from starting. A disk given
+by its file path keeps working without the pool's definition. Runs on a
+private libvirt test driver: "test:///default" is shared by the whole process."""
+
+import tempfile
+from unittest.mock import patch
+
+import libvirt
+from computes.models import Compute
+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
+from rest_framework.test import APIClient
+
+NODE = "<node><pool type='dir'><name>use-pool</name><target><path>/use-pool</path></target></pool></node>"
+VOLUME = "<volume><name>v1.img</name><capacity>1048576</capacity></volume>"
+
+
+def domain(name, disk):
+    return (f"<domain type='test'><name>{name}</name><memory>1024</memory><os><type>hvm</type></os>"
+            f"<devices><disk device='disk' {disk}<target dev='vda'/></disk></devices></domain>")
+
+
+class PoolInUseTests(TestCase):
+    def setUp(self):
+        node = tempfile.NamedTemporaryFile("w", suffix=".xml")
+        self.addCleanup(node.close)
+        node.write(NODE)
+        node.flush()
+        self.conn = libvirt.open(f"test://{node.name}")
+        self.addCleanup(self.conn.close)
+        patcher = patch("vrtManager.connection.connection_manager.get_connection", return_value=self.conn)
+        patcher.start()
+        self.addCleanup(patcher.stop)
+        self.pool = self.conn.storagePoolLookupByName("use-pool")
+        self.path = self.pool.createXML(VOLUME, 0).path()
+        self.compute = Compute.objects.create(name="pool-use", hostname="localhost", login="", password="", type=4)
+        self.admin = get_user_model().objects.create_superuser("pool-use", "pu@example.com", "pw")
+        self.client.force_login(self.admin)
+        self.url = reverse("storage", args=[self.compute.id, "use-pool"])
+
+    def post(self, action):
+        response = self.client.post(self.url, {action: ""}, HTTP_REFERER=f"http://testserver{self.url}")
+        return [str(m) for m in get_messages(response.wsgi_request)]
+
+    def test_a_pool_that_holds_a_running_vm_disk_is_not_stopped(self):
+        self.conn.createXML(domain("disk-user", f"type='file'><source file='{self.path}'/>"), 0)
+        messages = self.post("stop")
+        self.assertTrue(any(m.startswith("Pool use-pool holds disks of the running VMs disk-user") for m in messages), messages)
+        self.assertTrue(self.pool.isActive())
+        self.conn.lookupByName("disk-user").destroy()
+        self.post("stop")
+        self.assertFalse(self.pool.isActive())
+
+    def test_the_api_does_not_stop_it_either(self):
+        self.conn.createXML(domain("api-user", f"type='file'><source file='{self.path}'/>"), 0)
+        api = APIClient()
+        api.force_login(self.admin)
+        response = api.post(reverse("compute-storage-stop", kwargs={"compute_pk": self.compute.id, "pk": "use-pool"}))
+        self.assertEqual(response.status_code, 400)
+        self.assertIn("running VMs api-user", response.json()["detail"])
+        self.assertTrue(self.pool.isActive())
+
+    def test_a_pool_a_vm_definition_names_is_not_deleted(self):
+        self.conn.defineXML(domain("by-name", "type='volume'><source pool='use-pool' volume='v1.img'/>"))
+        self.conn.defineXML(domain("by-path", f"type='file'><source file='{self.path}'/>"))
+        self.pool.destroy()
+        messages = self.post("delete")
+        self.assertTrue(any("by-name" in m and "would no longer start" in m for m in messages), messages)
+        self.assertFalse(any("by-path" in m for m in messages), messages)
+        self.assertIn("use-pool", self.conn.listDefinedStoragePools())
+        self.conn.lookupByName("by-name").undefine()
+        self.post("delete")
+        self.assertNotIn("use-pool", self.conn.listDefinedStoragePools())

storages/views.py

@@ -185,10 +185,17 @@ def storage(request, compute_id, pool):
             conn.start()
             return HttpResponseRedirect(request.get_full_path())
         if "stop" in request.POST:
-            conn.stop()
+            try:
+                conn.stop()
+            except OperationError as err:  # a running VM's disk is in it
+                messages.error(request, str(err))
             return HttpResponseRedirect(request.get_full_path())
         if "delete" in request.POST:
-            conn.delete()
+            try:
+                conn.delete()
+            except OperationError as err:  # a VM names it for a disk
+                messages.error(request, str(err))
+                return HttpResponseRedirect(request.get_full_path())
             return HttpResponseRedirect(reverse("storages", args=[compute_id]))
         if "set_autostart" in request.POST:
             conn.set_autostart(1)

virtsecrets/tests/test_secret_in_use.py

@@ -0,0 +1,64 @@
+"""A secret a VM disk or a pool needs is not deleted: without it that VM or
+pool no longer starts. A disk or pool names its secret by UUID or by usage.
+libvirt's test driver has no secrets, so the connection is a mock."""
+
+from unittest.mock import MagicMock, patch
+
+from computes.models import Compute
+from django.contrib.auth import get_user_model
+from django.contrib.messages import get_messages
+from django.test import SimpleTestCase, TestCase
+from django.urls import reverse
+from vrtManager.util import OperationError
+from vrtManager.virtsecrets import wvmSecrets
+
+UUID = "8f2c0000-0000-4000-8000-000000000001"
+
+
+def libvirt_object(name, xml):
+    obj = MagicMock()
+    obj.name.return_value = name
+    obj.XMLDesc.return_value = xml
+    return obj
+
+
+def disk_vm(name, secret):
+    return libvirt_object(name, f"<domain><devices><disk type='network'><source protocol='rbd' name='rbd/{name}'/>"
+                                f"<auth username='libvirt'>{secret}</auth><target dev='vdb'/></disk></devices></domain>")
+
+
+class SecretInUseTests(SimpleTestCase):
+    def delete(self, domains=(), pools=()):
+        with patch("vrtManager.connection.connection_manager.get_connection") as get:
+            wvm = get.return_value
+            wvm.listAllDomains.return_value = list(domains)
+            wvm.listAllStoragePools.return_value = list(pools)
+            secret = wvm.secretLookupByUUIDString.return_value
+            secret.usageID.return_value = "client.libvirt secret"
+            wvmSecrets("host", "login", "pw", 1).delete_secret(UUID)
+        return secret
+
+    def test_a_secret_a_vm_disk_or_a_pool_names_is_not_deleted(self):
+        pool = libvirt_object("ceph-rbd", "<pool type='rbd'><source><auth type='ceph' username='libvirt'>"
+                                          "<secret usage='client.libvirt secret'/></auth></source></pool>")
+        with self.assertRaises(OperationError) as refused:
+            self.delete([disk_vm("db-2", f"<secret type='ceph' uuid='{UUID}'/>")], [pool])
+        self.assertIn("VMs db-2", str(refused.exception))
+        self.assertIn("pools ceph-rbd", str(refused.exception))
+
+    def test_an_unused_secret_is_deleted(self):
+        other = disk_vm("web-1", "<secret type='ceph' uuid='8f2c0000-0000-4000-8000-000000000002'/>")
+        self.delete([other]).undefine.assert_called_once_with()
+
+
+class SecretInUsePageTests(TestCase):
+    def test_the_page_says_who_needs_it(self):
+        compute = Compute.objects.create(name="secret-use", hostname="localhost", login="", password="", type=4)
+        self.client.force_login(get_user_model().objects.create_superuser("secret-use", "su@example.com", "pw"))
+        url = reverse("virtsecrets", args=[compute.id])
+        with patch("virtsecrets.views.wvmSecrets") as conn:
+            conn.return_value.get_secrets.return_value = []
+            conn.return_value.delete_secret.side_effect = OperationError(f"Secret {UUID} is used by VMs db-2")
+            response = self.client.post(url, {"delete": "", "uuid": UUID})
+        self.assertRedirects(response, url, fetch_redirect_response=False)
+        self.assertEqual([str(m) for m in get_messages(response.wsgi_request)], [f"Secret {UUID} is used by VMs db-2"])

virtsecrets/views.py

@@ -11,6 +11,7 @@ from libvirt import (
     VIR_SECRET_USAGE_TYPE_VOLUME,
     libvirtError,
 )
+from vrtManager.util import OperationError
 from vrtManager.virtsecrets import wvmSecrets
 
 from virtsecrets.forms import AddSecret
@@ -68,7 +69,10 @@ def secrets(request, compute_id):
                         messages.error(request, msg_err.as_text())
             if "delete" in request.POST:
                 uuid = request.POST.get("uuid", "")
-                conn.delete_secret(uuid)
+                try:
+                    conn.delete_secret(uuid)
+                except OperationError as err:  # a VM disk or a pool needs it
+                    messages.error(request, str(err))
                 return HttpResponseRedirect(request.get_full_path())
             if "set_value" in request.POST:
                 uuid = request.POST.get("uuid", "")

vrtManager/connection.py

@@ -955,6 +955,17 @@ class wvmConnect(object):
             instances.append(name)
         return instances
 
+    def domains_using(self, match, running_only=False):
+        """Names of the domains with a definition, parsed, that match(tree)
+        holds for: the live and the persistent one of each domain, or with
+        running_only the live one of each running domain."""
+        listing = libvirt.VIR_CONNECT_LIST_DOMAINS_ACTIVE if running_only else 0
+        flags = (0,) if running_only else (0, libvirt.VIR_DOMAIN_XML_INACTIVE)
+        return sorted(
+            dom.name() for dom in self.wvm.listAllDomains(listing)
+            if any(match(etree.fromstring(dom.XMLDesc(f))) for f in flags)
+        )
+
     def paths_in_use(self, skip_uuid=None, own=()):
         """Paths (names for network disks) a domain uses as a disk or a
         backing file of one, or as a file of its snapshots (overlays, memory,

vrtManager/network.py

@@ -161,10 +161,25 @@ class wvmNetwork(wvmConnect):
     def start(self):
         self.net.create()
 
+    def _vms_on_it(self, running_only):
+        name = self.net.name()
+        return self.domains_using(
+            lambda domain: domain.xpath("devices/interface[@type='network']/source[@network=$name]", name=name),
+            running_only,
+        )
+
     def stop(self):
+        """Refused while a running VM is on it: that VM would be cut off."""
+        if vms := self._vms_on_it(running_only=True):
+            raise util.OperationError(f"Network {self.net.name()} is in use by the running VMs {', '.join(vms)}; "
+                                      "shut them down or remove their network cards first")
         self.net.destroy()
 
     def delete(self):
+        """Refused while a VM's definition names it: that VM would no longer start."""
+        if vms := self._vms_on_it(running_only=False):
+            raise util.OperationError(f"VMs {', '.join(vms)} use network {self.net.name()} and would no longer "
+                                      "start; remove their network cards first")
         self.net.undefine()
 
     def update(self, command, section, xml, parentIndex, flags=0):

vrtManager/storage.py

@@ -155,9 +155,27 @@ class wvmStorage(wvmConnect):
         return self.pool.create(0)
 
     def stop(self):
+        """Refused while it holds a disk of a running VM, named by the pool
+        or by a volume's path: that VM would lose the disk."""
+        name = self.pool.name()
+        paths = {vol.path() for vol in self.pool.listAllVolumes()}
+        vms = self.domains_using(
+            lambda domain: domain.xpath("devices/disk/source[@pool=$name]", name=name)
+            or paths.intersection(self._source_paths(domain)),
+            running_only=True,
+        )
+        if vms:
+            raise util.OperationError(f"Pool {name} holds disks of the running VMs {', '.join(vms)}; "
+                                      "shut them down first")
         return self.pool.destroy()
 
     def delete(self):
+        """Refused while a VM's definition names it for a disk: that VM would
+        no longer start. A disk given by its file path does not need it."""
+        name = self.pool.name()
+        if vms := self.domains_using(lambda domain: domain.xpath("devices/disk/source[@pool=$name]", name=name)):
+            raise util.OperationError(f"VMs {', '.join(vms)} have disks in pool {name} by its name and would no "
+                                      "longer start; detach those disks first")
         self.pool.undefine()
 
     def get_autostart(self):

vrtManager/virtsecrets.py

@@ -1,6 +1,7 @@
 import base64
 
 from lxml import etree
+from vrtManager import util
 from vrtManager.connection import wvmConnect
 
 # the element of <usage> that holds the value, per usage type
@@ -22,5 +23,15 @@ class wvmSecrets(wvmConnect):
         secrt.setValue(base64.b64decode(value, validate=True))
 
     def delete_secret(self, uuid):
+        """Refused while a VM disk or a pool names it, by UUID or by usage:
+        without it that VM or pool would no longer start."""
         secrt = self.get_secret(uuid)
+        refs = "//secret[@uuid=$uuid or @usage=$usage]"
+        usage = secrt.usageID() or ""
+        vms = self.domains_using(lambda domain: domain.xpath(f"devices/disk{refs}", uuid=uuid, usage=usage))
+        pools = sorted(pool.name() for pool in self.wvm.listAllStoragePools()
+                       if etree.fromstring(pool.XMLDesc(0)).xpath(f"source{refs}", uuid=uuid, usage=usage))
+        if vms or pools:
+            users = "; ".join(f"{kind} {', '.join(names)}" for kind, names in (("VMs", vms), ("pools", pools)) if names)
+            raise util.OperationError(f"Secret {uuid} is used by {users}; remove it from them first")
         secrt.undefine()