[libvirt] [PATCH v3 07/10] conf: Add function virDomainUSBAddressIsAttachedToHub

[John Ferlan]

On 11/11/18 10:59 PM, Han Han wrote:
> Add this function to check if the a usb address is attached to a hub device.
> 
> Signed-off-by: Han Han <hhan at redhat.com>
> ---
>  src/conf/domain_addr.c   | 22 ++++++++++++++++++++++
>  src/conf/domain_addr.h   |  5 +++++
>  src/libvirt_private.syms |  1 +
>  3 files changed, 28 insertions(+)
> 

NB: Patches 1-6 were already Reviewed-by me, so I'll start here...

> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
> index e4ed143b76..722bd2c9fe 100644
> --- a/src/conf/domain_addr.c
> +++ b/src/conf/domain_addr.c
> @@ -2155,6 +2155,28 @@ virDomainUSBAddressEnsure(virDomainUSBAddressSetPtr addrs,
>  }
>  
>  
> +bool
> +virDomainUSBAddressIsAttachedToHub(virDomainDeviceInfoPtr info,
> +                                   virDomainHubDefPtr hub)
> +{
> +    unsigned int *hub_port = hub->info.addr.usb.port;
> +    unsigned int *device_port = info->addr.usb.port;
> +    size_t i;

These 3 can go inside the if or you could have done the reverse logic to:

    if (hub->info.addr.usb.bus != info->addr.usb.bus)
        return false;

> +    if (hub->info.addr.usb.bus == info->addr.usb.bus) {
> +        for (i = 0; i < VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH; ++i) {
> +            if (hub_port[i] == device_port[i])
> +                continue;
> +            else if (hub_port[i] == 0 && device_port[i] != 0)
> +                return true;
> +            else
> +                return false;

perhaps a brief comment about what each check means would help.

> +        }
> +    }
> +
> +    return false;
> +}
> +
> +
>  int
>  virDomainUSBAddressRelease(virDomainUSBAddressSetPtr addrs,
>                             virDomainDeviceInfoPtr info)
> diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
> index 2a9af9c00b..b1e0714923 100644
> --- a/src/conf/domain_addr.h
> +++ b/src/conf/domain_addr.h
> @@ -285,6 +285,11 @@ virDomainUSBAddressEnsure(virDomainUSBAddressSetPtr addrs,
>                            virDomainDeviceInfoPtr info)
>      ATTRIBUTE_NONNULL(2);
>  
> +bool
> +virDomainUSBAddressIsAttachedToHub(virDomainDeviceInfoPtr info,
> +                                   virDomainHubDefPtr hub)
> +    ATTRIBUTE_NONNULL(2);

I'm assuming a cut-n-paste, but since both @info and @hub are referenced
without checking - both would get the ATTRIBUTE_NONNULL even though it
really only matters if someone tries to pass a NULL to the function.

John

> +
>  int
>  virDomainUSBAddressRelease(virDomainUSBAddressSetPtr addrs,
>                             virDomainDeviceInfoPtr info)
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index b29c2bf62b..b45a7b92b4 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -131,6 +131,7 @@ virDomainPCIControllerModelToConnectType;
>  virDomainUSBAddressAssign;
>  virDomainUSBAddressCountAllPorts;
>  virDomainUSBAddressEnsure;
> +virDomainUSBAddressIsAttachedToHub;
>  virDomainUSBAddressPortFormatBuf;
>  virDomainUSBAddressPortIsValid;
>  virDomainUSBAddressPresent;
>