test(instances): isolate the live libvirt suite on shared hosts

- Runs only when TEST_LIBVIRT_HOST is set, so manage.py test on a
  developer machine no longer creates VMs on the local libvirtd.
- Every VM and volume is named wvc-test-*, and volumes live in a
  temporary wvc-test pool (/var/lib/libvirt/wvc-test) instead of the
  default pool next to real images.
- Cleanup removes only prefixed domains and the test pool's volumes;
  it also clears leftovers of an interrupted run before starting, and
  tolerates domains that turned transient.
- tearDownClass fails if any domain or volume outside the test
  namespace appeared, disappeared or was replaced.
- livetest gains create_volume/define_vm helpers and
  test_live_dataloss.py, whose setUpClass opens the class transaction
  last, so a failed libvirt connection cannot lock the test database
  for later test classes.

The first live tests reproduce two known external-snapshot data losses
and are marked expectedFailure: reverting deletes a disk added after
the snapshot, and deleting the older of two snapshots deletes the
newer one's overlay.

README.md

@@ -445,7 +445,7 @@ ruff check .
 ```
 
 > **Live Hypervisor Testing (Optional):**
-> Without these variables the live tests use the local socket (`qemu:///system`) or are skipped. They create and delete `test*` VMs and volumes in the host's `default` pool.
+> The instance tests run only when `TEST_LIBVIRT_HOST` is set. They create `wvc-test-*` VMs and a temporary `wvc-test` pool (`/var/lib/libvirt/wvc-test`), remove them afterwards, and fail if any other VM or volume on the host changed.
 > ```bash
 > export TEST_LIBVIRT_HOST=compute1 TEST_LIBVIRT_TYPE=2   # 1 TCP, 2 SSH, 3 TLS, 4 socket
 > export TEST_LIBVIRT_LOGIN=root TEST_LIBVIRT_PASSWORD=   # login/password for TCP and TLS

conf/test-vm.xml

@@ -1,5 +1,5 @@
 <domain type="kvm">
-    <name>test-vm</name>
+    <name>wvc-test-vm</name>
     <uuid>1bd3c1f2-dd12-4b8d-a298-dff387cb9f93</uuid>
     <memory unit="KiB">131072</memory>
     <currentMemory unit="KiB">131072</currentMemory>
@@ -22,7 +22,7 @@
     <devices>
         <disk type="file" device="disk">
             <driver name="qemu" type="qcow2" cache="directsync" />
-            <source file="/var/lib/libvirt/images/test-volume.qcow2" />
+            <source file="/var/lib/libvirt/wvc-test/wvc-test-volume.qcow2" />
             <target dev="vda" bus="virtio" />
             <address type="pci" domain="0x0000" bus="0x03" slot="0x00" function="0x0" />
         </disk>

instances/livetest.py

@@ -0,0 +1,127 @@
+"""Safety helpers for the live libvirt test suite.
+
+Everything the suite creates is named with PREFIX, and its volumes live
+in a dedicated pool. Cleanup only removes prefixed domains and the
+volumes of that pool, so a broken test cannot delete other VMs or images
+on a shared host. inventory() lets a suite check that nothing else
+changed.
+"""
+
+import os
+
+import libvirt
+
+PREFIX = "wvc-test-"
+POOL = "wvc-test"
+POOL_PATH = "/var/lib/libvirt/wvc-test"
+
+UNDEFINE_FLAGS = (
+    libvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE
+    | libvirt.VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA
+    | libvirt.VIR_DOMAIN_UNDEFINE_NVRAM
+    | libvirt.VIR_DOMAIN_UNDEFINE_CHECKPOINTS_METADATA
+)
+
+
+def enabled():
+    """The suite creates and deletes VMs, so it only runs on an explicitly named host."""
+    return bool(os.environ.get("TEST_LIBVIRT_HOST"))
+
+
+def ensure_pool(conn):
+    try:
+        pool = conn.storagePoolLookupByName(POOL)
+    except libvirt.libvirtError:
+        pool = conn.storagePoolDefineXML(
+            f"<pool type='dir'><name>{POOL}</name><target><path>{POOL_PATH}</path></target></pool>"
+        )
+        pool.build(0)
+    if not pool.isActive():
+        pool.create(0)
+    pool.refresh(0)
+    return pool
+
+
+def cleanup(conn):
+    """Remove prefixed domains and every volume of the test pool."""
+    for dom in conn.listAllDomains():
+        if not dom.name().startswith(PREFIX):
+            continue
+        persistent = dom.isPersistent()
+        if dom.isActive():
+            dom.destroy()
+        if persistent:
+            dom.undefineFlags(UNDEFINE_FLAGS)
+    try:
+        pool = conn.storagePoolLookupByName(POOL)
+    except libvirt.libvirtError:
+        return
+    if pool.isActive():
+        pool.refresh(0)
+        for vol in pool.listAllVolumes():
+            vol.delete(0)
+
+
+def remove_pool(conn):
+    cleanup(conn)
+    try:
+        pool = conn.storagePoolLookupByName(POOL)
+    except libvirt.libvirtError:
+        return
+    if pool.isActive():
+        pool.destroy()
+    pool.delete(0)
+    pool.undefine()
+
+
+def inventory(conn):
+    """Domains and volumes outside the test namespace."""
+    domains = {(d.name(), d.UUIDString()) for d in conn.listAllDomains() if not d.name().startswith(PREFIX)}
+    volumes = set()
+    for pool in conn.listAllStoragePools():
+        if pool.name() == POOL or not pool.isActive():
+            continue
+        pool.refresh(0)
+        volumes.update((pool.name(), v.name()) for v in pool.listAllVolumes())
+    return domains, volumes
+
+
+def create_volume(conn, name, size_mib=64):
+    """A qcow2 volume in the test pool; returns its path."""
+    assert name.startswith(PREFIX)
+    pool = conn.storagePoolLookupByName(POOL)
+    vol = pool.createXML(
+        f"<volume><name>{name}.qcow2</name><capacity unit='MiB'>{size_mib}</capacity>"
+        "<target><format type='qcow2'/></target></volume>",
+        0,
+    )
+    return vol.path()
+
+
+OVMF_CODE = "/usr/share/edk2/ovmf/OVMF_CODE.fd"
+
+
+def define_vm(conn, name, disk_paths, uefi=False):
+    """A small q35 VM without an OS; disks are attached as vda, vdb, ..."""
+    assert name.startswith(PREFIX)
+    disks = "".join(
+        f"<disk type='file' device='disk'><driver name='qemu' type='qcow2'/>"
+        f"<source file='{path}'/><target dev='vd{chr(ord('a') + i)}' bus='virtio'/></disk>"
+        for i, path in enumerate(disk_paths)
+    )
+    firmware = (
+        "<os><type arch='x86_64' machine='q35'>hvm</type>"
+        f"<loader readonly='yes' type='pflash'>{OVMF_CODE}</loader></os>"
+        if uefi
+        else "<os><type arch='x86_64' machine='q35'>hvm</type></os>"
+    )
+    return conn.defineXML(
+        f"<domain type='kvm'><name>{name}</name><memory unit='MiB'>128</memory><vcpu>1</vcpu>"
+        f"{firmware}<features><acpi/></features><devices>{disks}</devices></domain>"
+    )
+
+
+def volume_exists(conn, path):
+    pool = conn.storagePoolLookupByName(POOL)
+    pool.refresh(0)
+    return os.path.basename(path) in pool.listVolumes()

instances/test_live_dataloss.py

@@ -0,0 +1,115 @@
+"""Live reproductions of the Wave 1a data-loss findings.
+
+Each test asserts the safe behavior and is marked expectedFailure until
+its finding is fixed; a fix turns it into an unexpected success, which
+fails the run until the marker is removed. Runs only with
+TEST_LIBVIRT_HOST set (see instances/livetest.py).
+"""
+
+import os
+import unittest
+
+import libvirt
+from computes.models import Compute
+from django.contrib.auth import get_user_model
+from django.test import TestCase
+from django.urls import reverse
+from vrtManager.connection import connection_manager
+
+from . import livetest
+from .models import Instance
+from .utils import refr
+
+P = livetest.PREFIX
+
+
+class LiveDataLossTestCase(TestCase):
+    @classmethod
+    def setUpClass(cls):
+        if not livetest.enabled():
+            raise unittest.SkipTest("Set TEST_LIBVIRT_HOST to run the live libvirt tests")
+        cls.compute_args = dict(
+            hostname=os.environ["TEST_LIBVIRT_HOST"],
+            login=os.environ.get("TEST_LIBVIRT_LOGIN", ""),
+            password=os.environ.get("TEST_LIBVIRT_PASSWORD", ""),
+            type=int(os.environ.get("TEST_LIBVIRT_TYPE", 4)),
+        )
+        cls.conn = connection_manager.get_connection(*cls.compute_args.values())
+        livetest.cleanup(cls.conn)
+        livetest.ensure_pool(cls.conn)
+        cls.inventory = livetest.inventory(cls.conn)
+        # Last: a failure above must not leave the class transaction open,
+        # which would lock the shared test database for later test classes.
+        super().setUpClass()
+
+    @classmethod
+    def tearDownClass(cls):
+        try:
+            livetest.remove_pool(cls.conn)
+        finally:
+            super().tearDownClass()
+        if livetest.inventory(cls.conn) != cls.inventory:
+            raise AssertionError("Live tests changed objects outside the test namespace")
+
+    def setUp(self):
+        self.compute = Compute.objects.create(name="live-dataloss", details="test", **self.compute_args)
+        admin = get_user_model().objects.create_superuser("live_admin", "l@example.com", "pw")
+        self.client.force_login(admin)
+        self.client.raise_request_exception = False
+
+    def tearDown(self):
+        livetest.cleanup(self.conn)
+
+    # helpers
+
+    def vm(self, name, disks, uefi=False):
+        dom = livetest.define_vm(self.conn, P + name, disks, uefi=uefi)
+        refr(self.compute)
+        return dom, Instance.objects.get(compute=self.compute, uuid=dom.UUIDString())
+
+    def post(self, view, instance, data=None):
+        return self.client.post(
+            reverse(f"instances:{view}", args=[instance.id]),
+            data or {},
+            HTTP_REFERER=reverse("instances:instance", args=[instance.id]),
+        )
+
+    def disk_sources(self, dom, flags=0):
+        from lxml import etree
+
+        return {
+            d.find("target").get("dev"): d.find("source").get("file")
+            for d in etree.fromstring(dom.XMLDesc(flags)).findall("devices/disk")
+        }
+
+    # R-03: external snapshots
+
+    @unittest.expectedFailure
+    def test_r03_revert_external_snapshot_keeps_disk_added_later(self):
+        base = livetest.create_volume(self.conn, P + "r03-a")
+        dom, inst = self.vm("r03-a", [base])
+        self.post("create_external_snapshot", inst, {"name": "snap"})
+
+        later = livetest.create_volume(self.conn, P + "r03-later")
+        dom.attachDeviceFlags(
+            f"<disk type='file' device='disk'><driver name='qemu' type='qcow2'/>"
+            f"<source file='{later}'/><target dev='vdb' bus='virtio'/></disk>",
+            libvirt.VIR_DOMAIN_AFFECT_CONFIG,
+        )
+        self.post("revert_external_snapshot", inst, {"name": "s1.snap", "date": "", "desc": ""})
+
+        self.assertTrue(livetest.volume_exists(self.conn, later), "revert deleted a disk the snapshot never had")
+
+    @unittest.expectedFailure
+    def test_r03_delete_older_external_snapshot_keeps_newer_snapshot_files(self):
+        base = livetest.create_volume(self.conn, P + "r03-b")
+        dom, inst = self.vm("r03-b", [base])
+        self.post("create_external_snapshot", inst, {"name": "older"})
+        self.post("create_external_snapshot", inst, {"name": "newer"})
+        sources = set(self.disk_sources(dom).values())
+
+        self.post("delete_external_snapshot", inst, {"name": "s1.older"})
+
+        for path in sources:
+            self.assertTrue(livetest.volume_exists(self.conn, path), f"{path} was deleted")
+        self.assertIn("s1.newer", dom.snapshotListNames(0))

instances/tests.py

@@ -10,10 +10,10 @@ from django.contrib.auth.models import Permission
 from django.http.response import Http404
 from django.shortcuts import reverse
 from django.test import TestCase
-from libvirt import VIR_DOMAIN_UNDEFINE_NVRAM
 from vrtManager.create import wvmCreate
 from vrtManager.util import randomUUID
 
+from . import livetest
 from .models import Flavor, Instance
 from .utils import refr
 
@@ -26,6 +26,8 @@ TEST_COMPUTE_TYPE = int(os.environ.get("TEST_LIBVIRT_TYPE", 4))
 class InstancesTestCase(TestCase):
     @classmethod
     def setUpClass(cls):
+        if not livetest.enabled():
+            raise unittest.SkipTest("Set TEST_LIBVIRT_HOST to run the live libvirt tests")
         super().setUpClass()
 
         # Add users for testing purposes
@@ -60,11 +62,16 @@ class InstancesTestCase(TestCase):
                 cls.compute.type,
             )
 
+            # Remove leftovers of an interrupted run, then record what
+            # else is on the host so tearDownClass can prove it is untouched.
+            livetest.cleanup(cls.connection.wvm)
+            livetest.ensure_pool(cls.connection.wvm)
+            cls.inventory = livetest.inventory(cls.connection.wvm)
+
             # Add disks for testing
-            cls.volume_path = None
+            cls.connection.create_volume(
-            cls.volume_path = cls.connection.create_volume(
+                livetest.POOL,
-                "default",
+                "wvc-test-volume",
-                "test-volume",
                 1,
                 "qcow2",
                 False,
@@ -79,12 +86,12 @@ class InstancesTestCase(TestCase):
             cls.connection._defineXML(cls.xml)
             refr(cls.compute)
             # The compute may hold other VMs; never pick one of them.
-            cls.instance: Instance = Instance.objects.get(compute=cls.compute, name="test-vm")
+            cls.instance: Instance = Instance.objects.get(compute=cls.compute, name="wvc-test-vm")
         except Exception as e:
             cls.instance = None
-            if getattr(cls, "volume_path", None):
+            if getattr(cls, "connection", None):
                 try:
-                    cls.connection.get_volume_by_path(cls.volume_path).delete(0)
+                    livetest.remove_pool(cls.connection.wvm)
                 except Exception:
                     pass
             if getattr(cls, "compute", None):
@@ -100,16 +107,18 @@ class InstancesTestCase(TestCase):
 
     @classmethod
     def tearDownClass(cls):
-        # Destroy testing vm
+        conn = cls.connection.wvm
-        if getattr(cls, "instance", None):
+        try:
-            try:
+            livetest.remove_pool(conn)
-                cls.instance.proxy.delete_all_disks()
+        finally:
-                cls.instance.proxy.delete(VIR_DOMAIN_UNDEFINE_NVRAM)
-            except Exception:
-                pass
-        if getattr(cls, "compute", None):
             cls.compute.delete()
-        super().tearDownClass()
+            super().tearDownClass()
+        domains, volumes = livetest.inventory(conn)
+        if (domains, volumes) != cls.inventory:
+            raise AssertionError(
+                "Live tests changed objects outside the test namespace: "
+                f"domains {cls.inventory[0] ^ domains}, volumes {cls.inventory[1] ^ volumes}"
+            )
 
     def setUp(self):
         self.client.login(username="admin", password="admin")
@@ -167,16 +176,16 @@ class InstancesTestCase(TestCase):
                 "instances:create_instance", args=[self.compute.id, "x86_64", "q35"]
             ),
             {
-                "name": "test",
+                "name": "wvc-test-new",
                 "firmware": "BIOS",
                 "vcpu": 1,
                 "vcpu_mode": "host-model",
                 "memory": 128,
                 "device0": "disk",
                 "bus0": "virtio",
-                "images": "test-volume.qcow2",
+                "images": "wvc-test-volume.qcow2",
-                "storage-control": "default",
+                "storage-control": livetest.POOL,
-                "image-control": "test.qcow2",
+                "image-control": "wvc-test.qcow2",
                 "networks": "default",
                 "network-control": "default",
                 "cache_mode": "directsync",
@@ -194,7 +203,7 @@ class InstancesTestCase(TestCase):
         )
         self.assertEqual(response.status_code, 302)
 
-        instance_qs: Instance = Instance.objects.filter(name="test")
+        instance_qs: Instance = Instance.objects.filter(name="wvc-test-new")
         self.assertEqual(len(instance_qs), 1)
 
         instance = instance_qs[0]
@@ -212,7 +221,7 @@ class InstancesTestCase(TestCase):
 
     def test_create_from_xml(self):
         uuid = randomUUID()
-        xml = self.xml.replace("test-vm", "test-vm-xml")
+        xml = self.xml.replace("wvc-test-vm", "wvc-test-vm-xml")
         xml = re.sub(r"\s?<uuid>.*?</uuid>", f"<uuid>{uuid}</uuid>", xml)
         response = self.client.post(
             reverse("instances:create_instance_select_type", args=[self.compute.id]),
@@ -223,7 +232,7 @@ class InstancesTestCase(TestCase):
         )
         self.assertEqual(response.status_code, 302)
 
-        xml_instance_qs: Instance = Instance.objects.filter(name="test-vm-xml")
+        xml_instance_qs: Instance = Instance.objects.filter(name="wvc-test-vm-xml")
         self.assertEqual(len(xml_instance_qs), 1)
 
         xml_instance = xml_instance_qs[0]
@@ -391,8 +400,8 @@ class InstancesTestCase(TestCase):
         response = self.client.post(
             reverse("instances:add_new_vol", args=[self.instance.id]),
             {
-                "storage": "default",
+                "storage": livetest.POOL,
-                "name": "test-volume-2",
+                "name": "wvc-test-volume-2",
                 "size": 1,
             },
             HTTP_REFERER=reverse("index"),
@@ -405,9 +414,9 @@ class InstancesTestCase(TestCase):
         response = self.client.post(
             reverse("instances:delete_vol", args=[self.instance.id]),
             {
-                "storage": "default",
+                "storage": livetest.POOL,
                 "dev": "vdb",
-                "name": "test-volume-2.qcow2",
+                "name": "wvc-test-volume-2.qcow2",
             },
             HTTP_REFERER=reverse("index"),
         )
@@ -434,8 +443,8 @@ class InstancesTestCase(TestCase):
         response = self.client.post(
             reverse("instances:add_existing_vol", args=[self.instance.id]),
             {
-                "selected_storage": "default",
+                "selected_storage": livetest.POOL,
-                "vols": "test-volume.qcow2",
+                "vols": "wvc-test-volume.qcow2",
             },
             HTTP_REFERER=reverse("index"),
         )
@@ -448,7 +457,7 @@ class InstancesTestCase(TestCase):
         response = self.client.post(
             reverse("instances:edit_volume", args=[self.instance.id]),
             {
-                "vol_path": "/var/lib/libvirt/images/test-volume.qcow2",
+                "vol_path": f"{livetest.POOL_PATH}/wvc-test-volume.qcow2",
                 # 'vol_shareable': False,
                 # 'vol_readonly': False,
                 "vol_bus": "virtio",
@@ -657,9 +666,9 @@ class InstancesTestCase(TestCase):
         response = self.client.post(
             reverse("instances:clone", args=[self.instance.id]),
             {
-                "name": "test-vm-clone",
+                "name": "wvc-test-vm-clone",
                 "clone-net-mac-0": "de:ad:be:ef:de:ad",
-                "disk-vda": "test-clone.img",
+                "disk-vda": "wvc-test-clone.img",
                 "clone-title": "",
                 "clone-description": "",
                 "clone": "",
@@ -669,7 +678,7 @@ class InstancesTestCase(TestCase):
         self.assertEqual(response.status_code, 302)
         self.assertEqual(Instance.objects.count(), instance_count + 1)
 
-        clone_qs = Instance.objects.filter(name="test-vm-clone")
+        clone_qs = Instance.objects.filter(name="wvc-test-vm-clone")
         self.assertEqual(len(clone_qs), 1)
         clone = clone_qs[0]
 
@@ -704,9 +713,9 @@ class InstancesTestCase(TestCase):
         response = self.client.post(
             reverse("instances:clone", args=[self.instance.id]),
             {
-                "name": "test-vm-clone",
+                "name": "wvc-test-vm-clone",
                 "clone-net-mac-0": "de:ad:be:ef:de:ad",
-                "disk-vda": "test-clone.img",
+                "disk-vda": "wvc-test-clone.img",
                 "clone-title": "",
                 "clone-description": "",
                 "clone": "",
@@ -724,9 +733,9 @@ class InstancesTestCase(TestCase):
         response = self.client.post(
             reverse("instances:clone", args=[self.instance.id]),
             {
-                "name": "test-vm",
+                "name": "wvc-test-vm",
                 "clone-net-mac-0": "de:ad:be:ef:de:ad",
-                "disk-vda": "test.img",
+                "disk-vda": "wvc-test.img",
                 "clone-title": "",
                 "clone-description": "",
                 "clone": "",
@@ -756,9 +765,9 @@ class InstancesTestCase(TestCase):
         response = self.client.post(
             reverse("instances:clone", args=[self.instance.id]),
             {
-                "name": "test-vm-clone",
+                "name": "wvc-test-vm-clone",
                 "clone-net-mac-0": "gh:ad:be:ef:de:ad",
-                "disk-vda": "test-clone.img",
+                "disk-vda": "wvc-test-clone.img",
                 "clone-title": "",
                 "clone-description": "",
                 "clone": "",