Fix USB passthrough based on product/vendor

Changeset

  commit 5073aa994a
  Author: Cole Robinson <crobinso@redhat.com>
  Date:   Mon Jan 11 11:40:46 2010 -0500

Added support for product/vendor based passthrough, but it only
worked at the security driver layer. The main guest XML config
was not updated with the resolved bus/device ID. When the QEMU
argv refactoring removed use of product/vendor, this then broke
launching guests.

THe solution is to move the product/vendor resolution up a layer
into the QEMU driver. So the first thing QEMU does is resolve
the product/vendor to a bus/device and updates the XML config
with this info. The rest of the code, including security drivers
and QEMU argv generated can now rely on bus/device always being
set.

* src/util/hostusb.c, src/util/hostusb.h: Split vendor/product
  resolution code out of usbGetDevice and into usbFindDevice.
  Add accessors for bus/device ID
* src/security/virt-aa-helper.c, src/security/security_selinux.c,
  src/qemu/qemu_security_dac.c: Remove vendor/product from the
  usbGetDevice() calls
* src/qemu/qemu_driver.c: Use usbFindDevice to resolve vendor/product
  into a bus/device ID

src/libvirt_private.syms

@@ -599,7 +599,10 @@ virArgvToString;
 
 # usb.h
 usbGetDevice;
+usbFindDevice;
 usbFreeDevice;
+usbDeviceGetBus;
+usbDeviceGetDevno;
 usbDeviceFileIterate;
 
 # uuid.h

src/qemu/qemu_driver.c

@@ -2290,17 +2290,15 @@ cleanup:
     return ret;
 }
 
+
 static int
-qemuPrepareHostDevices(struct qemud_driver *driver,
+qemuPrepareHostPCIDevices(struct qemud_driver *driver,
-                       virDomainDefPtr def)
+                          virDomainDefPtr def)
 {
     pciDeviceList *pcidevs;
     int i;
     int ret = -1;
 
-    if (!def->nhostdevs)
-        return 0;
-
     if (!(pcidevs = qemuGetPciHostDeviceList(def)))
         return -1;
 
@@ -2351,6 +2349,56 @@ cleanup:
     return ret;
 }
 
