[libvirt] [PATCH 3/7] hostdev: Use common reattach code in virHostdevPCINodeDeviceReAttach()

[Daniel P. Berrange]

On Tue, Jan 19, 2016 at 04:36:05PM +0100, Andrea Bolognani wrote:
> This ensures the behavior for reattach is consistent, no matter how it
> was triggered (eg. 'virsh nodedev-reattach', 'virsh detach-device' or
> shutdown of a domain that is configured to use hostdevs).
> ---
>  src/util/virhostdev.c | 45 +++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 39 insertions(+), 6 deletions(-)
> 
> diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
> index 267aa55..74c43f2 100644
> --- a/src/util/virhostdev.c
> +++ b/src/util/virhostdev.c
> @@ -1625,10 +1625,24 @@ virHostdevPCINodeDeviceDetach(virHostdevManagerPtr hostdev_mgr,
>      return ret;
>  }
>  
> +/**
> + * virHostdevPCINodeDeviceReAttach:
> + * @hostdev_mgr: hostdev manager
> + * @pci: PCI device
> + *
> + * Reattach a specific PCI device to the host.
> + *
> + * This function makes sure the device is not in use before reattaching it
> + * to the host; if the device has already been reattached to the host, the
> + * operation is considered successful.
> + *
> + * Returns: 0 on success, <0 on failure
> + */
>  int
>  virHostdevPCINodeDeviceReAttach(virHostdevManagerPtr hostdev_mgr,
>                                  virPCIDevicePtr pci)
>  {
> +    virPCIDeviceListPtr pcidevs = NULL;
>      struct virHostdevIsPCINodeDeviceUsedData data = {hostdev_mgr, NULL,
>                                                       false};
>      int ret = -1;
> @@ -1637,18 +1651,37 @@ virHostdevPCINodeDeviceReAttach(virHostdevManagerPtr hostdev_mgr,
>      virObjectLock(hostdev_mgr->inactivePCIHostdevs);
>  
>      if (virHostdevIsPCINodeDeviceUsed(virPCIDeviceGetAddress(pci), &data))
> -        goto out;
> +        goto cleanup;
>  
> -    virPCIDeviceReattachInit(pci);
> +    /* We want this function to be idempotent, so if the device has already
> +     * been removed from the inactive list (and is not active either, as
> +     * per the check above) just return right away */
> +    if (!virPCIDeviceListFind(hostdev_mgr->inactivePCIHostdevs, pci)) {
> +        VIR_DEBUG("PCI device %s is already attached to the host",
> +                  virPCIDeviceGetName(pci));
> +        ret = 0;
> +        goto cleanup;
> +    }
>  
> -    if (virPCIDeviceReattach(pci, hostdev_mgr->activePCIHostdevs,
> -                             hostdev_mgr->inactivePCIHostdevs) < 0)
> -        goto out;
> +    /* Create a temporary device list */
> +    if (!(pcidevs = virPCIDeviceListNew()))
> +        goto cleanup;
> +    if (virPCIDeviceListAddCopy(pcidevs, pci) < 0)
> +        goto cleanup;

This is rather crazy - if we need to reattach single PCI devices
we should have an API for doing that, and have the list API call
the individual API multiple times.

> +
> +    /* Reattach the device. We don't want to skip unmanaged devices in
> +     * this case, because the user explicitly asked for the device to
> +     * be reattached to the host driver */
> +    if (reattachPCIDevices(hostdev_mgr, pcidevs, false) < 0)
> +        goto cleanup;

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|