diff --git a/README.md b/README.md index d0c310ff..aacb4aa8 100644 --- a/README.md +++ b/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). diff --git a/interfaces/templates/interface.html b/interfaces/templates/interface.html index 256c33e4..0513e4be 100644 --- a/interfaces/templates/interface.html +++ b/interfaces/templates/interface.html @@ -35,6 +35,7 @@ + {% if changeable is False %}{% include 'interfaces_unchangeable.html' %}{% endif %}
{% trans "Interface" %}
@@ -55,6 +56,9 @@
{{ start_mode }}
{% trans "State" %}
+ {% if changeable is False %} + {% if state == 0 %}{% trans "Inactive" %}{% else %}{% trans "Active" %}{% endif %} + {% else %}
{% csrf_token %} {% if state == 0 %} @@ -63,6 +67,7 @@ {% endif %}
+ {% endif %}
diff --git a/interfaces/templates/interfaces.html b/interfaces/templates/interfaces.html index d7ac502c..f5913b6b 100644 --- a/interfaces/templates/interfaces.html +++ b/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 @@
+ {% if changeable is False %}{% include 'interfaces_unchangeable.html' %}{% endif %}
{% if not ifaces_all %}
diff --git a/interfaces/templates/interfaces_unchangeable.html b/interfaces/templates/interfaces_unchangeable.html new file mode 100644 index 00000000..5377657a --- /dev/null +++ b/interfaces/templates/interfaces_unchangeable.html @@ -0,0 +1,5 @@ +{% load i18n %} +{% load bootstrap_icons %} +
+ {% 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." %} +
diff --git a/interfaces/test_forms.py b/interfaces/test_forms.py index 0c42a236..bfdc7341 100644 --- a/interfaces/test_forms.py +++ b/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()) diff --git a/interfaces/test_live_interfaces.py b/interfaces/test_live_interfaces.py new file mode 100644 index 00000000..0a7efe51 --- /dev/null +++ b/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) diff --git a/interfaces/views.py b/interfaces/views.py index f89a91b7..9397d99a 100644 --- a/interfaces/views.py +++ b/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()) diff --git a/vrtManager/connection.py b/vrtManager/connection.py index d340432d..e4b9c35b 100644 --- a/vrtManager/connection.py +++ b/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("", 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 [ diff --git a/vrtManager/test_host_reads.py b/vrtManager/test_host_reads.py index 70b09170..66df2ae6 100644 --- a/vrtManager/test_host_reads.py +++ b/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("", 0)