On 05/01/2012 10:16 AM, 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.
>
> 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)
> {
> 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 = 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);
> +}
> +
> +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);
> +}
> +
> 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;
This looks good to me, but I'd prefer one more ACK with mine. I'll go
through the rest of the series, but the ACK assurance will be the same
as I'm not very familiar with this still.
BTW I had problems applying this patch to current head, I had to add a
newline in the middle of:
@@ -1085,6 +1085,8 @@ usbDeviceListNew;
and make it:
@@ -1085,6 +1085,8 @@
usbDeviceListNew;
Don't know how that happened, though, other patches apply fine :)
Martin
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list