Re: [libvirt] [PATCH] qemu: Hack around asynchronous device_del

device_del command is not synchronous for PCI devices, it merely asks the guest to release the device and returns. If the host wants to use that device before the guest actually releases it, we are in big trouble. To avoid this, we already added a loop which waits up to 10 seconds until the device is actually released before we do anything else with that device. But we only added this loop for managed PCI devices before we try reattach them back to the host. However, we need to wait even for non-managed devices. We don't reattach them automatically, but we still want to prevent the host from using it. This was revealed thanks to sVirt: when we relabel sysfs files corresponding to the PCI device before the guest finished releasing the device, qemu is no longer allowed to access those files and if it wants (as a result of guest's request) to write anything to them, it just exits, which kills the guest. This is not a proper fix and needs some further work both on libvirt and qemu side in the future. --- I was thinking about another possibility to implement this since the wait loop doesn't have to be connected to the reattach action. We could move that loop into a separate function and let qemudReattach function do just what its name suggest. But in that case, we would need to add the call to such function before every call to qemudReattach, which doesn't look any better to me. Jirka src/qemu/qemu_driver.c | 17 +++++++++-------- 1 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 57b8271..e4f47d4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3209,16 +3209,17 @@ qemuPrepareChardevDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, static void -qemudReattachManagedDevice(pciDevice *dev, struct qemud_driver *driver) +qemudReattachPciDevice(pciDevice *dev, struct qemud_driver *driver) { int retries = 100; + while (pciWaitForDeviceCleanup(dev, "kvm_assigned_device") + && retries) { + usleep(100*1000); + retries--; + } + if (pciDeviceGetManaged(dev)) { - while (pciWaitForDeviceCleanup(dev, "kvm_assigned_device") - && retries) { - usleep(100*1000); - retries--; - } if (pciReAttachDevice(dev, driver->activePciHostdevs) < 0) { virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to re-attach PCI device: %s"), @@ -3264,7 +3265,7 @@ qemuDomainReAttachHostdevDevices(struct qemud_driver *driver, for (i = 0; i < pciDeviceListCount(pcidevs); i++) { pciDevice *dev = pciDeviceListGet(pcidevs, i); - qemudReattachManagedDevice(dev, driver); + qemudReattachPciDevice(dev, driver); } pciDeviceListFree(pcidevs); @@ -8997,7 +8998,7 @@ static int qemudDomainDetachHostPciDevice(struct qemud_driver *driver, pciDeviceListDel(driver->activePciHostdevs, pci); if (pciResetDevice(pci, driver->activePciHostdevs, NULL) < 0) ret = -1; - qemudReattachManagedDevice(pci, driver); + qemudReattachPciDevice(pci, driver); pciFreeDevice(pci); }