On Thu, Mar 04, 2010 at 11:52:43AM +0000, Daniel P. Berrange wrote:
> Changeset
>
> commit 5073aa994af460e775cb3e548528e28d7660fcc8
> Author: Cole Robinson <crobinso@xxxxxxxxxx>
> 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
Okay, it's a bit of a lng patch but makes sense with the explanations,
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list