diff --git a/README.md b/README.md index 2fb82528..adfc8190 100644 --- a/README.md +++ b/README.md @@ -542,6 +542,7 @@ A rollback leaves the clone on a detached HEAD at the restored commit; the updat - On a host whose libvirt cannot change network interfaces (the udev backend: RHEL 9 and later, Debian 12), the interface pages offer no create, start, stop or delete; they only failed there. - On a running VM the disk list, its edit form, the QoS table, the title, description, CPU, boot order, video and console fields show the settings as they will be at the next start, so saving an edit no longer reverts another pending change. A boot order that names a device missing from that definition is refused instead of clearing the order. The settings tab of a running VM says so. A QoS edit keeps the inbound floor. Buttons shown as disabled while the VM runs can no longer be submitted with Enter. - A new disk or CD-ROM on a running VM gets a device name that neither the running VM nor its next-start settings use; it could take the name of a pending disk and stay attached until the next start only. +- A disk, NIC or guest agent added to a running VM is requested for both the running VM and its next-start settings in one libvirt call: a disk name or guest agent already pending is refused before anything changes, and a NIC added without a MAC gets the same MAC in both. A failure after the device is plugged can still leave it in the running VM only. - The VM page shows each NIC's own QoS and each CD-ROM's own pool (they showed the previous device's), and a NIC without a source or model no longer breaks it. - A volume of an LVM or iSCSI pool can be attached to a VM, and bridge and direct NICs can be boot devices. The NIC model `default` lets libvirt choose, and `rt18139` is `rtl8139`: a VM with either did not start; a migration corrects the default NIC type setting. - A paused VM's memory is resized as a running VM's: the current memory changes and the guest takes it once resumed, the maximum stays (both only reached the VM at its next boot). diff --git a/instances/test_live_dataloss.py b/instances/test_live_dataloss.py index d97b16e7..72315563 100644 --- a/instances/test_live_dataloss.py +++ b/instances/test_live_dataloss.py @@ -463,6 +463,58 @@ class LiveDataLossTestCase(TestCase): # the new volume stays in the pool: it is never deleted unasked self.assertIn(P + "sata-new.qcow2", self.conn.storagePoolLookupByName(livetest.POOL).listVolumes()) + def test_an_attach_that_conflicts_with_a_pending_device_changes_neither_definition(self): + controllers = "" + def disk(path): + return (f"" + "") + + agent = "" + cases = ( + ("disk", lambda n: disk(livetest.create_volume(self.conn, P + n + "-pending")), + lambda vm, n: vm.attach_disk("sda", livetest.create_volume(self.conn, P + n + "-new"), + target_bus="scsi", format_type="qcow2")), + ("agent", lambda n: agent, lambda vm, n: vm.add_guest_agent()), + ) + both = (0, libvirt.VIR_DOMAIN_XML_INACTIVE) + for paused in (False, True): + for case, pending, attach in cases: + with self.subTest(case=case, paused=paused): + name = f"attach-{case}-{int(paused)}" # own volumes: a leftover plug must not lock them + dom, inst = self.vm(name, [], extra_devices=controllers) + dom.create() + dom.attachDeviceFlags(pending(name), libvirt.VIR_DOMAIN_AFFECT_CONFIG) # pending: persistent only + if paused: + dom.suspend() + before = [dom.XMLDesc(flags) for flags in both] + with self.assertRaises(libvirt.libvirtError): + attach(inst.proxy, name) + self.assertEqual([dom.XMLDesc(flags) for flags in both], before) + + def test_a_nic_without_a_mac_gets_the_same_one_in_both_definitions(self): + from lxml import etree + + nic = "" + dom, inst = self.vm("nic-mac", [], extra_devices=nic) + dom.create() + self.post("add_network", inst, {"add-net-network": "net:default", "add-net-mac": "", "add-net-model": "virtio"}) + + def macs(flags): + return set(etree.fromstring(dom.XMLDesc(flags)).xpath("devices/interface/mac/@address")) - {"52:54:00:aa:18:09"} + + self.assertEqual(len(macs(0)), 1) + self.assertEqual(macs(libvirt.VIR_DOMAIN_XML_INACTIVE), macs(0)) + + def test_an_empty_scsi_cdrom_reaches_both_definitions_of_a_running_vm(self): + from lxml import etree + + dom, inst = self.vm("cdrom-scsi", [], extra_devices="") + dom.create() + self.post("add_cdrom", inst, {"bus": "scsi"}) + for flags in (0, libvirt.VIR_DOMAIN_XML_INACTIVE): + tree = etree.fromstring(dom.XMLDesc(flags)) + self.assertEqual(tree.xpath("devices/disk[@device='cdrom']/target/@dev"), ["sda"]) + def test_pending_qos_change_is_shown_and_kept_by_the_next_edit(self): from lxml import etree, html diff --git a/vrtManager/instance.py b/vrtManager/instance.py index de19e26c..a5ac0b52 100644 --- a/vrtManager/instance.py +++ b/vrtManager/instance.py @@ -993,8 +993,7 @@ class wvmInstance(wvmConnect): if serial is not None and serial != "None" and serial != "": xml_disk += f"""{serial}""" xml_disk += """""" - for _, affect in self._definitions(): - self.instance.attachDeviceFlags(xml_disk, affect) + self._attach_device(xml_disk) def detach_disk(self, target_dev): """Detach target_dev from each definition that has it, as that definition describes it.""" @@ -1883,10 +1882,21 @@ class wvmInstance(wvmConnect): etree.SubElement(iface, "model", type=model) if nwfilter: etree.SubElement(iface, "filterref", filter=nwfilter) - xml_iface = etree.tostring(iface).decode() + self._attach_device(etree.tostring(iface).decode()) - for _, affect in self._definitions(): - self.instance.attachDeviceFlags(xml_iface, affect) + def _attach_device(self, xml): + """Attach to the persistent definition and, when the VM is active, to + the running one, in one call. QEMU checks a copy of the persistent + definition before the hotplug, so a conflict there (a disk target or + an agent channel already pending) changes neither, and a NIC without + a MAC gets the same one in both. Still possible: a failure after a + successful hotplug leaves the device in the running VM only, and a + SCSI controller added for a disk that then fails to plug stays. An + error never proves that a new volume is unused.""" + flags = VIR_DOMAIN_AFFECT_CONFIG + if self.instance.isActive(): + flags |= VIR_DOMAIN_AFFECT_LIVE + self.instance.attachDeviceFlags(xml, flags) def _definitions(self): """(XMLDesc flags, affect flag) of the definitions a device edit applies to: @@ -2076,8 +2086,7 @@ class wvmInstance(wvmConnect): """ - for _, affect in self._definitions(): - self.instance.attachDeviceFlags(channel_xml, affect) + self._attach_device(channel_xml) def remove_guest_agent(self): for xml_flags, affect in self._definitions(): diff --git a/vrtManager/test_persistent_edits.py b/vrtManager/test_persistent_edits.py index 8f5f345b..c9eb64b3 100644 --- a/vrtManager/test_persistent_edits.py +++ b/vrtManager/test_persistent_edits.py @@ -64,38 +64,30 @@ class XmlFidelityTestCase(unittest.TestCase): edit(vm) self.assertIn("vm """ class DeviceEditsByStateTestCase(unittest.TestCase): - """Disk and agent channel changes reach the running definition whenever the - VM is active, paused included, and the persistent one always.""" + """Disk, NIC and agent channel changes reach the running definition + whenever the VM is active, paused included, and the persistent one + always. An attach is one call for both; a removal is one per definition, + from that definition's own XML.""" EDITS = { "attach_disk": ("attachDeviceFlags", lambda vm: vm.attach_disk("vdb", "/var/lib/libvirt/images/d.qcow2")), + "add_network": ("attachDeviceFlags", lambda vm: vm.add_network("52:54:00:00:00:09", "default")), "add_guest_agent": ("attachDeviceFlags", lambda vm: vm.add_guest_agent()), "remove_guest_agent": ("detachDeviceFlags", lambda vm: vm.remove_guest_agent()), } + ATTACHES = ("attach_disk", "add_network", "add_guest_agent") def affected(self, state, active): from libvirt import VIR_DOMAIN_AFFECT_CONFIG, VIR_DOMAIN_AFFECT_LIVE - names = {VIR_DOMAIN_AFFECT_LIVE: "live", VIR_DOMAIN_AFFECT_CONFIG: "config"} + names = {VIR_DOMAIN_AFFECT_LIVE: "live", VIR_DOMAIN_AFFECT_CONFIG: "config", + VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG: "live+config"} result = {} for name, (method, edit) in self.EDITS.items(): vm = wvmInstance.__new__(wvmInstance) @@ -109,16 +101,29 @@ class DeviceEditsByStateTestCase(unittest.TestCase): def test_paused_vm(self): for name, affected in self.affected(state=3, active=True).items(): - self.assertEqual(affected, ["live", "config"], name) + self.assertEqual(affected, ["live+config"] if name in self.ATTACHES else ["live", "config"], name) def test_running_vm(self): for name, affected in self.affected(state=1, active=True).items(): - self.assertEqual(affected, ["live", "config"], name) + self.assertEqual(affected, ["live+config"] if name in self.ATTACHES else ["live", "config"], name) def test_stopped_vm(self): for name, affected in self.affected(state=5, active=False).items(): self.assertEqual(affected, ["config"], name) + def test_a_refused_attach_is_not_tried_again(self): + from libvirt import libvirtError + + for name in self.ATTACHES: + with self.subTest(name): + vm = wvmInstance.__new__(wvmInstance) + vm.instance = MagicMock() + vm.instance.isActive.return_value = True + vm.instance.attachDeviceFlags.side_effect = libvirtError("target sda already exists") + with self.assertRaises(libvirtError): + self.EDITS[name][1](vm) + vm.instance.attachDeviceFlags.assert_called_once() + if __name__ == "__main__": unittest.main()