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

[Daniel Veillard]

On Mon, Aug 09, 2010 at 02:46:07PM +0200, Jiri Denemark wrote:
> 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);
>      }
>  

  ACK, this is not perfect as you pointed out but without QEmu support
we can't do any better I think.

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/