fix(interfaces): no create, start, stop or delete where libvirt cannot change interfaces

libvirt's udev interface backend (RHEL 9 and later, Debian 12) only reads
interfaces, so the create form and the start, stop and delete buttons
only failed there. The pages ask the host with an invalid definition,
which defines nothing: a backend without define refuses it as not
supported. There they show the interface state and a note instead, and
posts change nothing; the host's devices, needed only by the create
form, are not read. Any other answer keeps the pages as they were.

README.md

@@ -539,6 +539,7 @@ A rollback leaves the clone on a detached HEAD at the restored commit; the updat
 - 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.
 - A VM's stats are polled every 5 s like the host graph, the first point as soon as the tab opens; each point is the average since the previous one, disk rates in MB/s (they were MiB/s labelled Mb/s); a counter the host does not report shows a gap instead of 0.
 - An ISO uploads to an SSH compute about 3.5 times faster (SFTP writes no longer wait for each 32 KiB); a failed write still fails the upload.
+- On a host whose libvirt cannot change network interfaces (the udev backend: RHEL 9 and later, Debian 12), the interface pages offer no create, start, stop or delete; they only failed there.
 - The VM page shows each NIC's own QoS and each CD-ROM's own pool (they showed the previous device's), and a NIC without a source or model no longer breaks it.
 - A volume of an LVM or iSCSI pool can be attached to a VM, and bridge and direct NICs can be boot devices. The NIC model `default` lets libvirt choose, and `rt18139` is `rtl8139`: a VM with either did not start; a migration corrects the default NIC type setting.
 - A paused VM's memory is resized as a running VM's: the current memory changes and the guest takes it once resumed, the maximum stays (both only reached the VM at its next boot).

interfaces/templates/interface.html

@@ -35,6 +35,7 @@
         </div>
     </div>
 
+    {% if changeable is False %}{% include 'interfaces_unchangeable.html' %}{% endif %}
     <div class="row">
         <dl class="ms-3 row">
             <dt class="col-4">{% trans "Interface" %}</dt>
@@ -55,6 +56,9 @@
             <dd class="col-8">{{ start_mode }}</dd>
             <dt class="col-4">{% trans "State" %}</dt>
             <dd class="col-8">
+                {% if changeable is False %}
+                    {% if state == 0 %}{% trans "Inactive" %}{% else %}{% trans "Active" %}{% endif %}
+                {% else %}
                 <form action="" method="post" role="form" aria-label="Interface start/stop/delete form">{% csrf_token %}
                     {% if state == 0 %}
                         <input type="submit" class="btn btn-sm btn-primary" name="start" value="{% trans "Start" %}">
@@ -63,6 +67,7 @@
                         <input type="submit" class="btn btn-sm btn-secondary" name="stop" value="{% trans "Stop" %}" onclick="return confirm('{% trans "Are you sure?" %}')">
                     {% endif %}
                 </form>
+                {% endif %}
             </dd>
         </dl>
 

interfaces/templates/interfaces.html

@@ -8,7 +8,7 @@
 {% block page_heading %}{{ compute.name }} - {% trans "Interfaces" %}{% endblock page_heading %}
 
 {% block page_heading_extra %}
-{% include 'create_iface_block.html' %}
+{% if changeable %}{% include 'create_iface_block.html' %}{% endif %}
 {% endblock page_heading_extra %}
 
 {% block content %}
@@ -42,6 +42,7 @@
         </div>
     </div>
 
+    {% if changeable is False %}{% include 'interfaces_unchangeable.html' %}{% endif %}
     <div class="row">
         {% if not  ifaces_all %}
             <div class="col-lg-12">

interfaces/templates/interfaces_unchangeable.html

@@ -0,0 +1,5 @@
+{% load i18n %}
+{% load bootstrap_icons %}
+<div class="alert alert-info shadow-sm">
+    {% bs_icon 'info-circle' %} {% trans "This host's libvirt does not support changing network interfaces. Configure them directly on the host, for example with NetworkManager." %}
+</div>

interfaces/test_forms.py

@@ -8,7 +8,7 @@ from computes.models import Compute
 from django.contrib.auth import get_user_model
 from django.test import SimpleTestCase, TestCase
 from django.urls import reverse
-from lxml import etree
+from lxml import etree, html
 from vrtManager.interface import wvmInterfaces
 
 from interfaces.forms import AddInterface
@@ -295,6 +295,7 @@ class CreateViewTestCase(TestCase):
             with self.subTest(netdev=netdev), patch("interfaces.views.wvmInterfaces") as conn_cls:
                 conn = conn_cls.return_value
                 conn.get_ifaces.return_value = []
+                conn.can_change_interfaces.return_value = True
                 conn.get_net_devices.return_value = ["eth0", "eth1"]
                 self.client.post(
                     reverse("interfaces", args=[compute.id]),
@@ -309,6 +310,7 @@ class CreateViewTestCase(TestCase):
         with patch("interfaces.views.wvmInterfaces") as conn_cls:
             conn = conn_cls.return_value
             conn.get_ifaces.return_value = []
+            conn.can_change_interfaces.return_value = True
             conn.get_net_devices.side_effect = RuntimeError("nodedev driver not running")
             response = self.client.post(
                 reverse("interfaces", args=[compute.id]),
@@ -320,6 +322,78 @@ class CreateViewTestCase(TestCase):
         self.assertFalse(any("not on this host" in m for m in messages), messages)
 
 
+class UnchangeableHostTestCase(TestCase):
+    """A host whose libvirt cannot change interfaces (udev backend) gets no
+    create form and no start/stop/delete, only a note; posts change nothing."""
+
+    NOTE = "does not support changing network interfaces"
+
+    def setUp(self):
+        self.client.force_login(get_user_model().objects.create_superuser("iface_ro", "ro@example.com", "x"))
+        self.compute = Compute.objects.create(name="iface-ro", hostname="127.0.0.1:1", login="root", password="", type=1)
+
+    def interfaces_page(self, changeable, post=None):
+        with patch("interfaces.views.wvmInterfaces") as conn_cls:
+            conn = conn_cls.return_value
+            conn.get_ifaces.return_value = []
+            conn.can_change_interfaces.return_value = changeable
+            conn.get_net_devices.return_value = ["eth0", "eth1"]
+            url = reverse("interfaces", args=[self.compute.id])
+            response = self.client.post(url, post) if post else self.client.get(url)
+        return conn, response.content.decode()
+
+    def test_the_interfaces_page_offers_create_only_where_it_works(self):
+        for changeable in (True, False):
+            with self.subTest(changeable=changeable):
+                conn, page = self.interfaces_page(changeable)
+                self.assertEqual("#AddInterface" in page, changeable)
+                self.assertEqual(self.NOTE in page, not changeable)
+                # the devices are only the form's choices
+                self.assertEqual(conn.get_net_devices.called, changeable)
+
+    def test_a_create_post_creates_nothing(self):
+        conn, page = self.interfaces_page(False, {**BASE, "netdev": "eth1", "ipv6_type": "none", "create": ""})
+        conn.create_iface.assert_not_called()
+        self.assertIn(self.NOTE, page)
+
+    def interface_page(self, changeable, state, post=None):
+        with patch("interfaces.views.wvmInterface") as conn_cls:
+            conn = conn_cls.return_value
+            conn.can_change_interfaces.return_value = changeable
+            conn.is_active.return_value = state
+            conn.get_bridge_slave_ifaces.return_value = []
+            url = reverse("interface", args=[self.compute.id, "br0"])
+            response = self.client.post(url, post) if post else self.client.get(url)
+        return conn, response
+
+    def test_the_interface_page_shows_the_state_without_actions(self):
+        for state, shown in ((1, "Active"), (0, "Inactive")):
+            with self.subTest(state=state):
+                _conn, response = self.interface_page(False, state)
+                page = response.content.decode()
+                self.assertNotIn('name="start"', page)
+                self.assertNotIn('name="stop"', page)
+                state_cell = html.fromstring(page).xpath("//dt[normalize-space()='State']/following-sibling::dd[1]")[0]
+                self.assertEqual(state_cell.text_content().strip(), shown)
+                self.assertIn(self.NOTE, page)
+        _conn, response = self.interface_page(True, 0)
+        self.assertIn('name="start"', response.content.decode())
+
+    def test_a_host_that_could_not_be_read_keeps_the_page_as_it_was(self):
+        with patch("interfaces.views.wvmInterface", side_effect=libvirt.libvirtError("unreachable")):
+            page = self.client.get(reverse("interface", args=[self.compute.id, "br0"])).content.decode()
+        self.assertNotIn(self.NOTE, page)
+        self.assertIn("Interface start/stop/delete form", page)
+
+    def test_start_stop_and_delete_posts_change_nothing(self):
+        for action in ("start", "stop", "delete"):
+            with self.subTest(action=action):
+                conn, response = self.interface_page(False, 1, {action: ""})
+                self.assertEqual(response.status_code, 200)
+                for method in (conn.start_iface, conn.stop_iface, conn.delete_iface):
+                    method.assert_not_called()
+
+
 class EthernetDeviceTestCase(SimpleTestCase):
     def test_an_ethernet_interface_needs_no_bridge_member(self):
         self.assertTrue(form(itype="ethernet", name="eth0", netdev="", ipv6_type="none").is_valid())

interfaces/test_live_interfaces.py

@@ -0,0 +1,42 @@
+"""Live check that asking a host whether it can change interfaces defines
+nothing, and that the interfaces page follows the answer. Runs only with
+TEST_LIBVIRT_HOST set; TEST_LIBVIRT_SSH_HOST/_LOGIN add an ssh compute."""
+
+import os
+import unittest
+
+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 wvmConnect
+
+
+@unittest.skipUnless(os.environ.get("TEST_LIBVIRT_HOST"), "Set TEST_LIBVIRT_HOST to run the live libvirt tests")
+class LiveInterfacesTestCase(TestCase):
+    def hosts(self):
+        yield "tcp", (
+            os.environ["TEST_LIBVIRT_HOST"],
+            os.environ.get("TEST_LIBVIRT_LOGIN", ""),
+            os.environ.get("TEST_LIBVIRT_PASSWORD", ""),
+            int(os.environ.get("TEST_LIBVIRT_TYPE", 1)),
+        )
+        if os.environ.get("TEST_LIBVIRT_SSH_HOST"):
+            yield "ssh", (os.environ["TEST_LIBVIRT_SSH_HOST"], os.environ.get("TEST_LIBVIRT_SSH_LOGIN", "root"), "", 2)
+
+    def test_the_question_defines_nothing_and_the_page_follows_the_answer(self):
+        self.client.force_login(get_user_model().objects.create_superuser("live-iface", "li@example.com", "x"))
+        for name, args in self.hosts():
+            with self.subTest(name):
+                conn = wvmConnect(*args)
+                before = sorted(i.XMLDesc(0) for i in conn.wvm.listAllInterfaces(0))
+                changeable = conn.can_change_interfaces()
+                self.assertIsInstance(changeable, bool)
+                self.assertEqual(sorted(i.XMLDesc(0) for i in conn.wvm.listAllInterfaces(0)), before)
+                hostname, login, password, conn_type = args
+                compute = Compute.objects.create(
+                    name=f"live-{name}", hostname=hostname, login=login, password=password, type=conn_type
+                )
+                page = self.client.get(reverse("interfaces", args=[compute.id])).content.decode()
+                self.assertEqual("#AddInterface" in page, changeable)
+                self.assertEqual("does not support changing network interfaces" in page, not changeable)

interfaces/views.py

@@ -29,13 +29,16 @@ def interfaces(request, compute_id):
             compute.type
         )
         ifaces = conn.get_ifaces()
+        changeable = conn.can_change_interfaces()
 
+        # the host's devices are only the create form's choices
         netdevs, netdev_error = [], None
-        try:
-            netdevs = conn.get_net_devices()
-        except Exception as err:
-            netdev_error = err
-            messages.error(request, _("The network devices of this host could not be read: %(error)s") % {"error": err})
+        if changeable:
+            try:
+                netdevs = conn.get_net_devices()
+            except Exception as err:
+                netdev_error = err
+                messages.error(request, _("The network devices of this host could not be read: %(error)s") % {"error": err})
 
         for iface in ifaces:
             interf = wvmInterface(
@@ -48,8 +51,9 @@ def interfaces(request, compute_id):
             ifaces_all.append(interf.get_details())
 
         if request.method == "POST":
-            # without the host's devices nothing can be checked, so nothing is created
-            if "create" in request.POST and netdev_error is None:
+            # the page says why nothing is created on a host that cannot change
+            # interfaces; without the host's devices nothing can be checked
+            if "create" in request.POST and changeable and netdev_error is None:
                 form = AddInterface(request.POST, netdevs=netdevs)
                 if form.is_valid():
                     data = form.cleaned_data
@@ -108,8 +112,10 @@ def interface(request, compute_id, iface):
         ipv6_type = conn.get_ipv6_type()
         bridge = conn.get_bridge()
         slave_ifaces = conn.get_bridge_slave_ifaces()
+        changeable = conn.can_change_interfaces()
 
-        if request.method == "POST":
+        # the page says why nothing is done on a host that cannot change interfaces
+        if request.method == "POST" and changeable:
             if "stop" in request.POST:
                 conn.stop_iface()
                 return HttpResponseRedirect(request.get_full_path())

vrtManager/connection.py

@@ -958,6 +958,17 @@ class wvmConnect(object):
                 instance.append(dom.name())
         return instance
 
+    def can_change_interfaces(self):
+        """False when the host's libvirt cannot define interfaces (the udev
+        backend only reads them). The definition sent is invalid, so nothing is
+        defined: a backend without define refuses it with VIR_ERR_NO_SUPPORT,
+        one with define with an XML error."""
+        try:
+            self.wvm.interfaceDefineXML("<interface/>", 0)
+        except libvirtError as err:
+            return err.get_error_code() != libvirt.VIR_ERR_NO_SUPPORT
+        return True
+
     def get_net_devices(self):
         """The host's network interface names; libvirt lists only the network devices"""
         return [

vrtManager/test_host_reads.py

@@ -2,7 +2,8 @@
 capabilities once per emulator, arch, machine and domain type: the create
 page's getters parse the capabilities some 60 times, the overview reads the
 info four times. An object lives for one request. The interfaces page lists
-only the host's network devices, not all of them."""
+only the host's network devices, not all of them, and knows whether the host
+can change interfaces at all."""
 
 import unittest
 from unittest.mock import MagicMock
@@ -12,7 +13,7 @@ from django.conf import settings
 if not settings.configured:
     settings.configure(MAC_OUI="52:54:10")
 
-from libvirt import VIR_CONNECT_LIST_NODE_DEVICES_CAP_NET
+from libvirt import VIR_CONNECT_LIST_NODE_DEVICES_CAP_NET, VIR_ERR_NO_SUPPORT, VIR_ERR_OPERATION_DENIED, VIR_ERR_XML_ERROR, libvirtError
 from vrtManager.connection import wvmConnect
 from vrtManager.hostdetails import wvmHostDetails
 
@@ -114,3 +115,20 @@ class NetDevicesTestCase(unittest.TestCase):
         }
         conn.wvm.listAllDevices = lambda flags: devices.get(flags, [])
         self.assertEqual(conn.get_net_devices(), ["eth0", "br0"])
+
+
+class ChangeInterfacesTestCase(unittest.TestCase):
+    def error(self, code):
+        err = libvirtError("refused")
+        err.get_error_code = lambda: code
+        return err
+
+    def test_a_backend_without_define_cannot_change_interfaces(self):
+        conn = connection()
+        # any other refusal (access denied, ...) leaves the page as it was
+        for code, changeable in ((VIR_ERR_NO_SUPPORT, False), (VIR_ERR_XML_ERROR, True), (VIR_ERR_OPERATION_DENIED, True)):
+            with self.subTest(code=code):
+                conn.wvm.interfaceDefineXML = MagicMock(side_effect=self.error(code))
+                self.assertIs(conn.can_change_interfaces(), changeable)
+                # an invalid definition: nothing can be defined by asking
+                conn.wvm.interfaceDefineXML.assert_called_once_with("<interface/>", 0)