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@xxxxxxxxxx> > --- > 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list