From 0c9a002ef44c7645f284c04ba09309e72f92d256 Mon Sep 17 00:00:00 2001 From: catborise Date: Wed, 28 Oct 2026 20:08:02 +0300 Subject: [PATCH] fix(networks): a network's addresses are read as libvirt writes them, and its changes keep each other Reviewed with Codex against libvirt's network format and driver. - Addresses: wvmNetwork found a network's by its text, so one written as or ... without whitespace had none, and a second address of a family overwrote the first. ip_networks reads every in order, its index being libvirt's parentIndex: family (ipv4 when written), address, subnet, DHCP (a , also with fixed addresses only), first range and fixed addresses. The parentIndex was guessed from the number of families, wrong when IPv6 comes first; changes now name the address by its index. - One read: the page read the network's XML once per getter, ten or more round trips; it reads it once (a cached_property of the request's wvmNetwork). A change starts from the saved definition read anew (VIR_NETWORK_XML_INACTIVE), not the running one: two QoS changes on a running network no longer undo the first. The XML editor shows and changes the saved definition, and defines this network only (its name, and its UUID if given), read for that with no entity expanded and handed to libvirt as written; an XML declaring its encoding is taken. The actions go back to the network's page by its own URL. - Fixed addresses are found by the address they had (a hidden field), not by the editable box: changing it and pressing Delete deleted another entry or none. A change deletes the entry and adds it anew from a copy, keeping what the page does not show (a client id, a lease), and puts it back if the add fails: libvirt's own change finds an entry by any of its MAC, address or name, so it could replace another entry, and never by an IPv6 DUID. The address must be of the subnet, not the host's, and neither it, the MAC or DUID nor the name another entry's; an entry gone meanwhile is refused, not added anew; an unchanged one writes no audit row. They can be added while the network is stopped; a running network saved with other addresses than it runs with is not changed live, as parentIndex would name another of it. - An address without a netmask or prefix takes libvirt's (IPv4 its class's, IPv6 /64), for its checks and the overlap check. - The DHCP range must be in the subnet, in order and not take in the host's address. - The create form takes a known mode only ("bogus" made an isolated network, a missing one a server error); NAT and routed networks need an IPv4 subnet, an isolated one may have none, a bridge or macvtap network needs its bridge or interface and gets no addresses; IPv6 only with its box (whose value was "false", so never); no name or subnet of another network of the host. "Fixed Addresses" at create goes: it made a random-MAC entry for every address of the range, for no real VM. - Refusals of a fixed address, the DHCP range, the XML and a QoS removal are messages, not server errors. - The IPv6 forwarding of a bridge or macvtap network is the outside network's, not ROUTE. - get_networks_info reads each network's XML once and gives its autostart and addresses too. Unused can_pxe, DHCP lease helpers, get_uuid and the per-family range and host getters go. --- networks/forms.py | 92 ++-- networks/templates/create_net_block.html | 8 +- .../templates/modify_ipv4_fixed_address.html | 4 +- .../templates/modify_ipv6_fixed_address.html | 4 +- networks/templates/network.html | 64 +-- networks/tests/driver.py | 15 + networks/tests/test_add_net_pool_form.py | 42 +- networks/tests/test_audit_rows.py | 12 +- networks/tests/test_dhcp_range.py | 26 +- networks/tests/test_network_changes.py | 210 ++++++-- networks/tests/test_network_in_use.py | 2 + networks/tests/test_network_qos.py | 2 + networks/views.py | 194 +++----- vrtManager/network.py | 447 +++++++++--------- vrtManager/tests/test_dhcp_leases.py | 36 -- vrtManager/tests/test_network_ip.py | 83 +--- vrtManager/tests/test_xpath_values.py | 3 +- webvirtcloud/tests/test_pages.py | 2 + webvirtcloud/tests/test_status_contrast.py | 2 + 19 files changed, 646 insertions(+), 602 deletions(-) create mode 100644 networks/tests/driver.py delete mode 100644 vrtManager/tests/test_dhcp_leases.py diff --git a/networks/forms.py b/networks/forms.py index 691dd8a1..002f2f1f 100644 --- a/networks/forms.py +++ b/networks/forms.py @@ -24,64 +24,80 @@ def checked_subnet(subnet, version): class AddNetPool(forms.Form): + """A new network of the host. networks: the host's networks + (get_networks_info), whose names it may not take nor overlap their + subnets. NAT and routed networks need an IPv4 subnet; an isolated one + may have none; a bridge or macvtap network takes its addresses from the + outside network, and needs the host's bridge or interface.""" + + MODES = ("nat", "route", "none", "bridge", "macvtap") + name = forms.CharField( error_messages={"required": _("No pool name has been entered")}, max_length=20 ) - subnet = forms.CharField( - error_messages={"required": _("No IPv4 subnet has been entered")}, - max_length=20, - required=False, - ) - subnet6 = forms.CharField( - error_messages={"required": _("No IPv6 subnet has been entered")}, - max_length=42, - required=False, - ) - forward = forms.CharField(max_length=100) + subnet = forms.CharField(label=_("IPv4 subnet"), max_length=20, required=False) + subnet6 = forms.CharField(label=_("IPv6 subnet"), max_length=42, required=False) + forward = forms.ChoiceField(choices=[(mode, mode) for mode in MODES]) dhcp4 = forms.BooleanField(required=False) + enable_ipv6 = forms.BooleanField(required=False) dhcp6 = forms.BooleanField(required=False) - fixed = forms.BooleanField(required=False) - bridge_name = forms.CharField(max_length=20, required=False) + bridge_name = forms.CharField(label=_("Bridge or interface"), max_length=20, required=False) openvswitch = forms.BooleanField(required=False) + def __init__(self, *args, networks=(), **kwargs): + super().__init__(*args, **kwargs) + self.networks = networks + def clean_name(self): name = self.cleaned_data["name"] - have_symbol = re.match(r"^[a-zA-Z0-9\.\_\-]+$", name) - if not have_symbol: + if not re.fullmatch(r"[a-zA-Z0-9._-]+", name): raise forms.ValidationError( _("The pool name must not contain any special characters") ) - elif len(name) > 20: - raise forms.ValidationError( - _("The pool name must not exceed 20 characters") - ) + if any(net["name"] == name for net in self.networks): + raise forms.ValidationError(_("This host has a network of that name"), code="taken") return name 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) def clean_bridge_name(self): bridge_name = self.cleaned_data["bridge_name"] - if self.cleaned_data["forward"] in ["bridge", "macvtap"]: - have_symbol = re.match(r"^[a-zA-Z0-9\.\_\:\-]+$", bridge_name) - if not have_symbol: - raise forms.ValidationError( - _("The pool bridge name must not contain any special characters") - ) - elif len(bridge_name) > 20: - raise forms.ValidationError( - _("The pool bridge name must not exceed 20 characters") - ) - return bridge_name + if bridge_name and not re.fullmatch(r"[a-zA-Z0-9._:-]+", bridge_name): + raise forms.ValidationError( + _("The pool bridge name must not contain any special characters") + ) + return bridge_name + + def clean(self): + data = super().clean() + mode = data.get("forward") + if mode in ("bridge", "macvtap"): + # the outside network gives the addresses: none of libvirt's + data.update(subnet="", subnet6="", dhcp4=False, dhcp6=False) + if not data.get("bridge_name") and "bridge_name" not in self.errors: + self.add_error("bridge_name", _("This field is required.")) + else: + data["bridge_name"] = "" + if mode in ("nat", "route") and not data.get("subnet") and "subnet" not in self.errors: + self.add_error("subnet", _("This field is required.")) + if not data.get("enable_ipv6"): + data["subnet6"] = "" + elif not data.get("subnet6") and "subnet6" not in self.errors: + self.add_error("subnet6", _("This field is required.")) + if mode != "bridge": + data["openvswitch"] = False + for field in ("subnet", "subnet6"): + if data.get(field) and field not in self.errors: + subnet = ipaddress.ip_network(data[field], strict=False) + for net in self.networks: + taken = next((ip["subnet"] for ip in net["ips"] + if ip["subnet"] and ip["subnet"].version == subnet.version and ip["subnet"].overlaps(subnet)), None) + if taken: + self.add_error(field, _("Network %(net)s has %(subnet)s") % {"net": net["name"], "subnet": taken}) + break + return data diff --git a/networks/templates/create_net_block.html b/networks/templates/create_net_block.html index 4caf6db6..9a180b77 100644 --- a/networks/templates/create_net_block.html +++ b/networks/templates/create_net_block.html @@ -45,16 +45,10 @@ -
- -
- -
-
- +
diff --git a/networks/templates/modify_ipv4_fixed_address.html b/networks/templates/modify_ipv4_fixed_address.html index 8a93108a..7f5ada1b 100644 --- a/networks/templates/modify_ipv4_fixed_address.html +++ b/networks/templates/modify_ipv4_fixed_address.html @@ -18,7 +18,7 @@
- +
@@ -41,7 +41,7 @@
- +
diff --git a/networks/templates/modify_ipv6_fixed_address.html b/networks/templates/modify_ipv6_fixed_address.html index d09e3f4d..bf782bda 100644 --- a/networks/templates/modify_ipv6_fixed_address.html +++ b/networks/templates/modify_ipv6_fixed_address.html @@ -18,7 +18,7 @@
- +
@@ -41,7 +41,7 @@
- +
diff --git a/networks/templates/network.html b/networks/templates/network.html index 64e1b95f..4920d182 100644 --- a/networks/templates/network.html +++ b/networks/templates/network.html @@ -86,27 +86,27 @@ {% endif %}
{% trans "Network" %}
-
{{ ipv4_network|default:"-" }}
+
{{ ipv4.subnet|default:"-" }}
{% trans "DHCP" %}:
- {% if ipv4_dhcp_range_start and ipv4_dhcp_range_end %} + {% if ipv4.dhcp %} {% trans "ON" %} {% else %} {% trans "OFF" %} {% endif %}
- {% if ipv4_dhcp_range_start and ipv4_dhcp_range_end %} + {% if ipv4.range %} {% if state %}
{% trans "Start" %}
-
{{ ipv4_dhcp_range_start }}
+
{{ ipv4.range.0 }}
{% trans "End" %}
-
{{ ipv4_dhcp_range_end }}
+
{{ ipv4.range.1 }}
{% else %}
{% csrf_token %}
{% trans "Start" %}
-
+
{% trans "End" %}
-
+
- +
{% endif %} {% endif %} - {% if ipv4_dhcp_range_start and ipv4_dhcp_range_end %} - {% if state %} - {% include 'modify_ipv4_fixed_address.html' %} - {% endif %} + {% if ipv4.dhcp %} + {% include 'modify_ipv4_fixed_address.html' %}
{% trans "IPv4 Fixed Addresses" %}
{% endif %} - {% if ipv4_fixed_address %} + {% if ipv4.hosts %}