On 05/01/2012 10:16 AM, Guannan Ren wrote:
> refactor qemuPrepareHostdevUSBDevices function, make it focus on
> adding usb device to activeUsbHostdevs after check. After that,
> the usb hotplug function qemuDomainAttachHostDevice also could use
> it.
>
> expand qemuPrepareHostUSBDevices to perform the usb search,
> rollback on failure.
> ---
> src/qemu/qemu_hostdev.c | 118 ++++++++++++++++++++++++++++++-----------------
> src/qemu/qemu_hostdev.h | 3 +-
> 2 files changed, 76 insertions(+), 45 deletions(-)
>
> diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
> index 8594fb2..a3d4ad4 100644
> --- a/src/qemu/qemu_hostdev.c
> +++ b/src/qemu/qemu_hostdev.c
> @@ -565,13 +565,51 @@ qemuPrepareHostPCIDevices(struct qemud_driver *driver,
> int
> qemuPrepareHostdevUSBDevices(struct qemud_driver *driver,
> const char *name,
> - virDomainHostdevDefPtr *hostdevs,
> - int nhostdevs)
> + usbDeviceList *list)
> {
> - int ret = -1;
> int i;
> + usbDevice *tmp;
> +
> + for (i = 0; i < usbDeviceListCount(list); i++) {
> + usbDevice *usb = usbDeviceListGet(list, i);
> + if ((tmp = usbDeviceListFind(driver->activeUsbHostdevs, usb))) {
> + const char *other_name = usbDeviceGetUsedBy(tmp);
> +
> + if (other_name)
> + qemuReportError(VIR_ERR_OPERATION_INVALID,
> + _("USB device %s is in use by domain %s"),
> + usbDeviceGetName(tmp), other_name);
> + else
> + qemuReportError(VIR_ERR_OPERATION_INVALID,
> + _("USB device %s is already in use"),
> + usbDeviceGetName(tmp));
> + return -1;
> + }
> +
> + usbDeviceSetUsedBy(usb, name);
> + VIR_DEBUG("Adding %03d.%03d dom=%s to activeUsbHostdevs",
> + usbDeviceGetBus(usb), usbDeviceGetDevno(usb), name);
> + /*
> + * The caller is responsible to steal the list of usb
> + * from the usbDeviceList that passed in on success,
> + * perform rollback on failure.
> + */
> + if (usbDeviceListAdd(driver->activeUsbHostdevs, usb) < 0)
> + return -1;
> +
> + }
> + return 0;
> +}
> +
> +static int
> +qemuPrepareHostUSBDevices(struct qemud_driver *driver,
> + virDomainDefPtr def)
> +{
> + int i, ret = -1;
> usbDeviceList *list;
> usbDevice *tmp;
> + virDomainHostdevDefPtr *hostdevs = def->hostdevs;
> + int nhostdevs = def->nhostdevs;
>
> /* To prevent situation where USB device is assigned to two domains
> * we need to keep a list of currently assigned USB devices.
> @@ -586,64 +624,65 @@ qemuPrepareHostdevUSBDevices(struct qemud_driver *driver,
> */
> for (i = 0 ; i < nhostdevs ; i++) {
> virDomainHostdevDefPtr hostdev = hostdevs[i];
> + usbDevice *usb = NULL;
>
> if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
> continue;
> if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
> continue;
>
> - /* Resolve a vendor/product to bus/device */
> - if (hostdev->source.subsys.u.usb.vendor) {
> - usbDevice *usb
> - = usbFindDevice(hostdev->source.subsys.u.usb.vendor,
> - hostdev->source.subsys.u.usb.product);
> + 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;
>
> - if ((tmp = usbDeviceListFind(driver->activeUsbHostdevs, usb))) {
> - const char *other_name = usbDeviceGetUsedBy(tmp);
> -
> - if (other_name)
> - qemuReportError(VIR_ERR_OPERATION_INVALID,
> - _("USB device %s is in use by domain %s"),
> - usbDeviceGetName(tmp), other_name);
> - else
> - qemuReportError(VIR_ERR_OPERATION_INVALID,
> - _("USB device %s is already in use"),
> - usbDeviceGetName(tmp));
> - usbFreeDevice(usb);
> + } 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);
>
> hostdev->source.subsys.u.usb.bus = usbDeviceGetBus(usb);
> hostdev->source.subsys.u.usb.device = usbDeviceGetDevno(usb);
>
> - if (usbDeviceListAdd(list, usb) < 0) {
> - usbFreeDevice(usb);
> + } else if (!vendor && bus) {
> + usb = usbFindDevByBus(bus, device);
> + if (!usb)
> goto cleanup;
> - }
> + }
>
> + if (!usb)
> + goto cleanup;
> +
> + if (usbDeviceListAdd(list, usb) < 0) {
> + usbFreeDevice(usb);
> + goto cleanup;
> }
> }
>
> - /* Loop 2: Mark devices in temporary list as used by @name
> + /* Mark devices in temporary list as used by @name
> * and add them do driver list. However, if something goes
> * wrong, perform rollback.
> */
> - for (i = 0; i < usbDeviceListCount(list); i++) {
> - tmp = usbDeviceListGet(list, i);
> - usbDeviceSetUsedBy(tmp, name);
> + if (qemuPrepareHostdevUSBDevices(driver, def->name, list) < 0)
> + goto inactivedevs;
>
> - VIR_DEBUG("Adding %03d.%03d dom=%s to activeUsbHostdevs",
> - usbDeviceGetBus(tmp), usbDeviceGetDevno(tmp), name);
> - if (usbDeviceListAdd(driver->activeUsbHostdevs, tmp) < 0) {
> - usbFreeDevice(tmp);
> - goto inactivedevs;
> - }
> - }
> -
> - /* Loop 3: Temporary list was successfully merged with
> + /* Loop 2: Temporary list was successfully merged with
> * driver list, so steal all items to avoid freeing them
> * in cleanup label.
> */
> @@ -669,13 +708,6 @@ cleanup:
> return ret;
> }
>
> -static int
> -qemuPrepareHostUSBDevices(struct qemud_driver *driver,
> - virDomainDefPtr def)
> -{
> - return qemuPrepareHostdevUSBDevices(driver, def->name, def->hostdevs, def->nhostdevs);
> -}
> -
> int qemuPrepareHostDevices(struct qemud_driver *driver,
> virDomainDefPtr def)
> {
> diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h
> index 371630a..a8acccf 100644
> --- a/src/qemu/qemu_hostdev.h
> +++ b/src/qemu/qemu_hostdev.h
> @@ -38,8 +38,7 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver,
> int nhostdevs);
> int qemuPrepareHostdevUSBDevices(struct qemud_driver *driver,
> const char *name,
> - virDomainHostdevDefPtr *hostdevs,
> - int nhostdevs);
> + usbDeviceList *list);
> int qemuPrepareHostDevices(struct qemud_driver *driver,
> virDomainDefPtr def);
> void qemuReattachPciDevice(pciDevice *dev, struct qemud_driver *driver);
This looks good also, ACK.
Martin
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list