Changeset commit 5073aa994af460e775cb3e548528e28d7660fcc8 Author: Cole Robinson <crobinso(a)redhat.com&gt; 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 | 3 + src/qemu/qemu_driver.c | 102 +++++++++++++++++++++++++++++++++++---- src/qemu/qemu_security_dac.c | 8 +-- src/security/security_selinux.c | 8 +-- src/security/virt-aa-helper.c | 4 +- src/util/hostusb.c | 39 +++++++++++---- src/util/hostusb.h | 13 +++-- 7 files changed, 136 insertions(+), 41 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ce9f013..c5ee23d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -599,7 +599,10 @@ virArgvToString; # usb.h usbGetDevice; +usbFindDevice; usbFreeDevice; +usbDeviceGetBus; +usbDeviceGetDevno; usbDeviceFileIterate; # uuid.h diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ef3cd6c..50bd55b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2283,17 +2283,15 @@ cleanup: return ret; } + static int -qemuPrepareHostDevices(struct qemud_driver *driver, - virDomainDefPtr def) +qemuPrepareHostPCIDevices(struct qemud_driver *driver, + virDomainDefPtr def) { pciDeviceList *pcidevs; int i; int ret = -1; - if (!def->nhostdevs) - return 0; - if (!(pcidevs = qemuGetPciHostDeviceList(def))) return -1; @@ -2344,6 +2342,62 @@ 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); + + fprintf(stderr, "Resolve %u %u -> %u %u\n", + hostdev->source.subsys.u.usb.vendor, + hostdev->source.subsys.u.usb.product, + hostdev->source.subsys.u.usb.bus, + hostdev->source.subsys.u.usb.device ); + + 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) { @@ -6114,6 +6168,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) @@ -6677,11 +6748,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; - - if (dev->data.hostdev->source.subsys.u.usb.bus == bus && - dev->data.hostdev->source.subsys.u.usb.device == device) { - detach = vm->def->hostdevs[i]; - break; + 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 && + dev->data.hostdev->source.subsys.u.usb.device) { + if (dev->data.hostdev->source.subsys.u.usb.bus == bus && + 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; + } } } diff --git a/src/qemu/qemu_security_dac.c b/src/qemu/qemu_security_dac.c index f281b96..6911f48 100644 --- a/src/qemu/qemu_security_dac.c +++ b/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.vendor, - dev->source.subsys.u.usb.product); + dev->source.subsys.u.usb.device); 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.vendor, - dev->source.subsys.u.usb.product); + dev->source.subsys.u.usb.device); if (!usb) goto done; diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 06a2479..b2c8581 100644 --- a/src/security/security_selinux.c +++ b/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.vendor, - dev->source.subsys.u.usb.product); + dev->source.subsys.u.usb.device); 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.vendor, - dev->source.subsys.u.usb.product); + dev->source.subsys.u.usb.device); if (!usb) goto done; diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 066e18b..78bef41 100644 --- a/src/security/virt-aa-helper.c +++ b/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.vendor, - dev->source.subsys.u.usb.product); + dev->source.subsys.u.usb.device); if (usb == NULL) continue; diff --git a/src/util/hostusb.c b/src/util/hostusb.c index bf96539..afba325 100644 --- a/src/util/hostusb.c +++ b/src/util/hostusb.c @@ -159,9 +159,7 @@ cleanup: usbDevice * usbGetDevice(unsigned bus, - unsigned devno, - unsigned vendor, - unsigned product) + unsigned devno) { 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) diff --git a/src/util/hostusb.h b/src/util/hostusb.h index bc22671..9a1b1b7 100644 --- a/src/util/hostusb.h +++ b/src/util/hostusb.h @@ -26,11 +26,14 @@ typedef struct _usbDevice usbDevice; -usbDevice *usbGetDevice (unsigned bus, - unsigned devno, - unsigned vendor, - unsigned product); -void usbFreeDevice (usbDevice *dev); +usbDevice *usbGetDevice(unsigned bus, + unsigned devno); +usbDevice *usbFindDevice(unsigned vendor, + unsigned product); +void usbFreeDevice (usbDevice *dev); + +unsigned usbDeviceGetBus(usbDevice *dev); +unsigned usbDeviceGetDevno(usbDevice *dev); /* * Callback that will be invoked once for each file