fix(instances): a disk on an LVM pool is not resized, the page says so instead of failing

libvirt's logical pool cannot change a volume's capacity, so every
resize of such a disk ended in a 500. Disks are resized one at a time;
VIR_ERR_NO_SUPPORT becomes a message naming the disk, and the other
disks still grow.

A live test creates an LVM pool from the storage page on a spare disk
(TEST_LIBVIRT_LVM_DEVICE; the Vagrant computes get /dev/vdb), attaches
new and existing volumes to stopped and running VMs as block disks, and
checks the refused resize in every state.

README.md

@@ -569,7 +569,7 @@ A version from before v3.0.0 has no worker: `systemctl disable --now webvirt-wor
 - A template VM is changed or deleted only by superusers and staff owners with the matching permission; other owners can still view, open the console and clone it. Power cycle no longer starts a template.
 - A VM that vanishes from its compute keeps its owners for `INSTANCE_OWNERSHIP_RETENTION_DAYS` (30) in case the same UUID comes back; superusers list and remove these records on the "Removed VMs" admin page.
 - The log rotates (10 MB × 5) in `data/webvirtcloud.log`.
-- A disk can be grown while its VM runs or is paused, not only when it is shut off.
+- A disk can be grown while its VM runs or is paused, not only when it is shut off. A disk on an LVM pool cannot be grown (libvirt cannot resize its volumes): the resize says so instead of failing, and the VM's other disks still grow.
 - Enabling vCPU hot plug keeps the VM's current vCPU count (it enabled all of them up to the maximum).
 - A host's CPU graph shows the average since its previous point (normally 5 s) instead of a 1 s sample taken while a worker waits.
 - The CPU usage in the new-VM dialog is the current load (over 1 s), read when the dialog opens; it was the average since the host booted.
@@ -690,19 +690,20 @@ The compute tests try a local libvirt and skip when none answers; the instance t
 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
 export TEST_LIBVIRT_GUEST_IMAGE=/var/lib/libvirt/images/linux.qcow2   # optional, see below
+export TEST_LIBVIRT_LVM_DEVICE=/dev/vdb   # optional: an empty disk of 4 GiB or more on the host, wiped by the LVM tests
 python manage.py test
 ```
 
 `TEST_LIBVIRT_GUEST_IMAGE` enables the tests that need a guest OS (disk and NIC hot-unplug, vCPU hotplug, guest agent, ACPI power off). It must be an installed, BIOS-bootable Linux qcow2 image with qemu-guest-agent, in a storage pool of the host other than `wvc-test`; the tests boot copies of it from overlays and only read the image. Shut down any VM using it first.
 
