Re: [PATCH RESEND 16/20] virpci.c: use virPCIDeviceAddressPtr in virPCIDeviceListFindIndex()

[Laine Stump <laine@xxxxxxxxxx>]

On 1/18/21 2:53 PM, Daniel Henrique Barboza wrote:
> We're going to need a way to remove a PCI Device from a list without having
> a valid virPCIDevicePtr, because the device is missing from the host. This
> means that virPCIDevicesListDel() must operate with a PCI Device address
> instead.
>
> Turns out that virPCIDevicesListDel() and its related functions only use
> the virPCIDeviceAddressPtr of the virPCIDevicePtr, so this change is
> simple to do and will not cause hassle in all other callers. Let's
> start adapting virPCIDeviceListFindIndex() and crawl our way up to
> virPCIDevicesListDel().
>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>

Reviewed-by: Laine Stump <laine@xxxxxxxxxx>

(also Declared-should-have-been-done-this-way-in-the-first-place-by: 
Laine Stump <laine@xxxxxxxxxx> :-))

> ---
>   src/util/virpci.c | 15 ++++++++-------
>   src/util/virpci.h |  2 +-
>   2 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/src/util/virpci.c b/src/util/virpci.c
> index 7143380348..1554acffb6 100644
> --- a/src/util/virpci.c
> +++ b/src/util/virpci.c
> @@ -1745,7 +1745,7 @@ virPCIDevicePtr
>   virPCIDeviceListSteal(virPCIDeviceListPtr list,
>                         virPCIDevicePtr dev)
>   {
> -    return virPCIDeviceListStealIndex(list, virPCIDeviceListFindIndex(list, dev));
> +    return virPCIDeviceListStealIndex(list, virPCIDeviceListFindIndex(list, &dev->address));
>   }
>   
>   void
> @@ -1756,16 +1756,17 @@ virPCIDeviceListDel(virPCIDeviceListPtr list,
>   }
>   
>   int
> -virPCIDeviceListFindIndex(virPCIDeviceListPtr list, virPCIDevicePtr dev)
> +virPCIDeviceListFindIndex(virPCIDeviceListPtr list,
> +                          virPCIDeviceAddressPtr devAddr)
>   {
>       size_t i;
>   
>       for (i = 0; i < list->count; i++) {
>           virPCIDevicePtr other = list->devs[i];
> -        if (other->address.domain   == dev->address.domain &&
> -            other->address.bus      == dev->address.bus    &&
> -            other->address.slot     == dev->address.slot   &&
> -            other->address.function == dev->address.function)
> +        if (other->address.domain   == devAddr->domain &&
> +            other->address.bus      == devAddr->bus    &&
> +            other->address.slot     == devAddr->slot   &&
> +            other->address.function == devAddr->function)
>               return i;
>       }
>       return -1;
> @@ -1798,7 +1799,7 @@ virPCIDeviceListFind(virPCIDeviceListPtr list, virPCIDevicePtr dev)
>   {
>       int idx;
>   
> -    if ((idx = virPCIDeviceListFindIndex(list, dev)) >= 0)
> +    if ((idx = virPCIDeviceListFindIndex(list, &dev->address)) >= 0)
>           return list->devs[idx];
>       else
>           return NULL;
> diff --git a/src/util/virpci.h b/src/util/virpci.h
> index a9c597a428..8c6776da21 100644
> --- a/src/util/virpci.h
> +++ b/src/util/virpci.h
> @@ -175,7 +175,7 @@ virPCIDeviceListFindByIDs(virPCIDeviceListPtr list,
>                             unsigned int slot,
>                             unsigned int function);
>   int virPCIDeviceListFindIndex(virPCIDeviceListPtr list,
> -                              virPCIDevicePtr dev);
> +                              virPCIDeviceAddressPtr devAddr);
>   
>   /*
>    * Callback that will be invoked once for each file