5:13 a.m.
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.
5:13 a.m.
New subject: [libvirt] [PATCH 1/3] usb: create functions to search usb device accurately
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
3 a.m.
New subject: [libvirt] [PATCH 1/3] usb: create functions to search usb device accurately
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);
3:25 a.m.
New subject: [libvirt] [PATCH 1/3] usb: create functions to search usb device accurately
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
8:55 a.m.
New subject: [libvirt] [PATCH 1/3] usb: create functions to search usb device accurately
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
3:27 a.m.
New subject: [libvirt] [PATCH 1/3] usb: create functions to search usb device accurately
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
5:13 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: make use of usb search function to initialize usb devices
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
5:13 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: search usb device accurately to improve usb device hotplug
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
4587
days inactive
4590
days old
7 comments
3 participants
participants (3)
-
Guannan Ren -
Martin Kletzander -
Osier Yang