[libvirt] [PATCH 2/2] libxl: allow vendor/product addressing for USB hostdevs

[Jim Fehlig]

On 08/05/2016 01:10 AM, Cédric Bosdonnat wrote:
> libxl only has API to address the host USB devices by bus/device.
> Find the bus/device if the user only provided the vendor/product
> of the USB device.
> ---
>  src/libxl/libxl_conf.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 5202ca1..4b758f1 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -1559,23 +1559,32 @@ int
>  libxlMakeUSB(virDomainHostdevDefPtr hostdev, libxl_device_usbdev *usbdev)
>  {
>      virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb;
> +    virUSBDevicePtr usb;

Needs initialized to NULL. I encountered a libvirtd segfault without it.

> +    int ret = -1;
>  
>      if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
> -        return -1;
> +        goto cleanup;
>      if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
> -        return -1;
> +        goto cleanup;
>  
> -    if (usbsrc->bus <= 0 || usbsrc->device <= 0) {
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                       _("libxenlight supports only USB device "
> -                         "specified by busnum:devnum"));
> -        return -1;
> +    if ((usbsrc->bus <= 0 || usbsrc->device <= 0) &&
> +        (virHostdevFindUSBDevice(hostdev, true, &usb) < 0)) {
> +        virReportError(VIR_ERR_OPERATION_FAILED,
> +                       _("failed to find USB device busnum:devnum "
> +                         "for %x:%x"),
> +                       usbsrc->vendor, usbsrc->product);
> +        goto cleanup;
>      }
>  
>      usbdev->u.hostdev.hostbus = usbsrc->bus;
>      usbdev->u.hostdev.hostaddr = usbsrc->device;

I had to read virHostdevFindUSBDevice to understand how this would work :-). If
the device is found by vendor/product, virHostdevFindUSBDevice sets 
hostdev->source.subsys.u.usb->{bus,device}. I don't think we should rely on that
internal behavior and instead set hostbus and hostaddr from usb->{bus,dev}.

Regards,
Jim

>  
> -    return 0;
> +    ret = 0;
> +
> + cleanup:
> +    virUSBDeviceFree(usb);
> +
> +    return ret;
>  }
>  
>  static int