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("