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

[libvirt] [PATCH v2] Implement path lookup for USB by vendor:product



Based off how QEMU does it, look through /sys/bus/usb/devices/* for
matching vendor:product info, and if found, use info from the surrounding
files to build the device's /dev/bus/usb path.

This fixes USB device assignment by vendor:product when running qemu
as non-root (well, it should, but for some reason I couldn't reproduce
the failure people are seeing in [1], but it appears to work properly)

[1] https://bugzilla.redhat.com/show_bug.cgi?id=542450

v2:
    Drop 'bus.addr only' checks in security drivers
    Use various util helpers

Signed-off-by: Cole Robinson <crobinso redhat com>
---
 po/POTFILES.in                  |    1 +
 src/qemu/qemu_driver.c          |    9 +--
 src/security/security_selinux.c |   25 ++++-----
 src/security/virt-aa-helper.c   |   32 +++++------
 src/util/hostusb.c              |  110 +++++++++++++++++++++++++++++++++++++-
 src/util/hostusb.h              |    4 +-
 6 files changed, 141 insertions(+), 40 deletions(-)

diff --git a/po/POTFILES.in b/po/POTFILES.in
index c9c78b6..3b82a74 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -55,6 +55,7 @@ src/uml/uml_conf.c
 src/uml/uml_driver.c
 src/util/bridge.c
 src/util/conf.c
+src/util/hostusb.c
 src/util/json.c
 src/util/logging.c
 src/util/pci.c
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index deb8adc..f03ce91 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2105,14 +2105,11 @@ static int qemuDomainSetHostdevUSBOwnership(virConnectPtr conn,
     struct qemuFileOwner owner = { uid, gid };
     int ret = -1;
 
-    /* XXX what todo for USB devs assigned based on product/vendor ? Doom :-( */
-    if (!def->source.subsys.u.usb.bus ||
-        !def->source.subsys.u.usb.device)
-        return 0;
-
     usbDevice *dev = usbGetDevice(conn,
                                   def->source.subsys.u.usb.bus,
-                                  def->source.subsys.u.usb.device);
+                                  def->source.subsys.u.usb.device,
+                                  def->source.subsys.u.usb.vendor,
+                                  def->source.subsys.u.usb.product);
 
     if (!dev)
         goto cleanup;
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 000bc8a..cb585ed 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -481,20 +481,17 @@ SELinuxSetSecurityHostdevLabel(virConnectPtr conn,
 
     switch (dev->source.subsys.type) {
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
-        if (dev->source.subsys.u.usb.bus && dev->source.subsys.u.usb.device) {
-            usbDevice *usb = usbGetDevice(conn,
-                                          dev->source.subsys.u.usb.bus,
-                                          dev->source.subsys.u.usb.device);
+        usbDevice *usb = usbGetDevice(conn,
+                                      dev->source.subsys.u.usb.bus,
+                                      dev->source.subsys.u.usb.device,
+                                      dev->source.subsys.u.usb.vendor,
+                                      dev->source.subsys.u.usb.product);
 
-            if (!usb)
-                goto done;
+        if (!usb)
+            goto done;
 
-            ret = usbDeviceFileIterate(conn, usb, SELinuxSetSecurityUSBLabel, vm);
-            usbFreeDevice(conn, usb);
-        } else {
-            /* XXX deal with product/vendor better */
-            ret = 0;
-        }
+        ret = usbDeviceFileIterate(conn, usb, SELinuxSetSecurityUSBLabel, vm);
+        usbFreeDevice(conn, usb);
         break;
     }
 
@@ -556,7 +553,9 @@ SELinuxRestoreSecurityHostdevLabel(virConnectPtr conn,
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
         usbDevice *usb = usbGetDevice(conn,
                                       dev->source.subsys.u.usb.bus,
-                                      dev->source.subsys.u.usb.device);
+                                      dev->source.subsys.u.usb.device,
+                                      dev->source.subsys.u.usb.vendor,
+                                      dev->source.subsys.u.usb.product);
 
         if (!usb)
             goto done;
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 35b29ad..3c8b49a 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -836,24 +836,22 @@ get_files(vahControl * ctl)
             virDomainHostdevDefPtr dev = ctl->def->hostdevs[i];
             switch (dev->source.subsys.type) {
             case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
-                if (dev->source.subsys.u.usb.bus &&
-                    dev->source.subsys.u.usb.device) {
-                    usbDevice *usb = usbGetDevice(NULL,
-                               dev->source.subsys.u.usb.bus,
-                               dev->source.subsys.u.usb.device);
-                    if (usb == NULL)
-                        continue;
-                    rc = usbDeviceFileIterate(NULL, usb,
-                                              file_iterate_cb, &buf);
-                    usbFreeDevice(NULL, usb);
-                    if (rc != 0)
-                        goto clean;
-                    else {
-                        /* TODO: deal with product/vendor better */
-                        rc = 0;
-                    }
-                }
+                usbDevice *usb = usbGetDevice(NULL,
+                                          dev->source.subsys.u.usb.bus,
+                                          dev->source.subsys.u.usb.device,
+                                          dev->source.subsys.u.usb.vendor,
+                                          dev->source.subsys.u.usb.product);
+
+                if (usb == NULL)
+                    continue;
+
+                rc = usbDeviceFileIterate(NULL, usb,
+                                          file_iterate_cb, &buf);
+                usbFreeDevice(NULL, usb);
+                if (rc != 0)
+                    goto clean;
                 break;
+                }
             }
 /* TODO: update so files in /sys are readonly
             case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
diff --git a/src/util/hostusb.c b/src/util/hostusb.c
index 07e10b1..8fbb486 100644
--- a/src/util/hostusb.c
+++ b/src/util/hostusb.c
@@ -37,9 +37,10 @@
 #include "util.h"
 #include "virterror_internal.h"
 
+#define USB_SYSFS "/sys/bus/usb"
 #define USB_DEVFS "/dev/bus/usb/"
-#define USB_ID_LEN 10 /* "XXXX XXXX" */
-#define USB_ADDR_LEN 8 /* "XXX:XXX" */
+#define USB_ID_LEN 10 /* "1234 5678" */
+#define USB_ADDR_LEN 8 /* "123:456" */
 
 struct _usbDevice {
     unsigned      bus;
@@ -57,11 +58,108 @@ struct _usbDevice {
     virReportErrorHelper(conn, VIR_FROM_NONE, code, __FILE__,  \
                          __FUNCTION__, __LINE__, fmt)
 
+static int usbSysReadFile(virConnectPtr conn,
+                          const char *f_name, const char *d_name,
+                          int base, unsigned *value)
+{
+    int ret = -1, tmp;
+    char *buf = NULL;
+    char *filename = NULL;
+    char *ignore = NULL;
+
+    tmp = virAsprintf(&filename, USB_SYSFS "/devices/%s/%s", d_name, f_name);
+    if (tmp < 0) {
+        virReportOOMError(conn);
+        goto error;
+    }
+
+    if (virFileReadAll(filename, 1024, &buf) < 0)
+        goto error;
+
+    if (virStrToLong_ui(buf, &ignore, base, value) < 0) {
+        usbReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                       _("Could not parse usb file %s"), filename);
+        goto error;
+    }
+
+    ret = 0;
+error:
+    VIR_FREE(filename);
+    VIR_FREE(buf);
+    return ret;
+}
+
+static int usbFindBusByVendor(virConnectPtr conn,
+                              unsigned vendor, unsigned product,
+                              unsigned *bus, unsigned *devno)
+{
+    DIR *dir = NULL;
+    int ret = -1, found = 0;
+    char *ignore = NULL;
+    struct dirent *de;
+
+    dir = opendir(USB_SYSFS "/devices");
+    if (!dir) {
+        virReportSystemError(conn, errno,
+                             _("Could not open directory %s"),
+                             USB_SYSFS "/devices");
+        goto error;
+    }
+
+    while ((de = readdir(dir))) {
+        unsigned found_prod, found_vend;
+        if (de->d_name[0] == '.' || strchr(de->d_name, ':'))
+            continue;
+
+        if (usbSysReadFile(conn, "idVendor", de->d_name,
+                           16, &found_vend) < 0)
+            goto error;
+        if (usbSysReadFile(conn, "idProduct", de->d_name,
+                           16, &found_prod) < 0)
+            goto error;
+
+        if (found_prod == product && found_vend == vendor) {
+            /* Lookup bus.addr info */
+            char *tmpstr = de->d_name;
+            unsigned found_bus, found_addr;
+
+            if (STREQ(de->d_name, "usb"))
+                tmpstr += 3;
+
+            if (virStrToLong_ui(tmpstr, &ignore, 10, &found_bus) < 0) {
+                usbReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                               _("Failed to parse dir name '%s'"),
+                               de->d_name);
+                goto error;
+            }
+
+            if (usbSysReadFile(conn, "devnum", de->d_name,
+                               10, &found_addr) < 0)
+                goto error;
+
+            *bus = found_bus;
+            *devno = found_addr;
+            found = 1;
+            break;
+        }
+    }
+
+    if (!found)
+        usbReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                       _("Did not find USB device %x:%x"), vendor, product);
+    else
+        ret = 0;
+
+error:
+    return ret;
+}
 
 usbDevice *
 usbGetDevice(virConnectPtr conn,
              unsigned bus,
-             unsigned devno)
+             unsigned devno,
+             unsigned vendor,
+             unsigned product)
 {
     usbDevice *dev;
 
@@ -70,6 +168,12 @@ usbGetDevice(virConnectPtr conn,
         return NULL;
     }
 
+    if (vendor) {
+        /* Look up bus.dev by vendor:product */
+        if (usbFindBusByVendor(conn, vendor, product, &bus, &devno) < 0)
+            return NULL;
+    }
+
     dev->bus     = bus;
     dev->dev     = devno;
 
diff --git a/src/util/hostusb.h b/src/util/hostusb.h
index 7f75c8b..739a4aa 100644
--- a/src/util/hostusb.h
+++ b/src/util/hostusb.h
@@ -28,7 +28,9 @@ typedef struct _usbDevice usbDevice;
 
 usbDevice *usbGetDevice      (virConnectPtr  conn,
                               unsigned       bus,
-                              unsigned       devno);
+                              unsigned       devno,
+                              unsigned       vendor,
+                              unsigned       product);
 void       usbFreeDevice     (virConnectPtr  conn,
                               usbDevice     *dev);
 
-- 
1.6.5.2


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