[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/3] usb: create functions to search usb device accurately



On 05/04/2012 05:38 AM, Eric Blake wrote:
On 05/03/2012 03:21 PM, Eric Blake wrote:
On 05/03/2012 04:51 AM, Guannan Ren wrote:
usbFindDevice():get usb device according to
                 idVendor, idProduct, bus, device
                 it is the exact match of the four parameters

usbFindDeviceByBus():get usb device according to bus, device
                   it returns only one usb device same as usbFindDevice

usbFindDeviceByVendor():get usb device according to idVendor,idProduct
                      it probably returns multiple usb devices.

usbDeviceSearch(): a helper function to do the actual search
---
  src/libvirt_private.syms |    2 +
  src/util/hostusb.c       |  209 +++++++++++++++++++++++++++++++++-------------
  src/util/hostusb.h       |   22 ++++--
  3 files changed, 170 insertions(+), 63 deletions(-)
This patch doesn't even compile!

qemu/qemu_hostdev.c: In function 'qemuPrepareHostdevUSBDevices':
qemu/qemu_hostdev.c:599:33: error: too few arguments to function
'usbFindDevice'
../src/util/hostusb.h:40:13: note: declared here

You can't change the signature of usbFindDevice unless you update all
callers in the same patch.

        I compiled it and tested it , then sent it out.
I think the changes on usbFindDevice caller is in PATCH2/3 and PATCH 3/3
        "usb = usbFindDevice(vendor, product, bus, device);" are there.








By the way, here are some suggestions I have for your patch:

diff --git i/src/util/hostusb.c w/src/util/hostusb.c
index cad0a6c..533b9c7 100644
--- i/src/util/hostusb.c
+++ w/src/util/hostusb.c
@@ -1,5 +1,5 @@
  /*
- * Copyright (C) 2009-2011 Red Hat, Inc.
+ * Copyright (C) 2009-2012 Red Hat, Inc.
   *
   * This library is free software; you can redistribute it and/or
   * modify it under the terms of the GNU Lesser General Public
@@ -65,9 +65,10 @@ struct _usbDeviceList {
  };

  typedef enum {
-    USB_DEVICE_ALL = 0,
-    USB_DEVICE_FIND_BY_VENDOR = 1,
-    USB_DEVICE_FIND_BY_BUS = 2,
+    USB_DEVICE_ALL            = 0,      /* no filtering */
+    USB_DEVICE_FIND_BY_VENDOR = 1<<  0, /* filter to vendor matches */
+    USB_DEVICE_FIND_BY_BUS    = 1<<  1, /* filter to bus matches */
+    USB_DEVICE_FIND_ONE       = 1<<  2, /* filter to first match */


       This patch is great, but "USB_DEVICE_FIND_ONE" is not very useful.
The bus, devno number could identify usb device enough, the bool 'found' just stop the loop because we are sure that the rest of usb devices are no need
       to search.




  } usbDeviceFindFlags;

  static int usbSysReadFile(const char *f_name, const char *d_name,
@@ -108,7 +109,6 @@ usbDeviceSearch(unsigned int vendor,
                  unsigned int flags)
  {
      DIR *dir = NULL;
-    bool found = false;
      char *ignore = NULL;
      struct dirent *de;
      usbDeviceList *list = NULL, *ret = NULL;
@@ -154,29 +154,13 @@ usbDeviceSearch(unsigned int vendor,
                             10,&found_devno)<  0)
              goto cleanup;

-        /*
-         * Don't set found to true in order to continue the loop
-         * to find multiple USB devices with same idVendor and idProduct
-         */
-        if (flags&&  !(flags&  ~USB_DEVICE_FIND_BY_VENDOR))
-            if (found_prod != product || found_vend != vendor)
-                continue;
-
-        if (flags&&  !(flags&  ~USB_DEVICE_FIND_BY_BUS)) {
-            if (found_bus != bus || found_devno != devno)
-                continue;
-            found = true;
-        }
+        if ((flags&  USB_DEVICE_FIND_BY_VENDOR)&&
+            (found_prod != product || found_vend != vendor))
+            continue;

-        if ((flags&  USB_DEVICE_FIND_BY_BUS)
-&&  (flags&  USB_DEVICE_FIND_BY_VENDOR)) {
-            if (found_prod != product ||
-                found_vend != vendor  ||
-                found_bus != bus ||
-                found_devno != devno)
-                continue;
-            found = true;
-        }
+        if ((flags&  USB_DEVICE_FIND_BY_BUS)&&
+            (found_bus != bus || found_devno != devno))
+            continue;

          usb = usbGetDevice(found_bus, found_devno);
          if (!usb)
@@ -187,7 +171,7 @@ usbDeviceSearch(unsigned int vendor,
              goto cleanup;
          }

-        if (found)
+        if (flags&  USB_DEVICE_FIND_ONE)
              break;
      }
      ret = list;
@@ -195,7 +179,7 @@ usbDeviceSearch(unsigned int vendor,
  cleanup:
      if (dir) {
          int saved_errno = errno;
-        closedir (dir);
+        closedir(dir);
          errno = saved_errno;
      }

@@ -209,7 +193,8 @@ usbFindDeviceByVendor(unsigned int vendor, unsigned
product)
  {

      usbDeviceList *list;
-    if (!(list = usbDeviceSearch(vendor, product, 0 , 0,
USB_DEVICE_FIND_BY_VENDOR)))
+    if (!(list = usbDeviceSearch(vendor, product, 0 , 0,
+                                 USB_DEVICE_FIND_BY_VENDOR)))
          return NULL;

      if (list->count == 0) {
@@ -228,12 +213,14 @@ usbFindDeviceByBus(unsigned int bus, unsigned devno)
      usbDevice *usb;
      usbDeviceList *list;

-    if (!(list = usbDeviceSearch(0, 0, bus, devno,
USB_DEVICE_FIND_BY_BUS)))
+    if (!(list = usbDeviceSearch(0, 0, bus, devno,
+                                 USB_DEVICE_FIND_BY_BUS |
USB_DEVICE_FIND_ONE)))
          return NULL;

      if (list->count == 0) {
          usbReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Did not find USB device bus:%u device:%u"),
bus, devno);
+                       _("Did not find USB device bus:%u device:%u"),
+                       bus, devno);
          usbDeviceListFree(list);
          return NULL;
      }
@@ -254,7 +241,9 @@ usbFindDevice(unsigned int vendor,
      usbDevice *usb;
      usbDeviceList *list;

-    unsigned int flags = USB_DEVICE_FIND_BY_VENDOR|USB_DEVICE_FIND_BY_BUS;
+    unsigned int flags = (USB_DEVICE_FIND_BY_VENDOR |
+                          USB_DEVICE_FIND_BY_BUS |
+                          USB_DEVICE_FIND_ONE);
      if (!(list = usbDeviceSearch(vendor, product, bus, devno, flags)))
          return NULL;





[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]