-Two disposable compute nodes for these tests come from the `Vagrantfile`: AlmaLinux 10 with nested KVM, set up by `dev/libvirt-bootstrap.sh` (with this checkout's gstfsd), a SASL user for TCP (`wvc`, password `wvc-test`) and your `~/.ssh/id_ed25519.pub` for root; migration runs between them. It needs Vagrant with the `vagrant-libvirt` plugin, a KVM host with `qemu-kvm` and `cyrus-sasl-md5`, and a user in the `libvirt` group. On a host that also runs Docker, let the VMs' network (the bridge of `virsh net-info vagrant-libvirt`, `virbr1` here) through Docker's forward rules (`sudo iptables -I DOCKER-USER -i virbr1 -j ACCEPT` and `sudo iptables -I DOCKER-USER -o virbr1 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT`).
+Two disposable compute nodes for these tests come from the `Vagrantfile`: AlmaLinux 10 with nested KVM, set up by `dev/libvirt-bootstrap.sh` (with this checkout's gstfsd), a SASL user for TCP (`wvc`, password `wvc-test`) and your `~/.ssh/id_ed25519.pub` for root; migration runs between them, and each has an empty second disk (`/dev/vdb`) for the LVM tests. It needs Vagrant with the `vagrant-libvirt` plugin, a KVM host with `qemu-kvm` and `cyrus-sasl-md5`, and a user in the `libvirt` group. On a host that also runs Docker, let the VMs' network (the bridge of `virsh net-info vagrant-libvirt`, `virbr1` here) through Docker's forward rules (`sudo iptables -I DOCKER-USER -i virbr1 -j ACCEPT` and `sudo iptables -I DOCKER-USER -o virbr1 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT`).
 
 ```bash
 vagrant up --provider=libvirt
 C1=$(vagrant ssh compute1 -c "hostname -I" | awk '{print $1}') C2=$(vagrant ssh compute2 -c "hostname -I" | awk '{print $1}')
 for h in $C1 $C2; do ssh-keygen -R $h; ssh -o StrictHostKeyChecking=accept-new root@$h true; done   # new VMs, new host keys
 export TEST_LIBVIRT_HOST=$C1 TEST_LIBVIRT_TYPE=1 TEST_LIBVIRT_LOGIN=wvc TEST_LIBVIRT_PASSWORD=wvc-test
-export TEST_LIBVIRT_SSH_HOST=$C2 TEST_LIBVIRT_SSH_LOGIN=root TEST_LIBVIRT_MIGRATE_ADDRESS=$C2
+export TEST_LIBVIRT_SSH_HOST=$C2 TEST_LIBVIRT_SSH_LOGIN=root TEST_LIBVIRT_MIGRATE_ADDRESS=$C2 TEST_LIBVIRT_LVM_DEVICE=/dev/vdb
 python manage.py test
 vagrant destroy -f
 ```

Vagrantfile

@@ -1,7 +1,8 @@
 # Two disposable compute nodes for the live tests (README: Running tests), so
 # that migration runs between them: AlmaLinux 10 with nested KVM, set up by
 # dev/libvirt-bootstrap.sh, with a SASL user for TCP and the host user's SSH
-# key for root (SASL user wvc, password wvc-test).
+# key for root (SASL user wvc, password wvc-test). The empty second disk
+# /dev/vdb is for the LVM tests (TEST_LIBVIRT_LVM_DEVICE).
 
 Vagrant.configure(2) do |config|
   config.vm.box = "almalinux/10"
@@ -15,6 +16,7 @@ Vagrant.configure(2) do |config|
     v.nested = true
     v.cpu_mode = "host-passthrough"
     v.video_type = "virtio"  # EL10 hosts have no cirrus
+    v.storage :file, size: "4G"
   end
   # the bootstrap installs this checkout's gstfsd
   config.vm.provision "file", source: "dev", destination: "/tmp/wvc/dev"

instances/tests/test_live_lvm.py

@@ -0,0 +1,173 @@
+"""Live checks of VM disks on an LVM pool, created on the storage page.
+
+Volumes of a logical pool are block devices: they attach as <disk
+type='block'> with a raw driver. The pool cannot resize them, so a disk
+resize is refused with a message. Runs only with TEST_LIBVIRT_HOST
+and TEST_LIBVIRT_LVM_DEVICE set; the device, an empty disk of at least
+4 GiB on that host, is wiped (the Vagrant computes' /dev/vdb).
+"""
+
+import os
+import unittest
+
+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 lxml import etree
+from vrtManager.connection import connection_manager
+
+from . import livetest
+from instances.models import Instance
+from instances.utils import refr
+
+P = livetest.PREFIX
+POOL = P + "lvm"
+GIB = 1 << 30
+# a running q35 VM hotplugs each disk into a free root port; the USB
+# controller and the balloon take two
+PCI = "<controller type='pci' index='0' model='pcie-root'/>" + "<controller type='pci' model='pcie-root-port'/>" * 4
+
+
+class LiveLvmTestCase(TestCase):
+    @classmethod
+    def setUpClass(cls):
+        if not (livetest.enabled() and os.environ.get("TEST_LIBVIRT_LVM_DEVICE")):
+            raise unittest.SkipTest("Set TEST_LIBVIRT_HOST and TEST_LIBVIRT_LVM_DEVICE to run the LVM 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)
+        cls.inventory = livetest.inventory(cls.conn)
+        super().setUpClass()
+
+    @classmethod
+    def tearDownClass(cls):
+        try:
+            livetest.cleanup(cls.conn)
+            try:
+                pool = cls.conn.storagePoolLookupByName(POOL)
+            except libvirt.libvirtError:
+                pass
+            else:
+                for vol in pool.listAllVolumes():
+                    vol.delete(0)
+                pool.destroy()
+                pool.delete(0)  # removes the volume group
+                pool.undefine()
+        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-lvm", details="test", **self.compute_args)
+        admin = get_user_model().objects.create_superuser("live_admin", "l@example.com", "pw")
+        self.client.force_login(admin)
+        self.pool = self.lvm_pool()
+
+    def tearDown(self):
+        self.clear()
+
+    def clear(self):
+        livetest.cleanup(self.conn)
+        for vol in self.pool.listAllVolumes():
+            vol.delete(0)
+
+    def lvm_pool(self):
+        try:
+            return self.conn.storagePoolLookupByName(POOL)
+        except libvirt.libvirtError:
+            self.client.post(
+                reverse("storages", args=[self.compute.id]),
+                {"create": "", "name": POOL, "stg_type": "logical", "source": os.environ["TEST_LIBVIRT_LVM_DEVICE"]},
+            )
+        pool = self.conn.storagePoolLookupByName(POOL)
+        self.assertTrue(pool.isActive())
+        self.assertEqual(etree.fromstring(pool.XMLDesc(0)).findtext("target/path"), f"/dev/{POOL}")
+        return pool
+
+    def vm(self, name, disk=None):
+        disks = (
+            "<disk type='block' device='disk'><driver name='qemu' type='raw'/>"
+            f"<source dev='{disk}'/><target dev='vda' bus='virtio'/></disk>"
+            if disk
+            else ""
+        )
+        dom = self.conn.defineXML(
+            f"<domain type='kvm'><name>{P}{name}</name><memory unit='MiB'>128</memory><vcpu>1</vcpu>"
+            "<os><type arch='x86_64' machine='q35'>hvm</type></os><features><acpi/></features>"
+            f"<devices>{disks}{PCI}</devices></domain>"
+        )
+        refr(self.compute)
+        return dom, Instance.objects.get(compute=self.compute, uuid=dom.UUIDString())
+
+    def lv(self, name, gib=1):
+        return self.pool.createXML(
+            f"<volume><name>{P}{name}</name><capacity unit='GiB'>{gib}</capacity></volume>", 0
+        ).path()
+
+    def post(self, view, instance, data):
+        return self.client.post(
+            reverse(f"instances:{view}", args=[instance.id]),
+            data,
+            HTTP_REFERER=reverse("instances:instance", args=[instance.id]),
+        )
+
+    def disks(self, dom, flags=0):
+        return {
+            d.find("target").get("dev"): (
+                d.get("type"), d.find("driver").get("type"), d.find("source").get("dev")
+            )
+            for d in etree.fromstring(dom.XMLDesc(flags)).findall("devices/disk")
+        }
+
+    def test_new_and_existing_volumes_attach_as_block_disks(self):
+        for running in (False, True):
+            with self.subTest(running=running):
+                dom, inst = self.vm(f"attach-{int(running)}")
+                if running:
+                    dom.create()
+                existing = self.lv(f"existing-{int(running)}")
+
+                self.post("add_new_vol", inst, {"storage": POOL, "name": f"{P}new-{int(running)}",
+                                                "size": "1", "format": "qcow2", "bus": "virtio", "cache": "default"})
+                self.post("add_existing_vol", inst, {"selected_storage": POOL, "vols": f"{P}existing-{int(running)}",
+                                                     "bus": "virtio", "cache": "default"})
+
+                expected = {
+                    "vda": ("block", "raw", f"/dev/{POOL}/{P}new-{int(running)}"),
+                    "vdb": ("block", "raw", existing),
+                }
+                self.assertEqual(self.disks(dom, libvirt.VIR_DOMAIN_XML_INACTIVE), expected)
+                self.assertEqual(self.disks(dom), expected)
+                self.clear()
+
+    def test_a_disk_resize_is_refused_with_a_message_and_changes_nothing(self):
+        # libvirt's LVM pool cannot change a volume's capacity
+        for state in ("stopped", "running", "paused"):
+            with self.subTest(state=state):
+                path = self.lv(f"resize-{state}")
+                dom, inst = self.vm(f"resize-{state}", path)
+                if state != "stopped":
+                    dom.create()
+                if state == "paused":
+                    dom.suspend()
+
+                response = self.post("resize_disk", inst, {"disk_size_vda": "2"})
+
+                self.assertRedirects(response, reverse("instances:instance", args=[inst.id]) + "#resize",
+                                     fetch_redirect_response=False)
+                shown = [str(m) for m in get_messages(response.wsgi_request)]
+                self.assertEqual(shown[-1], "The storage pool of vda cannot resize its volumes")
+                self.assertNotIn("Disk is resized: vda", shown)
+                self.assertEqual(self.conn.storageVolLookupByPath(path).info()[1], GIB)
+                if state != "stopped":
+                    self.assertEqual(dom.blockInfo("vda")[0], GIB)  # what the guest sees
+                self.clear()

instances/views.py

@@ -35,6 +35,7 @@ from libvirt import (VIR_DOMAIN_UNDEFINE_KEEP_NVRAM,
                      VIR_DOMAIN_UNDEFINE_MANAGED_SAVE,
                      VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA,
                      VIR_DOMAIN_UNDEFINE_NVRAM,
+                     VIR_ERR_NO_SUPPORT,
                      libvirtError)
 from logs.views import addlogmsg
 from operations import claims, jobs
@@ -960,13 +961,20 @@ def resize_disk(request, pk):
         })
         messages.error(request, msg)
     else:
-        instance.proxy.resize_disk(disks_new)
-        devs_str = ", ".join([d["dev"] for d in disks_new])
-        msg = _("Disk is resized: %(dev)s") % {"dev": devs_str}
-        addlogmsg(
-            request.user.username, instance.compute.name, instance.name, msg
-        )
-        messages.success(request, msg)
+        for disk in disks_new:
+            try:
+                instance.proxy.resize_disk([disk])
+            except libvirtError as err:
+                # an LVM pool, for one, cannot resize its volumes
+                if err.get_error_code() != VIR_ERR_NO_SUPPORT:
+                    raise
+                messages.error(request, _("The storage pool of %(dev)s cannot resize its volumes") % {"dev": disk["dev"]})
+                continue
+            msg = _("Disk is resized: %(dev)s") % {"dev": disk["dev"]}
+            addlogmsg(
+                request.user.username, instance.compute.name, instance.name, msg
+            )
+            messages.success(request, msg)
 
     return redirect(reverse("instances:instance", args=[instance.id]) + "#resize")
 

vrtManager/instance.py

@@ -1495,8 +1495,9 @@ class wvmInstance(wvmConnect):
 
     def resize_disk(self, disks):
         """Grow disks. QEMU holds the lock of a running domain's image files and
-        grows them itself; block volumes (e.g. LVM) are grown by the storage
-        pool first, then QEMU is told the new size."""
+        grows them itself; other volumes are grown by the storage pool first,
+        then QEMU is told the new size. A pool that cannot resize volumes (LVM
+        for one) raises VIR_ERR_NO_SUPPORT before anything changes."""
         active = self.instance.isActive()
         for disk in disks:
             vol = self.get_volume_by_path(disk["path"])