[libvirt] [PATCH v2] Implement path lookup for USB by vendor:product
Daniel P. Berrange
berrange at redhat.com
Thu Jan 14 12:33:02 UTC 2010
On Wed, Jan 13, 2010 at 03:50:05PM -0500, Cole Robinson wrote:
> 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 at 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);
>
> --
ACK
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
More information about the libvir-list
mailing list