[libvirt] [PATCH v2 3/3] qemu: search usb device accurately to improve usb device hotplug

[Martin Kletzander]

On 05/01/2012 10:16 AM, Guannan Ren wrote:
> One usb device could be allowed to hotplug in at a time. If user
> give a xml as follows. Probably there are two usb devices avaiable
> but with different value of "bus, device"
> 
> we give a error to let user use <address> to specify the desired one.
> 
> <hostdev mode='subsystem' type='usb' managed='yes'>
>   <source>
>     <vendor id='0x15e1'/>
>     <product id='0x2007'/>
>   </source>
> </hostdev>
> ---
>  src/qemu/qemu_hotplug.c |   68 +++++++++++++++++++++++++++++++++++++---------
>  1 files changed, 54 insertions(+), 14 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 7cf7b90..15693a0 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1121,6 +1121,9 @@ int qemuDomainAttachHostDevice(struct qemud_driver *driver,
>                                 virDomainObjPtr vm,
>                                 virDomainHostdevDefPtr hostdev)
>  {
> +    usbDeviceList *list;
> +    usbDevice * usb = NULL;
> +
>      if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
>          qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                          _("hostdev mode '%s' not supported"),
> @@ -1128,29 +1131,62 @@ int qemuDomainAttachHostDevice(struct qemud_driver *driver,
>          return -1;
>      }
>  
> -    /* Resolve USB product/vendor to bus/device */
> -    if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB &&
> -        hostdev->source.subsys.u.usb.vendor) {
> -        if (qemuPrepareHostdevUSBDevices(driver, vm->def->name, &hostdev, 1) < 0)
> -            goto error;
> +    if (!(list = usbDeviceListNew()))
> +        goto cleanup;
> +
> +    if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
> +        unsigned vendor = hostdev->source.subsys.u.usb.vendor;
> +        unsigned product = hostdev->source.subsys.u.usb.product;
> +        unsigned bus = hostdev->source.subsys.u.usb.bus;
> +        unsigned device = hostdev->source.subsys.u.usb.device;
> +
> +        if (vendor && bus) {
> +            usb = usbFindDevice(vendor, product, bus, device);
> +            if (!usb)
> +                goto cleanup;
> +
> +        } else if (vendor && !bus) {
> +            usbDeviceList *devs = usbFindDevByVendor(vendor, product);
> +            if (!devs)
> +                goto cleanup;
> +
> +            if (usbDeviceListCount(devs) > 1) {
> +                qemuReportError(VIR_ERR_XML_ERROR,
> +                                _("multiple USB deivces %x:%x, "
> +                                  "use <address> to specify one"), vendor, product);
> +                usbDeviceListFree(devs);
> +                goto cleanup;
> +            }
> +            usb = usbDeviceListGet(devs, 0);
> +            usbDeviceListSteal(devs, usb);
> +            usbDeviceListFree(devs);
>  
> -        usbDevice *usb
> -            = usbFindDevice(hostdev->source.subsys.u.usb.vendor,
> -                            hostdev->source.subsys.u.usb.product);
> +            hostdev->source.subsys.u.usb.bus = usbDeviceGetBus(usb);
> +            hostdev->source.subsys.u.usb.device = usbDeviceGetDevno(usb);
> +
> +        } else if (!vendor && bus) {
> +            usb = usbFindDevByBus(bus, device);
> +            if (!usb)
> +                goto cleanup;
> +        }
>  
>          if (!usb)
> -            return -1;
> +            goto cleanup;
>  
> -        hostdev->source.subsys.u.usb.bus = usbDeviceGetBus(usb);
> -        hostdev->source.subsys.u.usb.device = usbDeviceGetDevno(usb);
> +        if (usbDeviceListAdd(list, usb) < 0) {
> +            usbFreeDevice(usb);
> +            goto cleanup;
> +        }
>  
> -        usbFreeDevice(usb);
> -    }
> +        if (qemuPrepareHostdevUSBDevices(driver, vm->def->name, list) < 0)
> +            goto cleanup;
>  
> +        usbDeviceListSteal(list, usb);
> +    }
>  
>      if (virSecurityManagerSetHostdevLabel(driver->securityManager,
>                                            vm->def, hostdev) < 0)
> -        return -1;
> +        goto cleanup;
>  
>      switch (hostdev->source.subsys.type) {
>      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
> @@ -1172,6 +1208,7 @@ int qemuDomainAttachHostDevice(struct qemud_driver *driver,
>          goto error;
>      }
>  
> +    usbDeviceListFree(list);
>      return 0;
>  
>  error:
> @@ -1179,6 +1216,9 @@ error:
>                                                vm->def, hostdev) < 0)
>          VIR_WARN("Unable to restore host device labelling on hotplug fail");
>  
> +cleanup:
> +    usbDeviceListFree(list);
> +    usbDeviceListSteal(driver->activeUsbHostdevs, usb);
>      return -1;
>  }
>  

Seems all is well for me, ACK.

Martin