From 09d7b0c08a3160950aa34ed12749cd33d590aab6 Mon Sep 17 00:00:00 2001 From: catborise Date: Mon, 26 Oct 2026 14:23:19 +0300 Subject: [PATCH] fix(networks): fixed addresses, failed creates and stopped networks behave - A network created with fixed addresses gets one per address of its DHCP range for any subnet; the old prefix cut assumed a gateway ending in .1 and built addresses such as 10.0.0.1.130. Fixed addresses need a /24 or smaller subnet. - A new network that does not start (its subnet is in use) is undefined again, so the name is free for another try. - Fixed addresses are changed and deleted on a stopped network too: the running network is changed only while it runs. A host with only a name is deleted as defined, not with an empty MAC, shows no "None", and has no edit button it could not use. - The create dialog disables the fields it hides, so an IPv6 subnet typed and then unchecked is not created, and switching from MACVTAP back to BRIDGE restores the bridge label and the Open vSwitch option. - Isolated networks show QoS; QoS save buttons no longer raise a script error; saving an unchanged fixed address says so; two messages lose a stray backslash and spaces. --- networks/forms.py | 8 ++++ networks/templates/create_net_block.html | 2 +- networks/templates/network.html | 16 ++++--- networks/templates/networks.html | 39 +++++++--------- networks/tests/test_network_changes.py | 57 ++++++++++++++++++++++++ networks/views.py | 21 +++++---- vrtManager/network.py | 40 +++++++---------- 7 files changed, 119 insertions(+), 64 deletions(-) create mode 100644 networks/tests/test_network_changes.py diff --git a/networks/forms.py b/networks/forms.py index b3cc8e4f..691dd8a1 100644 --- a/networks/forms.py +++ b/networks/forms.py @@ -61,6 +61,14 @@ class AddNetPool(forms.Form): def clean_subnet(self): return checked_subnet(self.cleaned_data["subnet"], 4) + def clean(self): + cleaned = super().clean() + # one fixed address per address of the DHCP range + subnet = cleaned.get("subnet") + if cleaned.get("fixed") and subnet and ipaddress.ip_network(subnet, strict=False).prefixlen < 24: + self.add_error("fixed", _("Fixed addresses need an IPv4 subnet of /24 or smaller")) + return cleaned + def clean_subnet6(self): return checked_subnet(self.cleaned_data["subnet6"], 6) diff --git a/networks/templates/create_net_block.html b/networks/templates/create_net_block.html index 0b79c182..4caf6db6 100644 --- a/networks/templates/create_net_block.html +++ b/networks/templates/create_net_block.html @@ -60,7 +60,7 @@
- +
diff --git a/networks/templates/network.html b/networks/templates/network.html index b608d4bd..e67f3ea5 100644 --- a/networks/templates/network.html +++ b/networks/templates/network.html @@ -183,19 +183,21 @@ {% for fix4 in ipv4_fixed_address %}
{% csrf_token %} - + + {% if fix4.mac %}{# an entry is changed by its MAC #} + {% endif %} @@ -294,19 +296,21 @@ {% for fix6 in ipv6_fixed_address %} {% csrf_token %} - + + {% if fix6.id %}{# an entry is changed by its DUID #} + {% endif %} @@ -320,7 +324,7 @@
{% endif %} - {% if net_forward.0 == 'route' or net_forward.0 == 'nat' or net_forward.0 == 'isolated' %} + {% if net_forward.0 == 'route' or net_forward.0 == 'nat' or not net_forward.0 %}{# an isolated network has no forward #} {% if state == 0 and qos.items|length != 2%} {% csrf_token %} {% include 'add_network_qos.html' %} @@ -357,7 +361,7 @@ diff --git a/networks/templates/networks.html b/networks/templates/networks.html index 64bbf813..6d3abb9d 100644 --- a/networks/templates/networks.html +++ b/networks/templates/networks.html @@ -71,29 +71,22 @@ {% block script %} {% endblock %} \ No newline at end of file diff --git a/networks/tests/test_network_changes.py b/networks/tests/test_network_changes.py new file mode 100644 index 00000000..c81b0f06 --- /dev/null +++ b/networks/tests/test_network_changes.py @@ -0,0 +1,57 @@ +"""A new network with fixed addresses gets one per address of its DHCP range, +for any /24 or smaller subnet; a network that does not start is not left +defined; fixed addresses change on a stopped network and may have no MAC.""" + +from unittest.mock import patch + +from django.test import SimpleTestCase +from libvirt import VIR_NETWORK_UPDATE_AFFECT_CONFIG, VIR_NETWORK_UPDATE_AFFECT_LIVE, libvirtError +from lxml import etree +from networks.forms import AddNetPool +from vrtManager.network import network_size, wvmNetwork, wvmNetworks + +NET = ("n1" + "") + + +class NetworkChangeTests(SimpleTestCase): + def setUp(self): + patcher = patch("vrtManager.connection.connection_manager.get_connection") + self.wvm = patcher.start().return_value + self.addCleanup(patcher.stop) + + def create(self, subnet): + gateway, mask, dhcp = network_size(subnet, True) + wvmNetworks("host", "login", "pw", 1).create_network( + "n1", "nat", True, gateway, mask, dhcp, False, None, None, None, "", False, fixed=True) + return etree.fromstring(self.wvm.networkDefineXML.call_args.args[0]) + + def test_fixed_addresses_cover_the_dhcp_range(self): + ip = self.create("192.0.2.128/25").find("ip") + # the gateway is the subnet's first address, not one ending in .1 + self.assertEqual((ip.get("address"), ip.get("netmask")), ("192.0.2.129", "255.255.255.128")) + hosts = ip.xpath("dhcp/host/@ip") + self.assertEqual((hosts[0], hosts[-1], len(hosts)), ("192.0.2.130", "192.0.2.254", 125)) + + def test_fixed_addresses_need_a_small_subnet(self): + form = AddNetPool({"name": "n1", "forward": "nat", "subnet": "198.51.0.0/16", "dhcp4": True, "fixed": True}) + self.assertIn("fixed", form.errors) + + def test_a_network_that_does_not_start_is_undefined(self): + net = self.wvm.networkDefineXML.return_value + net.create.side_effect = libvirtError("network is already in use by interface virbr0") + with self.assertRaises(libvirtError): + self.create("192.0.2.0/24") + net.undefine.assert_called_once_with() + net.setAutostart.assert_not_called() + + def test_a_stopped_network_changes_its_configuration_only(self): + net = self.wvm.networkLookupByName.return_value + net.XMLDesc.return_value = NET + for active, flags in ((False, VIR_NETWORK_UPDATE_AFFECT_CONFIG), + (True, VIR_NETWORK_UPDATE_AFFECT_CONFIG | VIR_NETWORK_UPDATE_AFFECT_LIVE)): + with self.subTest(active=active): + net.isActive.return_value = active + wvmNetwork("host", "login", "pw", 1, "n1").delete_fixed_address("192.0.2.20") + # a host with only a name is named as it is, without an empty MAC + self.assertEqual(net.update.call_args.args[3:], ('', flags)) diff --git a/networks/views.py b/networks/views.py index f194f26e..70d25bbc 100644 --- a/networks/views.py +++ b/networks/views.py @@ -188,12 +188,15 @@ def network(request, compute_id, pool): try: with audit(request, compute, subject, f"set fixed address {address}"): - conn.modify_fixed_address(name, address, mac_duid, family) - messages.success( - request, - _("Fixed address operation completed for %(family)s") - % {"family": family.upper()}, - ) + changed = conn.modify_fixed_address(name, address, mac_duid, family) + if changed is False: + messages.info(request, _("The fixed address is unchanged")) + else: + messages.success( + request, + _("Fixed address operation completed for %(family)s") + % {"family": family.upper()}, + ) return HttpResponseRedirect(request.get_full_path()) except libvirtError as lib_err: messages.error(request, lib_err) @@ -232,7 +235,7 @@ def network(request, compute_id, pool): messages.success( request, _( - "Network XML is changed. \\" + "Network XML is changed. " "Stop and start network to activate new config." ), ) @@ -277,8 +280,8 @@ def network(request, compute_id, pool): messages.success( request, _( - "%(qos_dir)s QoS is deleted. Network XML is changed. \ - Stop and start network to activate new config" + "%(qos_dir)s QoS is deleted. Network XML is changed. " + "Stop and start network to activate new config" ) % {"qos_dir": qos_dir.capitalize()}, ) diff --git a/vrtManager/network.py b/vrtManager/network.py index 0044b7c1..a12b9c02 100644 --- a/vrtManager/network.py +++ b/vrtManager/network.py @@ -50,9 +50,6 @@ class wvmNetworks(wvmConnect): return networks - def define_network(self, xml): - self.wvm.networkDefineXML(xml) - def create_network( self, name, @@ -104,11 +101,9 @@ class wvmNetworks(wvmConnect): xml += f""" """ if fixed: - fist_oct = int(dhcp4[0].strip().split(".")[3]) - last_oct = int(dhcp4[1].strip().split(".")[3]) - gw_prefix = util.xml_escape(str(gateway)[:-2]) - for ip in range(fist_oct, last_oct + 1): - xml += f"""""" + first, last = (int(ipaddress.IPv4Address(ip)) for ip in dhcp4) + for ip in range(first, last + 1): + xml += f"""""" xml += """""" xml += """""" if ipv6: @@ -121,9 +116,12 @@ class wvmNetworks(wvmConnect): xml += """""" xml += """""" xml += """""" - self.define_network(xml) - net = self.get_network(name) - net.create() + net = self.wvm.networkDefineXML(xml) + try: + net.create() + except libvirtError: # e.g. the subnet is in use: the name stays free for another try + net.undefine() + raise net.setAutostart(1) @@ -182,7 +180,10 @@ class wvmNetwork(wvmConnect): "start; remove their network cards first") self.net.undefine() - def update(self, command, section, xml, parentIndex, flags=0): + def update(self, command, section, xml, parentIndex): + """Changes the network's configuration and, while it runs, the running + network too (libvirt refuses a live change on a stopped one).""" + flags = VIR_NETWORK_UPDATE_AFFECT_CONFIG | (VIR_NETWORK_UPDATE_AFFECT_LIVE if self.is_active() else 0) return self.net.update(command, section, parentIndex, xml, flags) def get_ip_networks(self): @@ -289,22 +290,13 @@ class wvmNetwork(wvmConnect): parent_index = self.parent_count - 1 for h in hosts: if h.get("ip") == ip: - h_name = util.xml_escape(h.get("name") or "") - name_attr = f' name="{h_name}"' if h.get("name") else "" - ip_esc = util.xml_escape(ip) - if family == "ipv4": - h_mac = util.xml_escape(h.get("mac") or "") - new_xml = f'' - if family == "ipv6": - h_id = util.xml_escape(h.get("id") or "") - new_xml = f'' - + # the host as it is: a host may have only a name and an address + new_xml = etree.tostring(h, with_tail=False).decode() self.update( VIR_NETWORK_UPDATE_COMMAND_DELETE, VIR_NETWORK_SECTION_IP_DHCP_HOST, new_xml, parent_index, - VIR_NETWORK_UPDATE_AFFECT_LIVE | VIR_NETWORK_UPDATE_AFFECT_CONFIG, ) break @@ -332,7 +324,6 @@ class wvmNetwork(wvmConnect): VIR_NETWORK_SECTION_IP_DHCP_HOST, new_xml, parent_index, - VIR_NETWORK_UPDATE_AFFECT_LIVE | VIR_NETWORK_UPDATE_AFFECT_CONFIG, ) else: # change the host @@ -344,7 +335,6 @@ class wvmNetwork(wvmConnect): VIR_NETWORK_SECTION_IP_DHCP_HOST, new_xml, parent_index, - VIR_NETWORK_UPDATE_AFFECT_LIVE | VIR_NETWORK_UPDATE_AFFECT_CONFIG, ) def get_qos(self):