From 5def4e41fd114205bec574fcfa3a3fac0ae00111 Mon Sep 17 00:00:00 2001 From: catborise Date: Sun, 4 Oct 2026 00:27:55 +0300 Subject: [PATCH] fix(instances): NIC edits keep their settings; edits keep metadata - change_network detached the NIC and attached one built from scratch, dropping its QoS, link state and other settings. It now edits the NIC of the persistent definition in place and defines once; a running VM gets the change at its next start, as the UI already says. - add_network, delete_network and set_link_state did nothing on a paused VM, and set_link_state rewrote the persistent NIC from the live XML. They now update the running definition when the VM is active and the persistent one always, each from its own XML. - Boot and console edits used xml.etree, which renamed the namespace prefixes of other tools' metadata ( became ); they use lxml now. - The NIC table and the clone form are filled from the persistent definition (Instance.config_networks, config_disks), so a pending NIC change is shown and can be edited again. A live test runs 18 domain edits on stopped, running and paused VMs whose definition has elements WebVirtCloud does not manage, and checks for each the effect, that nothing else in the persistent definition changed, and that persistent-only edits leave the running VM alone. --- instances/models.py | 15 + .../templates/instances/settings_tab.html | 8 +- instances/test_live_dataloss.py | 24 +- instances/test_live_xml_effects.py | 265 ++++++++++++++++++ instances/views.py | 4 +- vrtManager/instance.py | 136 +++++---- vrtManager/test_change_network.py | 45 +-- vrtManager/test_persistent_edits.py | 36 +++ 8 files changed, 444 insertions(+), 89 deletions(-) create mode 100644 instances/test_live_xml_effects.py diff --git a/instances/models.py b/instances/models.py index dbeade0e..c356f035 100644 --- a/instances/models.py +++ b/instances/models.py @@ -135,6 +135,21 @@ class Instance(models.Model): def networks(self) -> list[dict]: return self.proxy.get_net_devices() + @cached_property + def config_networks(self) -> list[dict]: + """NICs of the persistent definition, which the edit and clone forms + change; target is the running VM's device name, if any.""" + live_targets = {net["mac"]: net.get("target", "") for net in self.networks} + networks = self.proxy.get_net_devices(config=True) + for net in networks: + net["target"] = live_targets.get(net["mac"], "") + return networks + + @cached_property + def config_disks(self) -> list[dict]: + """Disks of the persistent definition, which a clone copies.""" + return self.proxy.get_disk_devices(config=True) + @cached_property def qos(self): return self.proxy.get_all_qos() diff --git a/instances/templates/instances/settings_tab.html b/instances/templates/instances/settings_tab.html index ea932f19..64710c77 100644 --- a/instances/templates/instances/settings_tab.html +++ b/instances/templates/instances/settings_tab.html @@ -360,7 +360,7 @@ - {% for network in instance.networks %} + {% for network in instance.config_networks %} eth{{ forloop.counter0 }}({{ network.target|default:"no target" }})
@@ -762,7 +762,7 @@ {% if request.user.is_superuser %} - {% for network in instance.networks %} + {% for network in instance.config_networks %}

@@ -779,13 +779,13 @@

{% endfor %} {% else %} - {% for network in instance.networks %} + {% for network in instance.config_networks %} {% endfor %} {% endif %} {% if request.user.is_superuser %} - {% for disk in instance.disks %} + {% for disk in instance.config_disks %}
diff --git a/instances/test_live_dataloss.py b/instances/test_live_dataloss.py index 494fa932..3c03f0c1 100644 --- a/instances/test_live_dataloss.py +++ b/instances/test_live_dataloss.py @@ -63,8 +63,8 @@ class LiveDataLossTestCase(TestCase): # helpers - def vm(self, name, disks, uefi=False): - dom = livetest.define_vm(self.conn, P + name, disks, uefi=uefi) + def vm(self, name, disks, uefi=False, extra_devices=""): + dom = livetest.define_vm(self.conn, P + name, disks, uefi=uefi, extra_devices=extra_devices) refr(self.compute) return dom, Instance.objects.get(compute=self.compute, uuid=dom.UUIDString()) @@ -268,3 +268,23 @@ class LiveDataLossTestCase(TestCase): self.post("change_options", inst, {"title": "for-old", "description": ""}) self.assertNotIn("for-old", dom_new.XMLDesc(0)) + + # NIC edits of a running VM are pending until restart; the form must show them + + def test_pending_nic_change_is_shown_and_can_be_edited_again(self): + nic = "" + dom, inst = self.vm("nic-pending", [], extra_devices=nic) + dom.create() + + def change(old, new): + return self.post("change_network", inst, { + "net-old-mac-0": old, "net-mac-0": new, "net-source-0": "net:default", "net-model-0": "virtio", + }) + + change("52:54:00:aa:05:01", "52:54:00:aa:05:02") + page = self.client.get(reverse("instances:instance", args=[inst.id])).content.decode() + self.assertIn('name="net-old-mac-0" value="52:54:00:aa:05:02"', page) + + change("52:54:00:aa:05:02", "52:54:00:aa:05:03") + self.assertIn("52:54:00:aa:05:03", dom.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE)) + diff --git a/instances/test_live_xml_effects.py b/instances/test_live_xml_effects.py new file mode 100644 index 00000000..59e42535 --- /dev/null +++ b/instances/test_live_xml_effects.py @@ -0,0 +1,265 @@ +"""Live checks that each domain edit does what it says and nothing else. + +For every edit the persistent definition (inactive, with secrets) is read +before and after. The edit must show its effect, and once the nodes it is +allowed to touch are removed from both sides, the rest of the definition +must be identical: unknown elements, secrets and pending changes survive. +On a running VM, edits of the persistent definition must leave the live +one alone. Runs only with TEST_LIBVIRT_HOST set (see instances/livetest.py). +""" + +import os +import unittest + +import libvirt +from django.test import SimpleTestCase +from lxml import etree +from vrtManager.connection import connection_manager +from vrtManager.instance import wvmInstance + +from . import livetest + +P = livetest.PREFIX +MAC = "52:54:00:aa:02:01" +PERSISTENT = libvirt.VIR_DOMAIN_XML_INACTIVE | libvirt.VIR_DOMAIN_XML_SECURE +PARSER = etree.XMLParser(remove_blank_text=True) + + +def domain_xml(name, disk, iso): + """A VM with elements WebVirtCloud does not manage, which edits must keep.""" + return f""" + + {name} + original title + keep-metadata + 128 + 128 + + 1 + 2048 + hvm + + restart + + + + + + keep-serial + 10485760 + + + + + + + + + + + + + + + + + + + /dev/urandom + + +""" + + +def first(tree, path): + found = tree.xpath(path) + return found[0] if found else None + + +# name -> (edit, check of the new persistent tree, paths the edit may change, +# allowed while running, changes the running VM too) +EDITS = { + "set_bootmenu": ( + lambda vm: vm.set_bootmenu(1), + lambda t: first(t, "os/bootmenu/@enable") == "yes", + ["os/bootmenu"], True, False, + ), + "set_bootorder": ( + lambda vm: vm.set_bootorder({0: {"type": "disk", "dev": "vda"}}), + lambda t: first(t, "devices/disk[target/@dev='vda']/boot/@order") == "1" and not t.xpath("os/boot"), + ["os/boot", "devices/*/boot"], True, False, + ), + "set_vcpu_hotplug": ( + lambda vm: vm.set_vcpu_hotplug(True, 1), + lambda t: bool(t.xpath("vcpus/vcpu")), + ["vcpus"], False, False, + ), + "set_console_keymap": ( + lambda vm: vm.set_console_keymap("de"), + lambda t: first(t, "devices/graphics/@keymap") == "de", + ["devices/graphics/@keymap"], True, False, + ), + "set_console_listener_addr": ( + lambda vm: vm.set_console_listener_addr("127.0.0.1"), + lambda t: first(t, "devices/graphics/listen/@address") == "127.0.0.1", + ["devices/graphics/@listen", "devices/graphics/listen/@address"], True, False, + ), + "set_console_passwd": ( + lambda vm: vm.set_console_passwd("new12345"), + lambda t: first(t, "devices/graphics/@passwd") == "new12345", + ["devices/graphics/@passwd"], True, False, + ), + "set_video_model": ( + lambda vm: vm.set_video_model("virtio"), + lambda t: first(t, "devices/video/model/@type") == "virtio", + ["devices/video"], True, False, + ), + "resize_cpu": ( + lambda vm: vm.resize_cpu("1", "2"), + lambda t: t.findtext("vcpu") == "2" and first(t, "vcpu/@current") == "1", + ["vcpu"], True, False, + ), + "resize_mem": ( + lambda vm: vm.resize_mem(128, 160), + lambda t: t.findtext("memory") == str(160 * 1024), + ["memory", "currentMemory"], False, False, + ), + "set_options": ( + lambda vm: vm.set_options({"title": "new title", "description": "new description"}), + lambda t: t.findtext("title") == "new title" and t.findtext("description") == "new description", + ["title", "description"], True, False, + ), + "set_qos": ( + lambda vm: vm.set_qos(MAC, "inbound", 100, 200, 300), + lambda t: first(t, "devices/interface/bandwidth/inbound/@average") == "100" + and first(t, "devices/interface/bandwidth/outbound/@average") == "1000", + ["devices/interface/bandwidth/inbound"], True, False, + ), + "unset_qos": ( + lambda vm: vm.unset_qos(MAC, "outbound"), + lambda t: not t.xpath("devices/interface/bandwidth/outbound"), + ["devices/interface/bandwidth"], True, False, + ), + "umount_iso": ( + lambda vm: vm.umount_iso("sda"), + lambda t: not t.xpath("devices/disk[target/@dev='sda']/source"), + ["devices/disk[target/@dev='sda']/source"], True, True, + ), + "set_link_state": ( + lambda vm: vm.set_link_state(MAC, "down"), + lambda t: first(t, "devices/interface/link/@state") == "down", + ["devices/interface/link"], True, True, + ), + "change_network (same MAC, new model)": ( + lambda vm: vm.change_network(MAC, MAC, "default", "net", "e1000e", ""), + lambda t: first(t, "devices/interface/model/@type") == "e1000e" + and first(t, "devices/interface/bandwidth/outbound/@average") == "1000", + ["devices/interface/model"], True, False, + ), + "add_network": ( + lambda vm: vm.add_network("52:54:00:aa:02:09", "default", "net", "virtio"), + lambda t: len(t.xpath("devices/interface")) == 2, + ["devices/interface[mac/@address='52:54:00:aa:02:09']"], True, True, + ), + "delete_network": ( + lambda vm: vm.delete_network(MAC), + lambda t: not t.xpath("devices/interface"), + ["devices/interface"], True, True, + ), + "edit_disk (cache mode)": ( + lambda vm: vm.edit_disk("vda", vm.get_disk_devices(config=True)[0]["path"], False, False, "virtio", + "keep-serial", "qcow2", "writeback", None, None, None), + lambda t: first(t, "devices/disk[target/@dev='vda']/driver/@cache") == "writeback", + ["devices/disk[target/@dev='vda']/driver/@cache"], True, False, + ), +} + + +def canonical(tree, paths): + tree = etree.fromstring(etree.tostring(tree), PARSER) + for path in paths: + for node in tree.xpath(path): + if isinstance(node, etree._ElementUnicodeResult): + node.getparent().attrib.pop(node.attrname, None) + else: + node.getparent().remove(node) + return etree.tostring(tree, method="c14n").decode() + + +class LiveXmlEffectsTestCase(SimpleTestCase): + @classmethod + def setUpClass(cls): + if not livetest.enabled(): + raise unittest.SkipTest("Set TEST_LIBVIRT_HOST to run the live libvirt tests") + cls.args = ( + os.environ["TEST_LIBVIRT_HOST"], + os.environ.get("TEST_LIBVIRT_LOGIN", ""), + os.environ.get("TEST_LIBVIRT_PASSWORD", ""), + int(os.environ.get("TEST_LIBVIRT_TYPE", 4)), + ) + cls.conn = connection_manager.get_connection(*cls.args) + livetest.cleanup(cls.conn) + livetest.ensure_pool(cls.conn) + cls.inventory = livetest.inventory(cls.conn) + super().setUpClass() + + @classmethod + def tearDownClass(cls): + try: + livetest.remove_pool(cls.conn) + finally: + super().tearDownClass() + if livetest.inventory(cls.conn) != cls.inventory: + raise AssertionError("Live tests changed objects outside the test namespace") + + def vm(self, running, paused=False): + disk = livetest.create_volume(self.conn, P + "fx") + iso = self.conn.storagePoolLookupByName(livetest.POOL).createXML( + f"{P}fx.iso1", 0 + ).path() + dom = self.conn.defineXML(domain_xml(P + "fx", disk, iso)) + if running: + dom.create() + # A pending change only in the persistent definition + tree = self.persistent(dom) + tree.find("on_crash").text = "destroy" + self.conn.defineXML(etree.tostring(tree).decode()) + if paused: + dom.suspend() + return dom, wvmInstance(*self.args, None, uuid=dom.UUIDString()) + + def persistent(self, dom): + return etree.fromstring(dom.XMLDesc(PERSISTENT), PARSER) + + def live(self, dom): + return etree.fromstring(dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE), PARSER) + + def check(self, running, paused=False): + for name, (edit, effect, touched, allowed_running, changes_live) in EDITS.items(): + if running and not allowed_running: + continue + with self.subTest(name): + try: + dom, vm = self.vm(running, paused) + before, live_before = self.persistent(dom), dom.isActive() and self.live(dom) + edit(vm) + after = self.persistent(dom) + self.assertTrue(effect(after), "edit had no effect") + self.assertEqual(canonical(after, touched), canonical(before, touched), "unrelated change") + if running and not changes_live: + self.assertEqual( + canonical(self.live(dom), ["devices/graphics/@port"]), + canonical(live_before, ["devices/graphics/@port"]), + "persistent-only edit changed the running VM", + ) + finally: + livetest.cleanup(self.conn) + + def test_edits_of_a_stopped_vm(self): + self.check(running=False) + + def test_edits_of_a_running_vm(self): + self.check(running=True) + + def test_edits_of_a_paused_vm(self): + self.check(running=True, paused=True) diff --git a/instances/views.py b/instances/views.py index 449aac71..55c55256 100755 --- a/instances/views.py +++ b/instances/views.py @@ -1748,11 +1748,11 @@ def clone(request, pk): clone_data[disk_dev] = disk_name if not request.user.is_superuser: - for disk in instance.disks: + for disk in instance.config_disks: clone_data[f"disk-{disk['dev']}"] = utils.get_clone_disk_name( disk, instance.name, clone_data["name"] ) - for num in range(max(1, len(instance.networks))): + for num in range(max(1, len(instance.config_networks))): key = f"clone-net-mac-{num}" if not clone_data.get(key): clone_data[key] = ( diff --git a/vrtManager/instance.py b/vrtManager/instance.py index 490f3c09..6553a350 100644 --- a/vrtManager/instance.py +++ b/vrtManager/instance.py @@ -413,7 +413,7 @@ class wvmInstance(wvmConnect): arp_flag = 3 # libvirt."VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP" self._ip_cache["arp"] = self._get_interface_addresses(arp_flag) - def get_net_devices(self): + def get_net_devices(self, config=False): def networks(ctx): result = [] inbound = outbound = [] @@ -460,7 +460,7 @@ class wvmInstance(wvmConnect): ) return result - return util.get_xml_path(self._XMLDesc(0), func=networks) + return util.get_xml_path(self._XMLDesc(PERSISTENT_XML if config else 0), func=networks) def get_disk_devices(self, config=False): def disks(doc): @@ -592,12 +592,12 @@ class wvmInstance(wvmConnect): return menu == "yes" def set_bootmenu(self, flag): - tree = ElementTree.fromstring(self._XMLDesc(PERSISTENT_XML)) + tree = etree.fromstring(self._XMLDesc(PERSISTENT_XML)) os = tree.find("os") menu = os.find("bootmenu") if menu is None: - bootmenu = ElementTree.fromstring("") + bootmenu = etree.fromstring("") os.append(bootmenu) menu = os.find("bootmenu") @@ -612,7 +612,7 @@ class wvmInstance(wvmConnect): "Unknown boot menu option, please choose one of 0:disable, 1:enable, -1:remove" ) - xmldom = ElementTree.tostring(tree).decode() + xmldom = etree.tostring(tree).decode() self._defineXML(xmldom) def get_bootorder(self): @@ -668,7 +668,7 @@ class wvmInstance(wvmConnect): return def remove_bootorder(): - tree = ElementTree.fromstring(self._XMLDesc(PERSISTENT_XML)) + tree = etree.fromstring(self._XMLDesc(PERSISTENT_XML)) os = tree.find("os") boot = os.findall("boot") # Remove old style boot order @@ -684,7 +684,7 @@ class wvmInstance(wvmConnect): tree = remove_bootorder() for idx, dev in devorder.items(): - order = ElementTree.fromstring("".format(idx + 1)) + order = etree.fromstring("".format(idx + 1)) if dev["type"] == "disk": devices = tree.findall("./devices/disk[@device='disk']") for d in devices: @@ -705,7 +705,7 @@ class wvmInstance(wvmConnect): d.append(order) else: raise Exception("Invalid Device Type for boot order") - self._defineXML(ElementTree.tostring(tree).decode()) + self._defineXML(etree.tostring(tree).decode()) def _set_cdrom_media(self, dev, path): """Insert path into, or with None eject, CD-ROM dev. @@ -1096,7 +1096,7 @@ class wvmInstance(wvmConnect): return listener_addr def set_console_listener_addr(self, listener_addr): - root = ElementTree.fromstring(self._XMLDesc(PERSISTENT_XML)) + root = etree.fromstring(self._XMLDesc(PERSISTENT_XML)) # The console is the first graphics device of the definition being # edited; the running VM may still have another type. graphic = root.find("devices/graphics") @@ -1113,7 +1113,7 @@ class wvmInstance(wvmConnect): graphic.attrib.pop("listen") listen.attrib.pop("address") - newxml = ElementTree.tostring(root).decode() + newxml = etree.tostring(root).decode() return self._defineXML(newxml) def get_console_socket(self): @@ -1153,7 +1153,7 @@ class wvmInstance(wvmConnect): ) def set_console_passwd(self, passwd): - root = ElementTree.fromstring(self._XMLDesc(PERSISTENT_XML)) + root = etree.fromstring(self._XMLDesc(PERSISTENT_XML)) graphic = root.find("devices/graphics") if graphic is None: return False @@ -1163,11 +1163,11 @@ class wvmInstance(wvmConnect): with contextlib.suppress(Exception): graphic.attrib.pop("passwd") - newxml = ElementTree.tostring(root).decode() + newxml = etree.tostring(root).decode() return self._defineXML(newxml) def set_console_keymap(self, keymap): - root = ElementTree.fromstring(self._XMLDesc(PERSISTENT_XML)) + root = etree.fromstring(self._XMLDesc(PERSISTENT_XML)) graphic = root.find("devices/graphics") if graphic is None: return False @@ -1177,7 +1177,7 @@ class wvmInstance(wvmConnect): with contextlib.suppress(Exception): graphic.attrib.pop("keymap") - newxml = ElementTree.tostring(root).decode() + newxml = etree.tostring(root).decode() self._defineXML(newxml) def get_console_keymap(self): @@ -1674,41 +1674,67 @@ class wvmInstance(wvmConnect): etree.SubElement(iface, "filterref", filter=nwfilter) xml_iface = etree.tostring(iface).decode() - if self.get_status() == 1: + if self.instance.isActive(): # running or paused self.instance.attachDeviceFlags(xml_iface, VIR_DOMAIN_AFFECT_LIVE) - self.instance.attachDeviceFlags(xml_iface, VIR_DOMAIN_AFFECT_CONFIG) - if self.get_status() == 5: - self.instance.attachDeviceFlags(xml_iface, VIR_DOMAIN_AFFECT_CONFIG) + self.instance.attachDeviceFlags(xml_iface, VIR_DOMAIN_AFFECT_CONFIG) + + def _definitions(self): + """(XMLDesc flags, affect flag) of the definitions a device edit applies to: + the running one when the VM is active (running or paused), and the + persistent one. Each is edited from its own XML.""" + definitions = [(PERSISTENT_XML, VIR_DOMAIN_AFFECT_CONFIG)] + if self.instance.isActive(): + definitions.insert(0, (VIR_DOMAIN_XML_SECURE, VIR_DOMAIN_AFFECT_LIVE)) + return definitions + + def _nic(self, xml_flags, mac_address): + found = etree.fromstring(self._XMLDesc(xml_flags)).xpath( + "./devices/interface[mac/@address=$mac]", mac=mac_address + ) + return found[0] if found else None def delete_network(self, mac_address): - tree = ElementTree.fromstring(self._XMLDesc(0)) - for interface in tree.findall("devices/interface"): - source = interface.find("mac") - if source.get("address", "") == mac_address: - new_xml = ElementTree.tostring(interface).decode() - - if self.get_status() == 1: - self.instance.detachDeviceFlags(new_xml, VIR_DOMAIN_AFFECT_LIVE) - self.instance.detachDeviceFlags(new_xml, VIR_DOMAIN_AFFECT_CONFIG) - if self.get_status() == 5: - self.instance.detachDeviceFlags(new_xml, VIR_DOMAIN_AFFECT_CONFIG) - return new_xml - return None + """Detach the NIC; returns its persistent XML, or None if there is none.""" + removed = None + for xml_flags, affect in self._definitions(): + nic = self._nic(xml_flags, mac_address) + if nic is not None: + removed = etree.tostring(nic).decode() + self.instance.detachDeviceFlags(removed, affect) + return removed def change_network(self, old_mac, mac, source, source_type, model, nwfilter): - """Replace the NIC with old_mac; put it back if the new one cannot be added.""" - old_xml = self.delete_network(old_mac) - if old_xml is None: + """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. + """ + tree = etree.fromstring(self._XMLDesc(PERSISTENT_XML)) + found = tree.xpath("./devices/interface[mac/@address=$mac]", mac=old_mac) + if not found: raise util.OperationError(f"No network interface with MAC {old_mac}") - try: - self.add_network(mac, source, source_type, model, nwfilter) - except libvirtError: - if self.get_status() == 1: - self.instance.attachDeviceFlags(old_xml, VIR_DOMAIN_AFFECT_LIVE) - self.instance.attachDeviceFlags(old_xml, VIR_DOMAIN_AFFECT_CONFIG) - if self.get_status() == 5: - self.instance.attachDeviceFlags(old_xml, VIR_DOMAIN_AFFECT_CONFIG) - raise + iface = found[0] + + 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"): + iface.remove(old_model) + if model: + etree.SubElement(iface, "model", type=model) + for old_filter in iface.findall("filterref"): + iface.remove(old_filter) + if nwfilter: + etree.SubElement(iface, "filterref", filter=nwfilter) + + self._defineXML(etree.tostring(tree).decode()) def change_network_oldway(self, network_data): """ @@ -1768,22 +1794,14 @@ class wvmInstance(wvmConnect): self._defineXML(new_xml) def set_link_state(self, mac_address, state): - tree = etree.fromstring(self._XMLDesc(0)) - for interface in tree.findall("devices/interface"): - source = interface.find("mac") - if source.get("address") == mac_address: - link = interface.find("link") - if link is not None: - interface.remove(link) - link_el = etree.Element("link") - link_el.attrib["state"] = state - interface.append(link_el) - new_xml = etree.tostring(interface).decode() - if self.get_status() == 1: - self.instance.updateDeviceFlags(new_xml, VIR_DOMAIN_AFFECT_LIVE) - self.instance.updateDeviceFlags(new_xml, VIR_DOMAIN_AFFECT_CONFIG) - if self.get_status() == 5: - self.instance.updateDeviceFlags(new_xml, VIR_DOMAIN_AFFECT_CONFIG) + for xml_flags, affect in self._definitions(): + nic = self._nic(xml_flags, mac_address) + if nic is None: + continue + for link in nic.findall("link"): + nic.remove(link) + etree.SubElement(nic, "link", state=state) + self.instance.updateDeviceFlags(etree.tostring(nic).decode(), affect) def _set_options(self, tree, options): for o in ["title", "description"]: diff --git a/vrtManager/test_change_network.py b/vrtManager/test_change_network.py index 8bc8b015..b26fc356 100644 --- a/vrtManager/test_change_network.py +++ b/vrtManager/test_change_network.py @@ -6,50 +6,51 @@ from django.conf import settings if not settings.configured: settings.configure(MAC_OUI="52:54:10") -from libvirt import VIR_DOMAIN_AFFECT_CONFIG, libvirtError +from libvirt import libvirtError from vrtManager.instance import wvmInstance DOMAIN_XML = """ - + + +
""" def stopped_instance(): inst = wvmInstance.__new__(wvmInstance) inst.instance = MagicMock() + inst.wvm = MagicMock() inst._XMLDesc = MagicMock(return_value=DOMAIN_XML) inst.get_status = MagicMock(return_value=5) return inst class TestChangeNetwork(unittest.TestCase): - def test_replaces_the_nic_with_the_old_mac(self): + def test_edits_the_nic_in_place_and_keeps_its_other_settings(self): + from lxml import etree + inst = stopped_instance() - inst.change_network("52:54:00:00:00:02", "52:54:00:00:00:03", "default", "net", "e1000", "") + inst.change_network("52:54:00:00:00:02", "52:54:00:00:00:03", "br0", "bridge", "e1000", "clean-traffic") - detached = inst.instance.detachDeviceFlags.call_args[0][0] - self.assertIn("52:54:00:00:00:02", detached) - attached = inst.instance.attachDeviceFlags.call_args[0][0] - self.assertIn("52:54:00:00:00:03", attached) - self.assertIn("e1000", attached) + defined = etree.fromstring(inst.wvm.defineXML.call_args[0][0]) + first, second = 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") + self.assertEqual(second.find("source").attrib, {"bridge": "br0"}) + self.assertEqual(second.find("model").get("type"), "e1000") + self.assertEqual(second.find("filterref").get("filter"), "clean-traffic") + # F-06: QoS, link state and the PCI address survive + self.assertEqual(second.find("bandwidth/outbound").get("average"), "1000") + self.assertEqual(second.find("link").get("state"), "down") + self.assertEqual(second.find("address").get("bus"), "0x02") - def test_unknown_old_mac_adds_nothing(self): + def test_unknown_old_mac_changes_nothing(self): inst = stopped_instance() with self.assertRaises(libvirtError): inst.change_network("52:54:00:00:00:09", "52:54:00:00:00:03", "default", "net", "virtio", "") - inst.instance.detachDeviceFlags.assert_not_called() - inst.instance.attachDeviceFlags.assert_not_called() - - def test_failed_add_restores_the_old_nic(self): - inst = stopped_instance() - inst.instance.attachDeviceFlags.side_effect = [libvirtError("bad source"), 0] - with self.assertRaises(libvirtError): - inst.change_network("52:54:00:00:00:01", "52:54:00:00:00:03", "missing", "net", "virtio", "") - - restored, flags = inst.instance.attachDeviceFlags.call_args[0] - self.assertIn("52:54:00:00:00:01", restored) - self.assertEqual(flags, VIR_DOMAIN_AFFECT_CONFIG) + inst.wvm.defineXML.assert_not_called() class TestAddNetworkXml(unittest.TestCase): diff --git a/vrtManager/test_persistent_edits.py b/vrtManager/test_persistent_edits.py index 1c683d05..89af7769 100644 --- a/vrtManager/test_persistent_edits.py +++ b/vrtManager/test_persistent_edits.py @@ -42,5 +42,41 @@ class PersistentEditsTestCase(unittest.TestCase): self.assertIn("secret", str(defined)) +class XmlFidelityTestCase(unittest.TestCase): + def test_edits_keep_metadata_namespace_prefixes(self): + xml = ( + "v" + "hvm" + "" + ) + edits = { + "set_bootmenu": lambda vm: vm.set_bootmenu(1), + "set_bootorder": lambda vm: vm.set_bootorder({0: {"type": "disk", "dev": "vda"}}), + "set_console_keymap": lambda vm: vm.set_console_keymap("de"), + "set_console_passwd": lambda vm: vm.set_console_passwd("x"), + "set_console_listener_addr": lambda vm: vm.set_console_listener_addr("127.0.0.1"), + } + for name, edit in edits.items(): + with self.subTest(name): + vm = wvmInstance.__new__(wvmInstance) + vm.wvm = MagicMock() + vm._XMLDesc = MagicMock(return_value=xml) + edit(vm) + self.assertIn("