1
0
Fork 0
mirror of https://github.com/retspen/webvirtcloud synced 2026-11-09 03:36:48 +00:00

fix(instances): a network device keeps its source unless another is chosen

The Edit dialog of a VM's network device selected the listed network
or interface whose name matched the device's source, and the first one
when none did: saving only a new model moved a macvtap device or a
bridge the host's lists did not show onto the first network, a bridge
named like a network became that network, and a macvtap device chosen
again lost its mode.

The dialog now offers the device's own source first, "<name>, as it
is", and posts nothing for it; the view passes no source and the
device's source element is left as it is. Choosing another source
changes it as before.
This commit is contained in:
catborise 2026-10-31 17:05:20 +03:00
parent 3d69dfe5d9
commit 2e17661390
6 changed files with 44 additions and 15 deletions

View file

@ -359,14 +359,16 @@
<div class="col-12">
<label class="form-label" for="net-source-{{ forloop.counter0 }}">{% trans "Source" %}</label>
<select class="form-select" id="net-source-{{ forloop.counter0 }}" name="net-source-{{ forloop.counter0 }}">
{# the NIC keeps its source, whatever its kind, unless another is chosen #}
<option value="" selected>{% blocktrans with nic=network.nic|default:_("none") %}{{ nic }}, as it is{% endblocktrans %}</option>
<optgroup label="{% trans "Networks" %}">
{% for c_net in networks_host %}
<option value="net:{{ c_net }}"{% if c_net == network.nic %} selected{% endif %}>{{ c_net }}</option>
<option value="net:{{ c_net }}">{{ c_net }}</option>
{% endfor %}
</optgroup>
<optgroup label="{% trans "Interfaces" %}">
{% for c_iface in interfaces_host %}
<option value="iface:{{ c_iface }}"{% if c_iface == network.nic %} selected{% endif %}>{{ c_iface }}</option>
<option value="iface:{{ c_iface }}">{{ c_iface }}</option>
{% endfor %}
</optgroup>
</select>

View file

@ -34,6 +34,10 @@ class ChangeNetworkViewTestCase(TestCase):
})
change.assert_called_once_with("52:54:00:00:00:02", "52:54:00:00:00:03", "default", "net", "e1000", "")
def test_no_source_keeps_the_nics_own(self):
change = self.post({"net-old-mac-1": "52:54:00:00:00:02", "net-source-1": "", "net-model-1": "e1000"})
change.assert_called_once_with("52:54:00:00:00:02", "52:54:00:00:00:02", None, None, "e1000", "")
def test_a_shut_off_vm_is_told_the_change_is_made(self):
with patch("instances.models.wvmInstance") as wvm:
wvm.return_value.get_status.return_value = 5

View file

@ -93,10 +93,16 @@ class PendingEditsTestCase(TestCase):
# the Edit dialog picks the source by kind and posts it as before
(source,) = doc.xpath("//select[@name='net-source-0']")
self.assertEqual(source.xpath("./optgroup/@label"), ["Networks", "Interfaces"])
self.assertEqual(source.xpath(".//option/@value"), ["net:default", "net:isolated", "iface:br0"])
self.assertEqual(source.xpath(".//option[@selected]/@value"), ["net:default"])
self.assertEqual(source.xpath(".//option/@value"), ["", "net:default", "net:isolated", "iface:br0"])
# the NIC keeps its source unless another is chosen
self.assertEqual(source.xpath(".//option[@selected]/text()"), ["default, as it is"])
self.assertEqual(doc.xpath("//div[@id='editInstanceNetwork0']//input[@name='net-old-mac-0']/@value"), [nic["mac"]])
self.assertTrue(doc.xpath("//div[@id='editInstanceNetwork0']//button[@name='change_network']"))
# so does one the host's lists do not show, or one whose name is in both
doc = self.page(nics=([{**nic, "type": "bridge", "nic": "br0"}],) * 2,
get_networks=(["br0"],) * 2, get_ifaces=(["br0"],) * 2)
(source,) = doc.xpath("//select[@name='net-source-0']")
self.assertEqual(source.xpath(".//option[@selected]/@value"), [""])
def test_the_disks_are_rows_of_one_list(self):
disks = [disk("vda", "default", "") | {"used": 1 << 29}, disk("vdb", "default", "") | {"used": (1 << 30) + (1 << 20)}]

