[libvirt] [PATCH 15/24] hostdev: Remove virHostdevGetActivePCIHostDeviceList()

[John Ferlan]

On 03/07/2016 12:24 PM, Andrea Bolognani wrote:
> virHostdevGetPCIHostDeviceList() is similar but does not filter out
> devices that are not in the active list; that said, we are looking
> up the device in the active list just a few lines after anyway, so
> we might as well just keep a single function around.
> 
> This also helps stress the fact the objects contained in pcidevs are
> only for looking up the actual devices, which is something later
> commits will make even more explicit.
> ---
>  src/util/virhostdev.c | 50 ++++----------------------------------------------
>  1 file changed, 4 insertions(+), 46 deletions(-)
> 


Existing code uses virPCIDeviceListAddCopy (as does code that populates
inactiveDevs list).  The AddCopy code will

1. virPCIDeviceCopy(virPCIDevicePtr dev)
   VIR_ALLOC(copy);
   *copy = *dev;
   Update copy->path, copy->used_by_drvname, & copy->used_by_domname

2. Add to a virPCIDeviceListPtr

The new code.

1. virPCIDeviceNew for a virPCIDevicePtr pci;
   VIR_ALLOC(dev);
   copy in dev->address
   generate dev->name, dev->id (similar to copy)
   generate dev->path from dev->name

2. Add to a virPCIDeviceListPtr

3. Caller sets the managed and stubDriver backend.

NOTE: there's no copy of 'used_by_drvname' and 'used_by_domname', like
the copy function nor are a few other fields set.  This seems to be
important later [1].

> diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
> index fef7898..67e6e7b 100644
> --- a/src/util/virhostdev.c
> +++ b/src/util/virhostdev.c
> @@ -245,49 +245,6 @@ virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs)
>  }
>  
>  
> -/*
> - * virHostdevGetActivePCIHostDeviceList - make a new list with a *copy* of
> - *   every virPCIDevice object that is found on the activePCIHostdevs
> - *   list *and* is in the hostdev list for this domain.
> - *
> - * Return the new list, or NULL if there was a failure.
> - *
> - * Pre-condition: activePCIHostdevs is locked
> - */
> -static virPCIDeviceListPtr
> -virHostdevGetActivePCIHostDeviceList(virHostdevManagerPtr mgr,
> -                                     virDomainHostdevDefPtr *hostdevs,
> -                                     int nhostdevs)
> -{
> -    virPCIDeviceListPtr list;
> -    size_t i;
> -
> -    if (!(list = virPCIDeviceListNew()))
> -        return NULL;
> -
> -    for (i = 0; i < nhostdevs; i++) {
> -        virDomainHostdevDefPtr hostdev = hostdevs[i];
> -        virDevicePCIAddressPtr addr;
> -        virPCIDevicePtr activeDev;
> -
> -        if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
> -            continue;
> -        if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
> -            continue;
> -
> -        addr = &hostdev->source.subsys.u.pci.addr;
> -        activeDev = virPCIDeviceListFindByIDs(mgr->activePCIHostdevs,
> -                                              addr->domain, addr->bus,
> -                                              addr->slot, addr->function);
> -        if (activeDev && virPCIDeviceListAddCopy(list, activeDev) < 0) {
> -            virObjectUnref(list);
> -            return NULL;
> -        }
> -    }
> -
> -    return list;
> -}
> -
>  static int
>  virHostdevPCISysfsPath(virDomainHostdevDefPtr hostdev,
>                         char **sysfs_path)
> @@ -800,9 +757,7 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr,
>      virObjectLock(hostdev_mgr->activePCIHostdevs);
>      virObjectLock(hostdev_mgr->inactivePCIHostdevs);
>  
> -    if (!(pcidevs = virHostdevGetActivePCIHostDeviceList(hostdev_mgr,
> -                                                         hostdevs,
> -                                                         nhostdevs))) {
> +    if (!(pcidevs = virHostdevGetPCIHostDeviceList(hostdevs, nhostdevs))) {
>          virErrorPtr err = virGetLastError();
>          VIR_ERROR(_("Failed to allocate PCI device list: %s"),
>                    err ? err->message : _("unknown error"));
> @@ -832,6 +787,9 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr,

[1] This code checks for usedby_drvname and usedby_domname

These are set by virHostdevPreparePCIDevices and
virHostdevUpdateActivePCIDevices.  The virHostdevPreparePCIDevices is
the only current caller of virHostdevGetPCIHostDeviceList.  The latter
will call the virPCIDeviceNew and then set the Managed, UsedBy, and
stubDriver fields.

The PreparePCIDevices code seems to do many of the same functions for
the activePCIHostdevs.

I think this one needs to be rethought..


John
>                  virPCIDeviceListDel(pcidevs, dev);
>                  continue;
>              }
> +        } else {
> +            virPCIDeviceListDel(pcidevs, dev);
> +            continue;
>          }
>  
>          VIR_DEBUG("Removing PCI device %s from active list",
>