mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-10 04:06:47 +00:00
fix(instances): attach a disk, NIC or guest agent to both definitions in one call
On an active VM a device was attached in two calls, running VM first. When the persistent attach was refused, for example because a disk target or an agent channel was already pending there, the device stayed in the running VM only and was gone at the next start. A NIC added without a MAC got a different generated MAC in each definition. One call with AFFECT_LIVE|AFFECT_CONFIG lets QEMU check the persistent copy before the hotplug and keeps one MAC. Detach and update still go per definition, each from its own XML.
This commit is contained in:
parent
e924f3e7a0
commit
87ffed66b9
4 changed files with 92 additions and 25 deletions
|
|
@ -993,8 +993,7 @@ class wvmInstance(wvmConnect):
|
|||
if serial is not None and serial != "None" and serial != "":
|
||||
xml_disk += f"""<serial>{serial}</serial>"""
|
||||
xml_disk += """</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):
|
|||
<target type='virtio' name='org.qemu.guest_agent.0'/>
|
||||
</channel>
|
||||
"""
|
||||
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():
|
||||
|
|
|
|||
|
|
@ -64,38 +64,30 @@ class XmlFidelityTestCase(unittest.TestCase):
|
|||
edit(vm)
|
||||
self.assertIn("<app:info", vm.wvm.defineXML.call_args[0][0])
|
||||
|
||||
def test_paused_vm_gets_nic_changes_live_and_persistent(self):
|
||||
from libvirt import VIR_DOMAIN_AFFECT_CONFIG, VIR_DOMAIN_AFFECT_LIVE
|
||||
|
||||
vm = wvmInstance.__new__(wvmInstance)
|
||||
vm.instance = MagicMock()
|
||||
vm.instance.isActive.return_value = True
|
||||
vm.get_status = MagicMock(return_value=3) # paused
|
||||
vm.add_network("52:54:00:00:00:09", "default", "net", "virtio")
|
||||
self.assertEqual(
|
||||
[c.args[1] for c in vm.instance.attachDeviceFlags.call_args_list],
|
||||
[VIR_DOMAIN_AFFECT_LIVE, VIR_DOMAIN_AFFECT_CONFIG],
|
||||
)
|
||||
|
||||
|
||||
AGENT_DOMAIN = """<domain><name>vm</name><devices><channel type='unix'>
|
||||
<target type='virtio' name='org.qemu.guest_agent.0'/></channel></devices></domain>"""
|
||||
|
||||
|
||||
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()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue