diff --git a/instances/templates/instances/host_devices_form.html b/instances/templates/instances/host_devices_form.html index 5e1ae585..51ada794 100644 --- a/instances/templates/instances/host_devices_form.html +++ b/instances/templates/instances/host_devices_form.html @@ -14,15 +14,16 @@
{% csrf_token %} - {% for device in host_pci %} {% endfor %} - +
- {% if running %}{% trans "Shut the VM down, without a saved state, to change its PCI devices." %} + {% if pci_frozen %}{% trans "A paused VM, or one with a saved state, does not change its PCI devices." %} + {% elif running %}{% trans "The device is plugged into the running VM at once and kept for its next start; libvirt binds it to vfio-pci. It needs a free slot: on a q35 VM a free pcie-root-port, added while the VM is shut off." %} {% else %}{% trans "libvirt binds the device to vfio-pci when the VM starts, and gives it back to the host when the VM stops; a device bound to vfio-pci before stays bound to it." %}{% endif %}
diff --git a/instances/templates/instances/settings_tab.html b/instances/templates/instances/settings_tab.html index e6ebbbee..c8e3b530 100644 --- a/instances/templates/instances/settings_tab.html +++ b/instances/templates/instances/settings_tab.html @@ -662,8 +662,8 @@
{% csrf_token %} + {% if instance.status != 1 and instance.status != 5 or instance.has_managed_save_image %}disabled title="{% trans "A paused VM, or one with a saved state, does not change its PCI devices" %}"{% endif %} + data-confirm="{% trans "Remove this PCI device from the VM? A running VM's guest has to let it go." %}" onclick="return confirm(this.dataset.confirm)">{% bs_icon 'trash' %}
{% endif %} diff --git a/instances/tests/test_host_devices.py b/instances/tests/test_host_devices.py index 8e26c489..b62223d0 100644 --- a/instances/tests/test_host_devices.py +++ b/instances/tests/test_host_devices.py @@ -22,6 +22,8 @@ from vrtManager.tests.test_migration import FakeSource, destination, with_disks MIGRATABLE = libvirt.VIR_DOMAIN_XML_MIGRATABLE CONTROLLER = "" +PCI_05 = ("" + "
") BY_MODEL = "{address}" @@ -38,7 +40,8 @@ def vm(running, saved, active=True, migratable=None): the migratable XML marks it), and whose next start is saved.""" proxy = wvmInstance.__new__(wvmInstance) proxy._read_cache = None - proxy.instance = MagicMock(**{"isActive.return_value": active, "hasManagedSaveImage.return_value": 0}) + proxy.instance = MagicMock(**{"isActive.return_value": active, "hasManagedSaveImage.return_value": 0, + "info.return_value": [libvirt.VIR_DOMAIN_RUNNING if active else libvirt.VIR_DOMAIN_SHUTOFF]}) def describe(flags=0): if flags & libvirt.VIR_DOMAIN_XML_INACTIVE: @@ -100,30 +103,30 @@ class UsbHostdevTests(SimpleTestCase): {"type": "mdev", "address": "00000000-0000-4000-8000-00000000a001", "now": True, "next": False}, {"type": "mdev", "address": "00000000-0000-4000-8000-00000000a002", "now": False, "next": True}]) - def test_a_pci_device_is_given_and_taken_back_only_while_the_vm_is_shut_off(self): + def test_a_shut_off_vm_gets_a_pci_device_for_its_next_start(self): proxy = vm("", domain(), active=False) proxy.add_pci_hostdev("0000:05:00.0") xml, flags = proxy.instance.attachDeviceFlags.call_args.args self.assertEqual(flags, libvirt.VIR_DOMAIN_AFFECT_CONFIG) self.assertEqual((etree.fromstring(xml).get("managed"), dict(etree.fromstring(xml).find("source/address").attrib)), ("yes", {"domain": "0x0000", "bus": "0x05", "slot": "0x00", "function": "0x0"})) - pci = "
" - proxy = vm("", domain(pci), active=False) - proxy.delete_pci_hostdev("0000:05:00.0") - self.assertEqual(proxy.instance.detachDeviceFlags.call_args.args[1], libvirt.VIR_DOMAIN_AFFECT_CONFIG) + proxy = vm("", domain(PCI_05), active=False) + self.assertFalse(proxy.delete_pci_hostdev("0000:05:00.0")) + self.assertEqual([c.args[1] for c in proxy.instance.detachDeviceFlags.call_args_list], [libvirt.VIR_DOMAIN_AFFECT_CONFIG]) with self.assertRaisesRegex(OperationError, "no PCI device 0000:06:00.0"): proxy.delete_pci_hostdev("0000:06:00.0") - running = vm(domain(pci), domain(pci)) - for change in (running.add_pci_hostdev, running.delete_pci_hostdev): - with self.assertRaisesRegex(OperationError, "Shut the VM down"): - change("0000:05:00.0") - running.instance.attachDeviceFlags.assert_not_called() - running.instance.detachDeviceFlags.assert_not_called() - # a saved state would start the VM without the change - saved = vm("", domain(pci), active=False) + + def test_a_paused_vm_or_one_with_a_saved_state_keeps_its_pci_devices(self): + paused = vm(domain(PCI_05), domain(PCI_05)) + paused.instance.info.return_value = [libvirt.VIR_DOMAIN_PAUSED] + saved = vm("", domain(PCI_05), active=False) saved.instance.hasManagedSaveImage.return_value = 1 - with self.assertRaisesRegex(OperationError, "without a saved state"): - saved.add_pci_hostdev("0000:06:00.0") + for name, proxy in (("paused", paused), ("saved", saved)): + for change in (proxy.add_pci_hostdev, proxy.delete_pci_hostdev): + with self.subTest(vm=name, change=change.__name__), self.assertRaisesRegex(OperationError, "runs, or is shut off"): + change("0000:05:00.0") + proxy.instance.attachDeviceFlags.assert_not_called() + proxy.instance.detachDeviceFlags.assert_not_called() def test_a_nic_that_is_a_host_device_is_left_to_the_xml_editor(self): nic = "
" @@ -145,6 +148,122 @@ class UsbHostdevTests(SimpleTestCase): self.assertEqual(tree.xpath("devices/interface/mac/@address"), ["52:54:00:00:00:1b"]) +class RunningVM: + """A running VM whose definitions change as libvirt changes them: the + running one and the next start's, each with the PCI device or not. + fail: the affect flag whose change libvirt fails, after doing it or not; + keeps: the guest does not let the device go.""" + + def __init__(self, now=True, later=True, fail=None, done=False, keeps=False, machine="pc-i440fx-10.1"): + self.has = {libvirt.VIR_DOMAIN_AFFECT_LIVE: now, libvirt.VIR_DOMAIN_AFFECT_CONFIG: later} + self.fail, self.done, self.keeps, self.machine = fail, done, keeps, machine + self.proxy = vm("", "") + self.proxy.instance.XMLDesc.side_effect = self.describe + self.proxy.instance.attachDeviceFlags.side_effect = lambda xml, flags: self.change(flags, True) + self.proxy.instance.detachDeviceFlags.side_effect = lambda xml, flags: self.change(flags, False) + + def describe(self, flags=0): + saved = flags & libvirt.VIR_DOMAIN_XML_INACTIVE + has = self.has[libvirt.VIR_DOMAIN_AFFECT_CONFIG if saved else libvirt.VIR_DOMAIN_AFFECT_LIVE] + return f"hvm{PCI_05 if has else ''}" + + def change(self, flags, adding): + for affect in (libvirt.VIR_DOMAIN_AFFECT_LIVE, libvirt.VIR_DOMAIN_AFFECT_CONFIG): + if flags & affect: + if affect == self.fail: + if self.done: + self.has[affect] = adding + raise libvirt.libvirtError("refused") + if adding or not (self.keeps and affect == libvirt.VIR_DOMAIN_AFFECT_LIVE): + self.has[affect] = adding + + +class PciHotplugTests(SimpleTestCase): + def test_a_running_vm_gets_it_at_once_and_for_its_next_start(self): + running = RunningVM(now=False, later=False) + running.proxy.add_pci_hostdev("0000:05:00.0") + self.assertEqual(running.proxy.instance.attachDeviceFlags.call_args.args[1], + libvirt.VIR_DOMAIN_AFFECT_LIVE | libvirt.VIR_DOMAIN_AFFECT_CONFIG) + self.assertEqual(list(running.has.values()), [True, True]) + + def test_a_hot_plug_that_fails_says_where_the_device_is(self): + cases = ((libvirt.VIR_DOMAIN_AFFECT_CONFIG, False, "the running VM has it, its next start does not have it"), + (libvirt.VIR_DOMAIN_AFFECT_CONFIG, True, "the running VM has it, its next start has it"), + (libvirt.VIR_DOMAIN_AFFECT_LIVE, False, "the running VM does not have it, its next start does not have it")) + for fail, done, where in cases: + # libvirt plugs into the running VM first, then saves the next start + running = RunningVM(now=False, later=False, fail=fail, done=done) + if fail == libvirt.VIR_DOMAIN_AFFECT_CONFIG: + running.has[libvirt.VIR_DOMAIN_AFFECT_LIVE] = True + with self.subTest(where=where), self.assertRaisesRegex(OperationError, f"refused; PCI device 0000:05:00.0: {where}"): + running.proxy.add_pci_hostdev("0000:05:00.0") + + def test_a_hot_plug_whose_outcome_cannot_be_read_says_so(self): + running = RunningVM(now=False, later=False) + running.proxy.instance.attachDeviceFlags.side_effect = libvirt.libvirtError("connection reset") + running.proxy.instance.XMLDesc.side_effect = libvirt.libvirtError("connection reset") + with self.assertRaisesRegex(OperationError, "connection reset; whether the VM has PCI device 0000:05:00.0 now is not known"): + running.proxy.add_pci_hostdev("0000:05:00.0") + + def test_a_vm_that_stops_meanwhile_is_said_to_be_not_running(self): + running = RunningVM(now=False, later=False, fail=libvirt.VIR_DOMAIN_AFFECT_CONFIG) + running.proxy.instance.isActive.return_value = False # its live XML is then the inactive one + with self.assertRaisesRegex(OperationError, "the VM is not running, its next start does not have it"): + running.proxy.add_pci_hostdev("0000:05:00.0") + + def test_a_running_vm_lets_it_go_as_its_guest_does(self): + running = RunningVM() + self.assertFalse(running.proxy.delete_pci_hostdev("0000:05:00.0")) + self.assertEqual(list(running.has.values()), [False, False]) + # not yet: the request stands, the next start no longer has it + running = RunningVM(keeps=True) + self.assertTrue(running.proxy.delete_pci_hostdev("0000:05:00.0")) + self.assertEqual(list(running.has.values()), [True, False]) + + def test_a_guest_that_refuses_keeps_it_for_its_next_start_too(self): + running = RunningVM(fail=libvirt.VIR_DOMAIN_AFFECT_LIVE) + with self.assertRaisesRegex(OperationError, "refused; .*the running VM has it, its next start has it"): + running.proxy.delete_pci_hostdev("0000:05:00.0") + self.assertEqual(list(running.has.values()), [True, True]) + # removed from the running VM, then an error: said, and the next start keeps it + running = RunningVM(fail=libvirt.VIR_DOMAIN_AFFECT_LIVE, done=True) + with self.assertRaisesRegex(OperationError, "the running VM does not have it, its next start has it"): + running.proxy.delete_pci_hostdev("0000:05:00.0") + + def test_a_failed_removal_from_the_next_start_of_a_running_vm_is_said(self): + running = RunningVM(now=False, later=True, fail=libvirt.VIR_DOMAIN_AFFECT_CONFIG) + with self.assertRaisesRegex(OperationError, "the running VM does not have it, its next start has it"): + running.proxy.delete_pci_hostdev("0000:05:00.0") + + def test_a_removal_whose_outcome_cannot_be_read_says_so(self): + running = RunningVM() + reads = [] + + def describe(flags=0): # the reads before the removal work, the one after it does not + reads.append(flags) + if len(reads) > 2: + raise libvirt.libvirtError("connection reset") + return running.describe(flags) + running.proxy.instance.XMLDesc.side_effect = describe + with self.assertRaisesRegex(OperationError, "connection reset; whether the VM has PCI device 0000:05:00.0 now is not known"): + running.proxy.delete_pci_hostdev("0000:05:00.0") + + def test_a_next_start_that_keeps_it_after_the_running_vm_let_it_go_is_said(self): + for keeps, where in ((False, "the running VM does not have it, its next start has it"), + (True, "the running VM has it, its next start has it")): + running = RunningVM(fail=libvirt.VIR_DOMAIN_AFFECT_CONFIG, keeps=keeps) + with self.subTest(keeps=keeps), self.assertRaisesRegex(OperationError, where): + running.proxy.delete_pci_hostdev("0000:05:00.0") + + def test_a_device_in_one_definition_only_is_taken_from_it(self): + for now, later in ((True, False), (False, True)): + running = RunningVM(now=now, later=later) + with self.subTest(now=now, later=later): + self.assertFalse(running.proxy.delete_pci_hostdev("0000:05:00.0")) + self.assertEqual(list(running.has.values()), [False, False]) + self.assertEqual(running.proxy.instance.detachDeviceFlags.call_count, 1) + + class MigrationTests(unittest.TestCase): def test_a_vm_with_host_devices_is_not_migrated(self): for devices, state in ((by_model(), 1), (by_model(), 5), ("", 1)): @@ -209,11 +328,42 @@ class HostUsbViewTests(TestCase): self.client.post(reverse("instances:delete_pci_hostdev", args=[self.instance.id]), {"address": "0000:00:0a.0"}) self.proxy.delete_pci_hostdev.assert_called_once_with("0000:00:0a.0") - def test_the_pci_form_waits_for_a_shut_off_vm(self): - self.proxy.instance.isActive.return_value = True - page = html.fromstring(self.client.get(reverse("instances:host_devices", args=[self.instance.id])).content) - self.assertEqual(page.xpath("//select[@name='pci_device']/@disabled"), ["disabled"]) - self.assertIn("Shut the VM down", page.text_content()) + def test_the_pci_form_waits_for_a_vm_that_runs_or_is_shut_off(self): + url = reverse("instances:host_devices", args=[self.instance.id]) + for state, saved, frozen in ((libvirt.VIR_DOMAIN_RUNNING, 0, False), (libvirt.VIR_DOMAIN_SHUTOFF, 0, False), + (libvirt.VIR_DOMAIN_PAUSED, 0, True), (libvirt.VIR_DOMAIN_SHUTOFF, 1, True)): + self.proxy.instance.info.return_value = [state] + self.proxy.instance.hasManagedSaveImage.return_value = saved + page = html.fromstring(self.client.get(url).content) + with self.subTest(state=state, saved=saved): + self.assertEqual(page.xpath("//select[@name='pci_device']/@disabled"), ["disabled"] if frozen else []) + + def test_a_removal_is_logged_as_done_or_as_requested(self): + from logs.models import Logs + + url = reverse("instances:delete_pci_hostdev", args=[self.instance.id]) + self.proxy.delete_pci_hostdev.return_value = False + self.client.post(url, {"address": "0000:00:0a.0"}) + self.proxy.delete_pci_hostdev.return_value = True # the guest has not let it go yet + response = self.client.post(url, {"address": "0000:00:0a.0"}) + self.assertIn("the guest has not let it go yet", " ".join(str(m) for m in get_messages(response.wsgi_request))) + rows = list(Logs.objects.filter(instance="kiosk-1").order_by("id").values_list("message", flat=True)) + self.assertEqual(rows, ["VM kiosk-1: remove PCI device 0000:00:0a.0", + "VM kiosk-1: request the removal of PCI device 0000:00:0a.0"]) + + def test_a_running_vm_is_given_a_device_alone_a_shut_off_one_with_its_group(self): + from nodedevs.tests.test_pci_rules import NODES as GROUPS + + # the next start has the GPU's audio function managed: it goes along when the VM starts, not at a hot plug + self.proxy.wvm.listAllDevices.return_value = [node(name, etree.tostring(n)) for name, n in GROUPS.items()] + self.proxy.domain_trees.side_effect = lambda: [] + self.proxy.get_hostdevs.return_value = ([], [{"address": "0000:00:0e.0", "now": False, "next": True, "managed": True}], []) + url = reverse("instances:host_devices", args=[self.instance.id]) + for active, offered in ((False, True), (True, False)): + self.proxy.instance.isActive.return_value = active + page = html.fromstring(self.client.get(url).content) + with self.subTest(active=active): + self.assertEqual(page.xpath("//option[@value='0000:00:0d.0']/@disabled") == [], offered) def test_a_device_is_taken_back_by_its_selector(self): url = reverse("instances:delete_usb_hostdev", args=[self.instance.id]) diff --git a/instances/views.py b/instances/views.py index 0e8c1fbd..d11b211b 100755 --- a/instances/views.py +++ b/instances/views.py @@ -34,6 +34,8 @@ from django.utils import timezone from django.utils.translation import gettext_noop as _ from django.views.decorators.http import require_POST from libvirt import (VIR_DOMAIN_PAUSED, + VIR_DOMAIN_RUNNING, + VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_UNDEFINE_KEEP_NVRAM, VIR_DOMAIN_UNDEFINE_MANAGED_SAVE, VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, @@ -1801,11 +1803,16 @@ def _host_devices(instance): """The host's PCI and USB devices, each with why it cannot be given to the VM (pci_problems, hostdev_problems; none: it can). A USB device is named by its address when the host has more than one of its model, else - by model; a PCI device is checked with the managed PCI devices of the - VM's next start, as libvirt takes them from the host along with it.""" + by model; a PCI device is checked alone on a running VM, as it is + hot-plugged, and with the managed PCI devices of the VM's next start on + a shut-off one, as libvirt takes them from the host along with it.""" pci, usb = node_devices(instance.proxy) own_pci = instance.proxy.get_hostdevs()[1] - managed = {row["address"] for row in own_pci if row["managed"] and row["next"]} + # libvirt takes a hot-plugged device alone from the host, and a group's managed devices as the VM starts + if instance.proxy.instance.isActive(): + managed = set() + else: + managed = {row["address"] for row in own_pci if row["managed"] and row["next"]} by_address = {device["address"]: device for device in pci} for device in pci: device["problems"] = pci_problems(instance.name, device, by_address, managed | {device["address"]}) @@ -1828,8 +1835,10 @@ def host_devices(request, pk): try: pci, usb = _host_devices(instance) domain = instance.proxy.instance - context = {"instance": instance, "host_pci": pci, "host_usb": usb, - "running": domain.isActive() or domain.hasManagedSaveImage(0)} + state = domain.info()[0] + context = {"instance": instance, "host_pci": pci, "host_usb": usb, "running": state == VIR_DOMAIN_RUNNING, + # paused, or with a saved state the VM would start from without the change + "pci_frozen": state not in (VIR_DOMAIN_RUNNING, VIR_DOMAIN_SHUTOFF) or domain.hasManagedSaveImage(0)} except libvirtError as err: context = {"instance": instance, "error": err} return render(request, "instances/host_devices_form.html", context) @@ -1859,8 +1868,9 @@ def add_usb_hostdev(request, pk): @superuser_only @serialize_compute_mutation def add_pci_hostdev(request, pk): - """Gives the shut-off VM a PCI device of its host; checked under the - compute's claim, so no other VM gets it or its IOMMU group meanwhile.""" + """Gives the VM a PCI device of its host, hot-plugged while it runs; + checked under the compute's claim, so no other VM gets it or its IOMMU + group meanwhile.""" instance = get_instance(request.user, pk) chosen = request.POST.get("pci_device", "") device = next((d for d in _host_devices(instance)[0] if d["address"] == chosen), None) @@ -1880,8 +1890,11 @@ def add_pci_hostdev(request, pk): def delete_pci_hostdev(request, pk): instance = get_instance(request.user, pk) address = request.POST.get("address", "") - with audit_vm(request, instance, f"remove PCI device {address}"): - instance.proxy.delete_pci_hostdev(address) + with audit_vm(request, instance, f"remove PCI device {address}") as entry: + if instance.proxy.delete_pci_hostdev(address): + entry.action = f"request the removal of PCI device {address}" + messages.warning(request, _("Removal of PCI device %(address)s requested: the guest has not let it go yet; " + "it stays in the running VM until it does or the VM stops.") % {"address": address}) return _back(request, pk, "hostdevs") diff --git a/vrtManager/instance.py b/vrtManager/instance.py index c4751659..6b54a08d 100644 --- a/vrtManager/instance.py +++ b/vrtManager/instance.py @@ -15,6 +15,8 @@ try: VIR_DOMAIN_AFFECT_CONFIG, VIR_DOMAIN_AFFECT_LIVE, VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP, + VIR_DOMAIN_RUNNING, + VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_XML_SECURE, VIR_DOMAIN_XML_INACTIVE, VIR_DOMAIN_XML_MIGRATABLE, @@ -2433,39 +2435,81 @@ class wvmInstance(wvmConnect): row[where] = True return list(usb.values()), list(pci.values()), list(others.values()) - def _pci_hostdev(self, address): - """The persistent definition's of the PCI device at address, or None.""" - tree = etree.fromstring(self._XMLDesc(PERSISTENT_XML)) + def _pci_hostdev(self, address, xml_flags=PERSISTENT_XML): + """A definition's of the PCI device at address, or None.""" + tree = etree.fromstring(self._XMLDesc(xml_flags)) for found in tree.xpath(PCI_HOSTDEVS + "/source/address"): if pci_address(found.get("domain", "0"), found.get("bus"), found.get("slot"), found.get("function")) == address: return found.getparent().getparent() return None - def _check_shut_off_for_pci(self): - """PCI devices change in the persistent definition of a shut-off VM; - a saved state (managed save) would start the VM without the change.""" - if self.instance.isActive() or self.instance.hasManagedSaveImage(0): - raise util.OperationError("Shut the VM down, without a saved state, to change its PCI devices") + def _pci_running(self): + """Whether a PCI change goes to the running VM too: True while it + runs, False while it is shut off. A paused VM, or one with a saved + state (managed save, which would start without the change), is + refused.""" + status = self.instance.info()[0] + if self.instance.hasManagedSaveImage(0) or status not in (VIR_DOMAIN_RUNNING, VIR_DOMAIN_SHUTOFF): + raise util.OperationError("A VM changes its PCI devices while it runs, or is shut off without a saved state") + return status == VIR_DOMAIN_RUNNING + + def _pci_outcome(self, address, err): + """An OperationError with err and where the PCI device at address is + now: in the running VM, in its next start; the outcome is unknown + when the VM cannot be read.""" + try: + running = self.instance.isActive() + now = running and self._pci_hostdev(address, VIR_DOMAIN_XML_SECURE) is not None + later = self._pci_hostdev(address) is not None + except libvirtError: + return util.OperationError(f"{err}; whether the VM has PCI device {address} now is not known") + next_start = "its next start has it" if later else "its next start does not have it" + if not running: + return util.OperationError(f"{err}; PCI device {address}: the VM is not running, {next_start}") + now_text = "the running VM has it" if now else "the running VM does not have it" + return util.OperationError(f"{err}; PCI device {address}: {now_text}, {next_start}") def add_pci_hostdev(self, address): - """Gives the shut-off VM the host's PCI device at address - (0000:05:00.0): libvirt binds it to vfio-pci when the VM starts and - gives it back to the host when it stops.""" - self._check_shut_off_for_pci() + """Gives the VM the host's PCI device at address (0000:05:00.0), for + its next start and, while it runs, at once: libvirt binds it to + vfio-pci, and gives it back when the VM stops or lets it go. A hot + plug that fails says where the device is (the two are not changed + together).""" + running = self._pci_running() domain, bus, slot_function = address.split(":") slot, function = slot_function.split(".") hostdev = etree.Element("hostdev", mode="subsystem", type="pci", managed="yes") etree.SubElement(etree.SubElement(hostdev, "source"), "address", domain=f"0x{domain}", bus=f"0x{bus}", slot=f"0x{slot}", function=f"0x{function}") - self.instance.attachDeviceFlags(etree.tostring(hostdev, encoding="unicode"), VIR_DOMAIN_AFFECT_CONFIG) + flags = VIR_DOMAIN_AFFECT_CONFIG | (VIR_DOMAIN_AFFECT_LIVE if running else 0) + try: + self.instance.attachDeviceFlags(etree.tostring(hostdev, encoding="unicode"), flags) + except libvirtError as err: + if not running: + raise + raise self._pci_outcome(address, err) from None def delete_pci_hostdev(self, address): - """Takes the PCI device at address from the shut-off VM, also one the host no longer has.""" - self._check_shut_off_for_pci() - hostdev = self._pci_hostdev(address) - if hostdev is None: + """Takes the PCI device at address from the VM: from the running VM + (the guest has to let it go; libvirt waits a few seconds for it) and + from its next start, each as it has it. Returns True when the guest + has not let it go yet: it stays in the running VM until it does or + the VM stops. A failure on a running VM says where the device is.""" + running = self._pci_running() + live = self._pci_hostdev(address, VIR_DOMAIN_XML_SECURE) if running else None + saved = self._pci_hostdev(address) + if live is None and saved is None: raise util.OperationError(f"The VM has no PCI device {address}") - self.instance.detachDeviceFlags(etree.tostring(hostdev, encoding="unicode"), VIR_DOMAIN_AFFECT_CONFIG) + try: + if live is not None: # a refusal stops here; a request the guest has not answered goes on + self.instance.detachDeviceFlags(etree.tostring(live, encoding="unicode"), VIR_DOMAIN_AFFECT_LIVE) + if saved is not None: + self.instance.detachDeviceFlags(etree.tostring(saved, encoding="unicode"), VIR_DOMAIN_AFFECT_CONFIG) + return live is not None and self._pci_hostdev(address, VIR_DOMAIN_XML_SECURE) is not None + except libvirtError as err: + if not running: + raise + raise self._pci_outcome(address, err) from None def add_usb_hostdev(self, vendor, product, address=None): """Gives the VM the host's USB device of vendor and product, at