+
+static int
+qemuPrepareHostUSBDevices(struct qemud_driver *driver ATTRIBUTE_UNUSED,
+                          virDomainDefPtr def)
+{
+    int i;
+    for (i = 0 ; i < def->nhostdevs ; i++) {
+        virDomainHostdevDefPtr hostdev = def->hostdevs[i];
+
+        if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
+            continue;
+        if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
+            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);
+
+            if (!usb)
+                return -1;
+
+            hostdev->source.subsys.u.usb.bus = usbDeviceGetBus(usb);
+            hostdev->source.subsys.u.usb.device = usbDeviceGetDevno(usb);
+
+            usbFreeDevice(usb);
+        }
+    }
+
+    return 0;
+}
+
+static int
+qemuPrepareHostDevices(struct qemud_driver *driver,
+                       virDomainDefPtr def)
+{
+    if (!def->nhostdevs)
+        return 0;
+
+    if (qemuPrepareHostPCIDevices(driver, def) < 0)
+        return -1;
+
+    if (qemuPrepareHostUSBDevices(driver, def) < 0)
+        return -1;
+
+    return 0;
+}
+
+
 static void
 qemudReattachManagedDevice(pciDevice *dev)
 {
@@ -6478,6 +6526,23 @@ static int qemudDomainAttachHostDevice(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) {
+        usbDevice *usb
+            = usbFindDevice(hostdev->source.subsys.u.usb.vendor,
+                            hostdev->source.subsys.u.usb.product);
+
+        if (!usb)
+            return -1;
+
+        hostdev->source.subsys.u.usb.bus = usbDeviceGetBus(usb);
+        hostdev->source.subsys.u.usb.device = usbDeviceGetDevno(usb);
+
+        usbFreeDevice(usb);
+    }
+
+
     if (driver->securityDriver &&
         driver->securityDriver->domainSetSecurityHostdevLabel &&
         driver->securityDriver->domainSetSecurityHostdevLabel(vm, hostdev) < 0)
@@ -7041,11 +7106,22 @@ static int qemudDomainDetachHostUsbDevice(struct qemud_driver *driver,
 
         unsigned bus = vm->def->hostdevs[i]->source.subsys.u.usb.bus;
         unsigned device = vm->def->hostdevs[i]->source.subsys.u.usb.device;
+        unsigned product = vm->def->hostdevs[i]->source.subsys.u.usb.product;
+        unsigned vendor = vm->def->hostdevs[i]->source.subsys.u.usb.vendor;
 
-        if (dev->data.hostdev->source.subsys.u.usb.bus == bus &&
+        if (dev->data.hostdev->source.subsys.u.usb.bus &&
-            dev->data.hostdev->source.subsys.u.usb.device == device) {
+            dev->data.hostdev->source.subsys.u.usb.device) {
-            detach = vm->def->hostdevs[i];
+            if (dev->data.hostdev->source.subsys.u.usb.bus == bus &&
-            break;
+                dev->data.hostdev->source.subsys.u.usb.device == device) {
+                detach = vm->def->hostdevs[i];
+                break;
+            }
+        } else {
+            if (dev->data.hostdev->source.subsys.u.usb.product == product &&
+                dev->data.hostdev->source.subsys.u.usb.vendor == vendor) {
+                detach = vm->def->hostdevs[i];
+                break;
+            }
         }
     }
 

src/qemu/qemu_security_dac.c

@@ -206,9 +206,7 @@ qemuSecurityDACSetSecurityHostdevLabel(virDomainObjPtr vm,
     switch (dev->source.subsys.type) {
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
         usbDevice *usb = usbGetDevice(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;
@@ -277,9 +275,7 @@ qemuSecurityDACRestoreSecurityHostdevLabel(virDomainObjPtr vm ATTRIBUTE_UNUSED,
     switch (dev->source.subsys.type) {
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
         usbDevice *usb = usbGetDevice(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;

src/security/security_selinux.c

@@ -491,9 +491,7 @@ SELinuxSetSecurityHostdevLabel(virDomainObjPtr vm,
     switch (dev->source.subsys.type) {
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
         usbDevice *usb = usbGetDevice(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;
@@ -561,9 +559,7 @@ SELinuxRestoreSecurityHostdevLabel(virDomainObjPtr vm,
     switch (dev->source.subsys.type) {
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
         usbDevice *usb = usbGetDevice(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;

src/security/virt-aa-helper.c

@@ -836,9 +836,7 @@ get_files(vahControl * ctl)
             switch (dev->source.subsys.type) {
             case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
                 usbDevice *usb = usbGetDevice(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 == NULL)
                     continue;

src/util/hostusb.c

@@ -159,9 +159,7 @@ cleanup:
 
 usbDevice *
 usbGetDevice(unsigned bus,
-             unsigned devno,
+             unsigned devno)
-             unsigned vendor,
-             unsigned product)
 {
     usbDevice *dev;
 
@@ -170,14 +168,6 @@ usbGetDevice(unsigned bus,
         return NULL;
     }
 
-    if (vendor) {
-        /* Look up bus.dev by vendor:product */
-        if (usbFindBusByVendor(vendor, product, &bus, &devno) < 0) {
-            VIR_FREE(dev);
-            return NULL;
-        }
-    }
-
     dev->bus     = bus;
     dev->dev     = devno;
 
@@ -194,6 +184,21 @@ 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)
 {
@@ -202,6 +207,18 @@ usbFreeDevice(usbDevice *dev)
 }
 
 
+unsigned usbDeviceGetBus(usbDevice *dev)
+{
+    return dev->bus;
+}
+
+
+unsigned usbDeviceGetDevno(usbDevice *dev)
+{
+    return dev->dev;
+}
+
+
 int usbDeviceFileIterate(usbDevice *dev,
                          usbDeviceFileActor actor,
                          void *opaque)

src/util/hostusb.h

@@ -26,11 +26,14 @@
 
 typedef struct _usbDevice usbDevice;
 
-usbDevice *usbGetDevice      (unsigned       bus,
+usbDevice *usbGetDevice(unsigned bus,
-                              unsigned       devno,
+                        unsigned devno);
-                              unsigned       vendor,
+usbDevice *usbFindDevice(unsigned vendor,
-                              unsigned       product);
+                         unsigned product);
-void       usbFreeDevice     (usbDevice     *dev);
+void       usbFreeDevice (usbDevice *dev);
+
+unsigned usbDeviceGetBus(usbDevice *dev);
+unsigned usbDeviceGetDevno(usbDevice *dev);
 
 /*
  * Callback that will be invoked once for each file