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