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@xxxxxxxxxx>
> ---
> 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;
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