View file

@ -1856,7 +1856,9 @@ def change_network(request, pk):
messages.error(request, err)
return redirect(back)
(source, source_type) = utils.get_network_tuple(request.POST.get(f"net-source-{num}", ""))
# no source: the NIC keeps the one it has, which the host's lists may not show
raw_source = request.POST.get(f"net-source-{num}", "")
(source, source_type) = utils.get_network_tuple(raw_source) if raw_source else (None, None)
try:
with audit_vm(request, instance, f"change interface {old_mac}"):
if source_type == "iface":

View file

@ -2706,7 +2706,8 @@ class wvmInstance(wvmConnect):
"""Edit the NIC with old_mac in the persistent definition, in place.
Settings this form does not edit (QoS, link state, driver, PCI address)
stay. A running VM gets the change at its next start.
stay, and so does the source when source is None. A running VM gets
the change at its next start.
"""
tree = etree.fromstring(self._XMLDesc(PERSISTENT_XML))
found = tree.xpath("./devices/interface[mac/@address=$mac]", mac=old_mac)
@ -2716,14 +2717,15 @@ class wvmInstance(wvmConnect):
if iface.get("type") == "hostdev":
raise util.OperationError("A network interface that is a host device is changed in the XML editor")
iface_type = {"net": "network", "bridge": "bridge"}.get(source_type, "direct")
iface.set("type", iface_type)
for old_source in iface.findall("source"):
iface.remove(old_source)
source_attrs = {"network": {"network": source}, "bridge": {"bridge": source}}.get(
iface_type, {"dev": source, "mode": "bridge"}
)
iface.insert(1, etree.Element("source", **source_attrs))
if source is not None:
iface_type = {"net": "network", "bridge": "bridge"}.get(source_type, "direct")
iface.set("type", iface_type)
for old_source in iface.findall("source"):
iface.remove(old_source)
source_attrs = {"network": {"network": source}, "bridge": {"bridge": source}}.get(
iface_type, {"dev": source, "mode": "bridge"}
)
iface.insert(1, etree.Element("source", **source_attrs))
iface.find("mac").set("address", mac)
for old_model in iface.findall("model"):

View file

@ -14,6 +14,7 @@ DOMAIN_XML = """<domain><devices>
<interface type='network'><mac address='52:54:00:00:00:02'/><source network='default'/><model type='virtio'/>
<bandwidth><outbound average='1000'/></bandwidth><link state='down'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/></interface>
<interface type='direct'><mac address='52:54:00:00:00:04'/><source dev='eth0' mode='vepa'/><model type='virtio'/></interface>
</devices></domain>"""
@ -34,7 +35,7 @@ class TestChangeNetwork(unittest.TestCase):
inst.change_network("52:54:00:00:00:02", "52:54:00:00:00:03", "br0", "bridge", "e1000", "clean-traffic")
defined = etree.fromstring(inst.wvm.defineXML.call_args[0][0])
first, second = defined.findall("devices/interface")
first, second, _direct = defined.findall("devices/interface")
self.assertEqual(first.find("mac").get("address"), "52:54:00:00:00:01")
self.assertEqual(second.get("type"), "bridge")
self.assertEqual(second.find("mac").get("address"), "52:54:00:00:00:03")
@ -46,6 +47,18 @@ class TestChangeNetwork(unittest.TestCase):
self.assertEqual(second.find("link").get("state"), "down")
self.assertEqual(second.find("address").get("bus"), "0x02")
def defined_nic(self, *args):
from lxml import etree
inst = stopped_instance()
inst.change_network(*args)
return etree.fromstring(inst.wvm.defineXML.call_args[0][0]).findall("devices/interface")[2]
def test_no_source_keeps_the_one_the_nic_has(self):
nic = self.defined_nic("52:54:00:00:00:04", "52:54:00:00:00:04", None, None, "e1000", "")
self.assertEqual((nic.get("type"), nic.find("source").attrib), ("direct", {"dev": "eth0", "mode": "vepa"}))
self.assertEqual(nic.find("model").get("type"), "e1000")
def test_unknown_old_mac_changes_nothing(self):
inst = stopped_instance()
with self.assertRaises(libvirtError):