On Tue, Jan 12, 2010 at 03:26:29PM -0500, Cole Robinson wrote: > 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..e015c06 100644 > --- a/src/security/security_selinux.c > +++ b/src/security/security_selinux.c > @@ -484,7 +484,9 @@ SELinuxSetSecurityHostdevLabel(virConnectPtr conn, > 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); > + dev->source.subsys.u.usb.device, > + dev->source.subsys.u.usb.vendor, > + dev->source.subsys.u.usb.product); > > if (!usb) > goto done;
You need to remove the surrounding if/else clause here > diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c > index 35b29ad..6a52d4c 100644 > --- a/src/security/virt-aa-helper.c > +++ b/src/security/virt-aa-helper.c > @@ -839,8 +839,11 @@ get_files(vahControl * ctl) > 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); > + 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, And likewise here. > diff --git a/src/util/hostusb.c b/src/util/hostusb.c > index 07e10b1..3e9ac83 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,95 @@ 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, > + const char *fmt, unsigned *value) > +{ > + int ret = -1; > + char *buf = NULL; > + char filename[PATH_MAX]; > + > + snprintf(filename, PATH_MAX, USB_SYSFS "/devices/%s/%s", d_name, f_name); Lets us virAsprintf() here. > + > + if (virFileReadAll(filename, 1024, &buf) < 0) > + goto error; > + > + if (sscanf(buf, fmt, value) != 1) { > + virReportSystemError(conn, errno, > + _("Could not parse usb file %s"), filename); > + goto error; > + } The callers all either pass in "%x" or "%d" to this, so how about just using virStrToLong and passing in the 'base' arg instead of format. > +static int usbFindBusByVendor(virConnectPtr conn, > + unsigned vendor, unsigned product, > + unsigned *bus, unsigned *devno) > +{ > + DIR *dir = NULL; > + int ret = -1, found = 0; > + 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, > + "%x", &found_vend) < 0) > + goto error; > + if (usbSysReadFile(conn, "idProduct", de->d_name, > + "%x", &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; > + found_bus = atoi(tmpstr); This should be virStrToLong > + > + if (usbSysReadFile(conn, "devnum", de->d_name, > + "%d", &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 +155,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); > > -- Overall, it is alot simpler than I was expecting it to be which is nice ! 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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list