[libvirt] [PATCH v2 1/4] hostdev: Streamline device ownership tracking

[John Ferlan]

On 03/18/2016 01:03 PM, Andrea Bolognani wrote:
> After this patch, ownership of virPCIDevice instances is very easy
> to keep track of: for each host PCI device, the only instance that
> actually matters is the one inside one of the bookkeeping list.
> 
> Whenever some operation needs to be performed on a PCI device, the
> actual device is looked up first; when this is not the case, a
> comment explains the reason.
> ---
>  src/util/virhostdev.c | 130 ++++++++++++++++++++++++++++----------------------
>  1 file changed, 72 insertions(+), 58 deletions(-)
> 

ACK - couple of minor textual edits if you're interested below

John
> diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
> index 572aec0..e8efc53 100644
> --- a/src/util/virhostdev.c
> +++ b/src/util/virhostdev.c

[...]

>   inactivedevs:
> -    /* Only steal all the devices from activePCIHostdevs. We will
> -     * free them in virObjectUnref().
> -     */
> +    /* Move devices back to the inactive list so that they can be
> +     * processed properly below (reattachdevs label) */
>      for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
>          virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i);
> +        virPCIDevicePtr actual;
>  
>          VIR_DEBUG("Removing PCI device %s from active list",
>                    virPCIDeviceGetName(pci));
> -        virPCIDeviceListSteal(mgr->activePCIHostdevs, pci);
> +        if (!(actual = virPCIDeviceListSteal(mgr->activePCIHostdevs, pci)))
> +            continue;
> +
> +        VIR_DEBUG("Adding PCI device %s to inactive list",
> +                  virPCIDeviceGetName(pci));
> +        if (virPCIDeviceListAdd(mgr->inactivePCIHostdevs, actual) < 0)
> +            VIR_WARN("Failed to add PCI device %s to the inactive list",

or "Failed to return PCI device ..."


> +                     virPCIDeviceGetName(pci));
>      }
>  

[...]

> +    /* Step 2: Move devices from the active list to the inactive list */
> +    for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
> +        virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i);
> +        virPCIDevicePtr actual;
> +
>          VIR_DEBUG("Removing PCI device %s from active list",
>                    virPCIDeviceGetName(pci));
> -        virPCIDeviceListDel(mgr->activePCIHostdevs, pci);
> -        i++;
> +        actual = virPCIDeviceListSteal(mgr->activePCIHostdevs, pci);
> +
> +        VIR_DEBUG("Adding PCI device %s to inactive list",

...to the inactive list

> +                  virPCIDeviceGetName(pci));
> +        if (!actual ||
> +            virPCIDeviceListAdd(mgr->inactivePCIHostdevs, actual) < 0) {
> +
> +            virErrorPtr err = virGetLastError();
> +            VIR_ERROR(_("Failed to add PCI device %s to inactive list"),
> +                      err ? err->message : _("unknown error"));
> +            virResetError(err);
> +        }
>      }
>