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

Osier Yang jyang at redhat.com
Wed May 2 18:06:01 UTC 2012


On 2012年05月01日 16:16, Guannan Ren wrote:
> One usb device could be allowed to hotplug in at a time. If user
> give a xml as follows.

s/give/gives/

Probably there are two usb devices avaiable

s/avaiable/available/

> 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;

s/usbDevice * usb/usbDevice *usb/

> +
>       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()))

virReportOOMError()

> +        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;

Should be removed. See [1]

> +
> +        } 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;

Should be removed. see [1]

> +        }
>
>           if (!usb)
> -            return -1;
> +            goto cleanup;

[1] There is a duplicate check here.

>
> -        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;

Do we really need a list here? there is only found usb device is , I guess
the only purpose is to take use of the new helper function
qemuPrepareHostdevUSBDevices, I.e. to use the new helper function,
it create a new list, and the only one usb device is inserted to
and removed from the list. IMO it's a waste.

>
> +        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;
>   }
>




More information about the libvir-list mailing list