On 2012年05月01日 16:16, 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
Should we name it as usbFindDevByAddress? Given that both 'bus'
and 'device' are used to find the device actually.
it returns only one usb device same as usbFindDevice
usbFindDevByVendor():get usb device according to idVendor,idProduct
it probably returns multiple usb devices.
usbDeviceSearch(): 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 *
+usbDeviceSearch(unsigned vendor,
+ unsigned product,
+ unsigned bus,
+ unsigned devno,
+ unsigned flags)
Though no difference between 'unsigned' and 'unsigned int',
expect more typing, should we keep consistent across the
project? I.e. Don't introduce new 'unsigned', and substitute
it with 'unsigned int' as a follow up patch.
{
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()))
virReportOOMError();
+ 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)
As a habit:
if (found_prod != product ||
found_vend != vendor ||
found_bus != bus ||
found_devno != devno)
+ continue;
found = 1;
Given 'found' can only be '0' and '1', why not boolean?
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;
It's desired to be:
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 = usbDeviceSearch(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 = usbDeviceSearch(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);
The left items in the list are leaked. (In case of there are
multiple devices found)
+}
+
+usbDevice *
+usbFindDevice(unsigned vendor,
+ unsigned product,
+ unsigned bus,
+ unsigned devno)
+{
+ usbDeviceList *list;
+ if (!(list = usbDeviceSearch(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);
Likewise.
+}
+
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);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 025816a..290dad7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1085,6 +1085,8 @@ usbDeviceListNew;
usbDeviceListSteal;
usbDeviceSetUsedBy;
usbFindDevice;
+usbFindDevByBus;
+usbFindDevByVendor;
usbFreeDevice;
usbGetDevice;
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list