[libvirt] [PATCH 0/3] usb devices with same vendor, productID hotplug support
https://bugzilla.redhat.com/show_bug.cgi?id=815755 The set of patch tries to fix the issue when multiple usb devices with same idVendor, idProduct are availible on host, the usb device with lowest bus:device will be attached to guest if usb xml file is given like this: <hostdev mode='subsystem' type='usb' managed='yes'> <source> <vendor id='0x15e1'/> <product id='0x2007'/> </source> </hostdev> The reason is that the usb hotplug function searchs usb device in system files to match vendor and product id, the file with lowest number is always found first. After fix, in this case, libvirt will report an error like: At the same time, the usb part of domain initilization is also update in patch 2/3 # virsh attach-device rhel6u1 /tmp/usb.xml error: Failed to attach device from /tmp/usb.xml error: XML error: multiple USB deivces 15e1:2007, use <address> to specify one.
usbFindDevice():get usb device according to idVendor, idProduct, bus, device it is the most strict search usbFindDevByBus():get usb device according to bus, device it returns only one usb device same as usbFindDevice usbFindDevByVendor():get usb device according to idVendor,idProduct it probably returns multiple usb devices. usbDeivceSearch(): a helper function to do the actual search --- src/util/hostusb.c | 162 ++++++++++++++++++++++++++++++++++++++------------- src/util/hostusb.h | 20 ++++++- 2 files changed, 138 insertions(+), 44 deletions(-) diff --git a/src/util/hostusb.c b/src/util/hostusb.c index 92f52a2..73dc959 100644 --- a/src/util/hostusb.c +++ b/src/util/hostusb.c @@ -94,13 +94,22 @@ cleanup: return ret; } -static int usbFindBusByVendor(unsigned vendor, unsigned product, - unsigned *bus, unsigned *devno) +static usbDeviceList * +usbDeivceSearch(unsigned vendor, + unsigned product, + unsigned bus, + unsigned devno, + unsigned flags) { DIR *dir = NULL; - int ret = -1, found = 0; + int found = 0; char *ignore = NULL; struct dirent *de; + usbDeviceList *list = NULL, *ret = NULL; + usbDevice *usb; + + if (!(list = usbDeviceListNew())) + goto cleanup; dir = opendir(USB_SYSFS "/devices"); if (!dir) { @@ -111,48 +120,72 @@ static int usbFindBusByVendor(unsigned vendor, unsigned product, } while ((de = readdir(dir))) { - unsigned found_prod, found_vend; + unsigned found_prod, found_vend, found_bus, found_devno; + char *tmpstr = de->d_name; + if (de->d_name[0] == '.' || strchr(de->d_name, ':')) continue; if (usbSysReadFile("idVendor", de->d_name, 16, &found_vend) < 0) goto cleanup; + if (usbSysReadFile("idProduct", de->d_name, 16, &found_prod) < 0) goto cleanup; - if (found_prod == product && found_vend == vendor) { - /* Lookup bus.addr info */ - char *tmpstr = de->d_name; - unsigned found_bus, found_addr; + if (STRPREFIX(de->d_name, "usb")) + tmpstr += 3; - if (STRPREFIX(de->d_name, "usb")) - tmpstr += 3; + if (virStrToLong_ui(tmpstr, &ignore, 10, &found_bus) < 0) { + usbReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to parse dir name '%s'"), + de->d_name); + goto cleanup; + } + + if (usbSysReadFile("devnum", de->d_name, + 10, &found_devno) < 0) + goto cleanup; - if (virStrToLong_ui(tmpstr, &ignore, 10, &found_bus) < 0) { - usbReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to parse dir name '%s'"), - de->d_name); - goto cleanup; - } + switch (flags) { + /* + * Don't set found to 1 in order to continue the loop + * to find multiple USB devices with same idVendor and idProduct + */ + case USB_DEVICE_FIND_BY_VENDOR: + if (found_prod != product || found_vend != vendor) + continue; + break; - if (usbSysReadFile("devnum", de->d_name, - 10, &found_addr) < 0) - goto cleanup; + case USB_DEVICE_FIND_BY_BUS: + if (found_bus != bus || found_devno != devno) + continue; + found = 1; + break; - *bus = found_bus; - *devno = found_addr; + case USB_DEVICE_FIND_BY_ALL: + if (found_prod != product + || found_vend != vendor + || found_bus != bus + || found_devno != devno) + continue; found = 1; break; } - } - if (!found) - usbReportError(VIR_ERR_INTERNAL_ERROR, - _("Did not find USB device %x:%x"), vendor, product); - else - ret = 0; + usb = usbGetDevice(found_bus, found_devno); + if (!usb) + goto cleanup; + + if (usbDeviceListAdd(list, usb) < 0) { + usbFreeDevice(usb); + goto cleanup; + } + + if (found) break; + } + ret = list; cleanup: if (dir) { @@ -160,9 +193,69 @@ cleanup: closedir (dir); errno = saved_errno; } + + if (!ret) + usbDeviceListFree(list); return ret; } +usbDeviceList * +usbFindDevByVendor(unsigned vendor, unsigned product) +{ + + usbDeviceList *list; + if (!(list = usbDeivceSearch(vendor, product, 0 , 0, USB_DEVICE_FIND_BY_VENDOR))) + return NULL; + + if (list->count == 0) { + usbReportError(VIR_ERR_INTERNAL_ERROR, + _("Did not find USB device %x:%x"), vendor, product); + usbDeviceListFree(list); + return NULL; + } + + return list; +} + +usbDevice * +usbFindDevByBus(unsigned bus, unsigned devno) +{ + usbDeviceList *list; + if (!(list = usbDeivceSearch(0, 0, bus, devno, USB_DEVICE_FIND_BY_BUS))) + return NULL; + + if (list->count == 0) { + usbReportError(VIR_ERR_INTERNAL_ERROR, + _("Did not find USB device bus:%u device:%u"), bus, devno); + usbDeviceListFree(list); + return NULL; + } + + return usbDeviceListGet(list, 0); +} + +usbDevice * +usbFindDevice(unsigned vendor, + unsigned product, + unsigned bus, + unsigned devno) +{ + usbDeviceList *list; + if (!(list = usbDeivceSearch(vendor, product, + bus, devno, USB_DEVICE_FIND_BY_ALL))) + return NULL; + + if (list->count == 0) { + usbReportError(VIR_ERR_INTERNAL_ERROR, + _("Did not find USB device %x:%x bus:%u device:%u"), + vendor, product, bus, devno); + usbDeviceListFree(list); + return NULL; + } + + return usbDeviceListGet(list, 0); +} + usbDevice * usbGetDevice(unsigned bus, unsigned devno) @@ -207,21 +300,6 @@ usbGetDevice(unsigned bus, return dev; } - -usbDevice * -usbFindDevice(unsigned vendor, - unsigned product) -{ - unsigned bus = 0, devno = 0; - - if (usbFindBusByVendor(vendor, product, &bus, &devno) < 0) { - return NULL; - } - - return usbGetDevice(bus, devno); -} - - void usbFreeDevice(usbDevice *dev) { diff --git a/src/util/hostusb.h b/src/util/hostusb.h index afaa32f..7f18bce 100644 --- a/src/util/hostusb.h +++ b/src/util/hostusb.h @@ -28,10 +28,26 @@ typedef struct _usbDevice usbDevice; typedef struct _usbDeviceList usbDeviceList; +typedef enum { + USB_DEVICE_FIND_BY_VENDOR = 0, + USB_DEVICE_FIND_BY_BUS = 1 << 0, + USB_DEVICE_FIND_BY_ALL = 1 << 1, +} usbDeviceFindFlags; + usbDevice *usbGetDevice(unsigned bus, unsigned devno); -usbDevice *usbFindDevice(unsigned vendor, - unsigned product); + +usbDevice * usbFindDevByBus(unsigned bus, + unsigned devno); + +usbDeviceList * usbFindDevByVendor(unsigned vendor, + unsigned product); + +usbDevice * usbFindDevice(unsigned vendor, + unsigned product, + unsigned bus, + unsigned devno); + void usbFreeDevice (usbDevice *dev); void usbDeviceSetUsedBy(usbDevice *dev, const char *name); const char *usbDeviceGetUsedBy(usbDevice *dev); -- 1.7.7.5
On 04/28/2012 12:13 PM, Guannan Ren wrote:
usbFindDevice():get usb device according to idVendor, idProduct, bus, device it is the most strict search
usbFindDevByBus():get usb device according to bus, device it returns only one usb device same as usbFindDevice
usbFindDevByVendor():get usb device according to idVendor,idProduct it probably returns multiple usb devices.
usbDeivceSearch(): a helper function to do the actual search
Typo: s/Deivce/Device/ but unfortunately in the whole patch =)
--- src/util/hostusb.c | 162 ++++++++++++++++++++++++++++++++++++++------------- src/util/hostusb.h | 20 ++++++- 2 files changed, 138 insertions(+), 44 deletions(-)
diff --git a/src/util/hostusb.c b/src/util/hostusb.c index 92f52a2..73dc959 100644 --- a/src/util/hostusb.c +++ b/src/util/hostusb.c @@ -94,13 +94,22 @@ cleanup: return ret; }
-static int usbFindBusByVendor(unsigned vendor, unsigned product, - unsigned *bus, unsigned *devno) +static usbDeviceList * +usbDeivceSearch(unsigned vendor, + unsigned product, + unsigned bus, + unsigned devno, + unsigned flags) { DIR *dir = NULL; - int ret = -1, found = 0; + int found = 0; char *ignore = NULL; struct dirent *de; + usbDeviceList *list = NULL, *ret = NULL; + usbDevice *usb; + + if (!(list = usbDeviceListNew())) + goto cleanup;
dir = opendir(USB_SYSFS "/devices"); if (!dir) { @@ -111,48 +120,72 @@ static int usbFindBusByVendor(unsigned vendor, unsigned product, }
while ((de = readdir(dir))) { - unsigned found_prod, found_vend; + unsigned found_prod, found_vend, found_bus, found_devno; + char *tmpstr = de->d_name; + if (de->d_name[0] == '.' || strchr(de->d_name, ':')) continue;
if (usbSysReadFile("idVendor", de->d_name, 16, &found_vend) < 0) goto cleanup; + if (usbSysReadFile("idProduct", de->d_name, 16, &found_prod) < 0) goto cleanup;
- if (found_prod == product && found_vend == vendor) { - /* Lookup bus.addr info */ - char *tmpstr = de->d_name; - unsigned found_bus, found_addr; + if (STRPREFIX(de->d_name, "usb")) + tmpstr += 3;
- if (STRPREFIX(de->d_name, "usb")) - tmpstr += 3; + if (virStrToLong_ui(tmpstr, &ignore, 10, &found_bus) < 0) { + usbReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to parse dir name '%s'"), + de->d_name); + goto cleanup; + } + + if (usbSysReadFile("devnum", de->d_name, + 10, &found_devno) < 0) + goto cleanup;
- if (virStrToLong_ui(tmpstr, &ignore, 10, &found_bus) < 0) { - usbReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to parse dir name '%s'"), - de->d_name); - goto cleanup; - } + switch (flags) { + /* + * Don't set found to 1 in order to continue the loop + * to find multiple USB devices with same idVendor and idProduct + */ + case USB_DEVICE_FIND_BY_VENDOR: + if (found_prod != product || found_vend != vendor) + continue; + break;
- if (usbSysReadFile("devnum", de->d_name, - 10, &found_addr) < 0) - goto cleanup; + case USB_DEVICE_FIND_BY_BUS: + if (found_bus != bus || found_devno != devno) + continue; + found = 1; + break;
- *bus = found_bus; - *devno = found_addr; + case USB_DEVICE_FIND_BY_ALL: + if (found_prod != product + || found_vend != vendor + || found_bus != bus + || found_devno != devno) + continue; found = 1; break; } - }
- if (!found) - usbReportError(VIR_ERR_INTERNAL_ERROR, - _("Did not find USB device %x:%x"), vendor, product); - else - ret = 0; + usb = usbGetDevice(found_bus, found_devno); + if (!usb) + goto cleanup; + + if (usbDeviceListAdd(list, usb) < 0) { + usbFreeDevice(usb); + goto cleanup; + } + + if (found) break; + } + ret = list;
cleanup: if (dir) { @@ -160,9 +193,69 @@ cleanup: closedir (dir); errno = saved_errno; } + + if (!ret) + usbDeviceListFree(list); return ret; }
+usbDeviceList * +usbFindDevByVendor(unsigned vendor, unsigned product) +{ + + usbDeviceList *list; + if (!(list = usbDeivceSearch(vendor, product, 0 , 0, USB_DEVICE_FIND_BY_VENDOR))) + return NULL; + + if (list->count == 0) { + usbReportError(VIR_ERR_INTERNAL_ERROR, + _("Did not find USB device %x:%x"), vendor, product); + usbDeviceListFree(list); + return NULL; + } + + return list; +} + +usbDevice * +usbFindDevByBus(unsigned bus, unsigned devno) +{ + usbDeviceList *list; + if (!(list = usbDeivceSearch(0, 0, bus, devno, USB_DEVICE_FIND_BY_BUS))) + return NULL; + + if (list->count == 0) { + usbReportError(VIR_ERR_INTERNAL_ERROR, + _("Did not find USB device bus:%u device:%u"), bus, devno); + usbDeviceListFree(list); + return NULL; + } + + return usbDeviceListGet(list, 0); +} + +usbDevice * +usbFindDevice(unsigned vendor, + unsigned product, + unsigned bus, + unsigned devno) +{ + usbDeviceList *list; + if (!(list = usbDeivceSearch(vendor, product, + bus, devno, USB_DEVICE_FIND_BY_ALL))) + return NULL; + + if (list->count == 0) { + usbReportError(VIR_ERR_INTERNAL_ERROR, + _("Did not find USB device %x:%x bus:%u device:%u"), + vendor, product, bus, devno); + usbDeviceListFree(list); + return NULL; + } + + return usbDeviceListGet(list, 0); +} + usbDevice * usbGetDevice(unsigned bus, unsigned devno) @@ -207,21 +300,6 @@ usbGetDevice(unsigned bus, return dev; }
- -usbDevice * -usbFindDevice(unsigned vendor, - unsigned product) -{ - unsigned bus = 0, devno = 0; - - if (usbFindBusByVendor(vendor, product, &bus, &devno) < 0) { - return NULL; - } - - return usbGetDevice(bus, devno); -} - - void usbFreeDevice(usbDevice *dev) { diff --git a/src/util/hostusb.h b/src/util/hostusb.h index afaa32f..7f18bce 100644 --- a/src/util/hostusb.h +++ b/src/util/hostusb.h @@ -28,10 +28,26 @@ typedef struct _usbDevice usbDevice; typedef struct _usbDeviceList usbDeviceList;
+typedef enum { + USB_DEVICE_FIND_BY_VENDOR = 0, + USB_DEVICE_FIND_BY_BUS = 1 << 0, + USB_DEVICE_FIND_BY_ALL = 1 << 1, +} usbDeviceFindFlags; + usbDevice *usbGetDevice(unsigned bus, unsigned devno); -usbDevice *usbFindDevice(unsigned vendor, - unsigned product); + +usbDevice * usbFindDevByBus(unsigned bus, + unsigned devno); + +usbDeviceList * usbFindDevByVendor(unsigned vendor, + unsigned product); + +usbDevice * usbFindDevice(unsigned vendor, + unsigned product, + unsigned bus, + unsigned devno); + void usbFreeDevice (usbDevice *dev); void usbDeviceSetUsedBy(usbDevice *dev, const char *name); const char *usbDeviceGetUsedBy(usbDevice *dev);
On 04/30/2012 04:00 PM, Martin Kletzander wrote:
On 04/28/2012 12:13 PM, Guannan Ren wrote:
usbFindDevice():get usb device according to idVendor, idProduct, bus, device it is the most strict search
usbFindDevByBus():get usb device according to bus, device it returns only one usb device same as usbFindDevice
usbFindDevByVendor():get usb device according to idVendor,idProduct it probably returns multiple usb devices.
usbDeivceSearch(): a helper function to do the actual search Typo:
s/Deivce/Device/
but unfortunately in the whole patch =)
Thanks, fixed in v2. Guannan Ren
On 2012年04月28日 18:13, Guannan Ren wrote:
usbFindDevice():get usb device according to idVendor, idProduct, bus, device it is the most strict search
usbFindDevByBus():get usb device according to bus, device it returns only one usb device same as usbFindDevice
usbFindDevByVendor():get usb device according to idVendor,idProduct it probably returns multiple usb devices.
usbDeivceSearch(): a helper function to do the actual search --- src/util/hostusb.c | 162 ++++++++++++++++++++++++++++++++++++++------------- src/util/hostusb.h | 20 ++++++- 2 files changed, 138 insertions(+), 44 deletions(-)
Didn't review the patch yet, but apparently these new functions is not added in src/libvirt_private.syms. Osier
On 04/30/2012 09:55 PM, Osier Yang wrote:
On 2012年04月28日 18:13, Guannan Ren wrote:
usbFindDevice():get usb device according to idVendor, idProduct, bus, device it is the most strict search
usbFindDevByBus():get usb device according to bus, device it returns only one usb device same as usbFindDevice
usbFindDevByVendor():get usb device according to idVendor,idProduct it probably returns multiple usb devices.
usbDeivceSearch(): a helper function to do the actual search --- src/util/hostusb.c | 162 ++++++++++++++++++++++++++++++++++++++------------- src/util/hostusb.h | 20 ++++++- 2 files changed, 138 insertions(+), 44 deletions(-)
Didn't review the patch yet, but apparently these new functions is not added in src/libvirt_private.syms.
Osier
Thanks, added usbFindDevByBus and usbFindDevByVendor in libvirt_private.syms in v2. Guannan Ren
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 rollbakc 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); -- 1.7.7.5
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; } -- 1.7.7.5
participants (3)
-
Guannan Ren -
Martin Kletzander -
Osier Yang