[libvirt] [PATCH] Add optional serial id for USB devices in hostdev

From: David Waring <david.waring@rd.bbc.co.uk> This patch adds the ability to include a serial element in a hostdev/source to select a device with a particular serial number. Patch for bugzilla #914883. This allows specific USB devices to attached to guests when there may be more than one of the same USB device attached to a host. The serial number is optional so that without it existing behaviour is maintained. David Waring (1): Add ability to attach USB devices to guests using vendor, product and serial id. docs/formatdomain.html.in | 21 +++++++++++--------- src/conf/domain_conf.c | 18 +++++++++++++++-- src/conf/domain_conf.h | 1 + src/conf/node_device_conf.c | 5 +++++ src/conf/node_device_conf.h | 1 + src/lxc/lxc_hostdev.c | 5 +++-- src/qemu/qemu_hostdev.c | 5 +++-- src/util/virusb.c | 47 +++++++++++++++++++++++++++++++++++++++++---- src/util/virusb.h | 2 ++ 9 files changed, 86 insertions(+), 19 deletions(-) -- 1.8.1.4

From: David Waring <davidjw@rd.bbc.co.uk> --- docs/formatdomain.html.in | 21 +++++++++++--------- src/conf/domain_conf.c | 18 +++++++++++++++-- src/conf/domain_conf.h | 1 + src/conf/node_device_conf.c | 5 +++++ src/conf/node_device_conf.h | 1 + src/lxc/lxc_hostdev.c | 5 +++-- src/qemu/qemu_hostdev.c | 5 +++-- src/util/virusb.c | 47 +++++++++++++++++++++++++++++++++++++++++---- src/util/virusb.h | 2 ++ 9 files changed, 86 insertions(+), 19 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 755d084..87dca10 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2428,10 +2428,11 @@ </dd> <dt><code>source</code></dt> <dd>The source element describes the device as seen from the host. - The USB device can either be addressed by vendor / product id using the - <code>vendor</code> and <code>product</code> elements or by the device's - address on the hosts using the <code>address</code> element. PCI devices - on the other hand can only be described by their <code>address</code>. + The USB device can either be addressed by vendor / product id and optional + serial number using the <code>vendor</code>, <code>product</code> and + <code>serial</code> elements or by the device's address on the hosts using + the <code>address</code> element. PCI devices on the other hand can only + be described by their <code>address</code>. SCSI devices are described by both the <code>adapter</code> and <code>address</code> elements. @@ -2455,11 +2456,13 @@ </tr> </table> </dd> - <dt><code>vendor</code>, <code>product</code></dt> - <dd>The <code>vendor</code> and <code>product</code> elements each have an - <code>id</code> attribute that specifies the USB vendor and product id. - The ids can be given in decimal, hexadecimal (starting with 0x) or - octal (starting with 0) form.</dd> + <dt><code>vendor</code>, <code>product</code>, <code>serial</code></dt> + <dd>The <code>vendor</code>, <code>product</code> and <code>serial</code> + elements each have an <code>id</code> attribute that specifies the USB + vendor, product and device serial id. The ids can be given in decimal, + hexadecimal (starting with 0x) or octal (starting with 0) form for + vendor and product. The serial number is a string that matches the serial + number of the device.</dd> <dt><code>boot</code></dt> <dd>Specifies that the device is bootable. The <code>order</code> attribute determines the order in which devices will be tried during diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2373397..1ef775d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3541,6 +3541,16 @@ virDomainHostdevSubsysUsbDefParseXML(const xmlNodePtr node, "%s", _("usb product needs id")); goto out; } + } else if (xmlStrEqual(cur->name, BAD_CAST "serial")) { + char* serial = virXMLPropString(cur, "id"); + + if (serial) { + def->source.subsys.u.usb.serial = serial; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("usb serial needs id")); + goto out; + } } else if (xmlStrEqual(cur->name, BAD_CAST "address")) { char *bus, *device; @@ -9532,9 +9542,10 @@ virDomainHostdevMatchSubsysUSB(virDomainHostdevDefPtr a, a->source.subsys.u.usb.device == b->source.subsys.u.usb.device) return 1; } else { - /* specified by product & vendor id */ + /* specified by product, vendor id and optionally serial number */ if (a->source.subsys.u.usb.product == b->source.subsys.u.usb.product && - a->source.subsys.u.usb.vendor == b->source.subsys.u.usb.vendor) + a->source.subsys.u.usb.vendor == b->source.subsys.u.usb.vendor && + STREQ_NULLABLE(a->source.subsys.u.usb.serial, b->source.subsys.u.usb.serial)) return 1; } return 0; @@ -14343,6 +14354,9 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, def->source.subsys.u.usb.vendor); virBufferAsprintf(buf, "<product id='0x%.4x'/>\n", def->source.subsys.u.usb.product); + if (def->source.subsys.u.usb.serial != NULL) + virBufferAsprintf(buf, "<serial id='%s' />\n", + def->source.subsys.u.usb.serial); } if (def->source.subsys.u.usb.bus || def->source.subsys.u.usb.device) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 5b159ac..d1086f4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -413,6 +413,7 @@ struct _virDomainHostdevSubsys { unsigned vendor; unsigned product; + char *serial; } usb; struct { virDevicePCIAddress addr; /* host address */ diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 4eeb3b3..7a56cc3 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -347,6 +347,9 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDefPtr def) data->usb_dev.vendor_name); else virBufferAddLit(&buf, " />\n"); + if (data->usb_dev.serial) + virBufferEscapeString(&buf, " <serial id='%s' />\n", + data->usb_dev.serial); break; case VIR_NODE_DEV_CAP_USB_INTERFACE: virBufferAsprintf(&buf, " <number>%d</number>\n", @@ -952,6 +955,7 @@ virNodeDevCapUsbDevParseXML(xmlXPathContextPtr ctxt, data->usb_dev.vendor_name = virXPathString("string(./vendor[1])", ctxt); data->usb_dev.product_name = virXPathString("string(./product[1])", ctxt); + data->usb_dev.serial = virXPathString("string(./serial[1]/@id)", ctxt); ret = 0; out: @@ -1383,6 +1387,7 @@ void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) case VIR_NODE_DEV_CAP_USB_DEV: VIR_FREE(data->usb_dev.product_name); VIR_FREE(data->usb_dev.vendor_name); + VIR_FREE(data->usb_dev.serial); break; case VIR_NODE_DEV_CAP_USB_INTERFACE: VIR_FREE(data->usb_if.description); diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 1c5855c..2c14d63 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -118,6 +118,7 @@ struct _virNodeDevCapsDef { unsigned int vendor; char *product_name; char *vendor_name; + char *serial; } usb_dev; struct { unsigned int number; diff --git a/src/lxc/lxc_hostdev.c b/src/lxc/lxc_hostdev.c index 257e93b..ec931bc 100644 --- a/src/lxc/lxc_hostdev.c +++ b/src/lxc/lxc_hostdev.c @@ -127,6 +127,7 @@ virLXCFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev, { unsigned vendor = hostdev->source.subsys.u.usb.vendor; unsigned product = hostdev->source.subsys.u.usb.product; + const char *serial = hostdev->source.subsys.u.usb.serial; unsigned bus = hostdev->source.subsys.u.usb.bus; unsigned device = hostdev->source.subsys.u.usb.device; bool autoAddress = hostdev->source.subsys.u.usb.autoAddress; @@ -135,7 +136,7 @@ virLXCFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev, *usb = NULL; if (vendor && bus) { - rc = virUSBDeviceFind(vendor, product, bus, device, + rc = virUSBDeviceFind(vendor, product, serial, bus, device, NULL, autoAddress ? false : mandatory, usb); @@ -157,7 +158,7 @@ virLXCFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev, if (vendor) { virUSBDeviceList *devs; - rc = virUSBDeviceFindByVendor(vendor, product, + rc = virUSBDeviceFindByVendor(vendor, product, serial, NULL, mandatory, &devs); if (rc < 0) diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index 9013f60..d8582d7 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -722,6 +722,7 @@ qemuFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev, { unsigned vendor = hostdev->source.subsys.u.usb.vendor; unsigned product = hostdev->source.subsys.u.usb.product; + const char *serial = hostdev->source.subsys.u.usb.serial; unsigned bus = hostdev->source.subsys.u.usb.bus; unsigned device = hostdev->source.subsys.u.usb.device; bool autoAddress = hostdev->source.subsys.u.usb.autoAddress; @@ -730,7 +731,7 @@ qemuFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev, *usb = NULL; if (vendor && bus) { - rc = virUSBDeviceFind(vendor, product, bus, device, + rc = virUSBDeviceFind(vendor, product, serial, bus, device, NULL, autoAddress ? false : mandatory, usb); @@ -752,7 +753,7 @@ qemuFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev, if (vendor) { virUSBDeviceListPtr devs; - rc = virUSBDeviceFindByVendor(vendor, product, NULL, mandatory, &devs); + rc = virUSBDeviceFindByVendor(vendor, product, serial, NULL, mandatory, &devs); if (rc < 0) return -1; diff --git a/src/util/virusb.c b/src/util/virusb.c index d34e44f..5ca396e 100644 --- a/src/util/virusb.c +++ b/src/util/virusb.c @@ -54,6 +54,7 @@ struct _virUSBDevice { char name[USB_ADDR_LEN]; /* domain:bus:slot.function */ char id[USB_ID_LEN]; /* product vendor */ + char *serial; /* serial number */ char *path; const char *used_by; /* name of the domain using this dev */ }; @@ -87,6 +88,30 @@ static int virUSBOnceInit(void) VIR_ONCE_GLOBAL_INIT(virUSB) +static int virUSBSysReadFileString(const char *f_name, const char *d_name, char **buf) +{ + int ret = -1, tmp; + char *filename = NULL; + + tmp = virAsprintf(&filename, USB_SYSFS "/devices/%s/%s", d_name, f_name); + if (tmp < 0) { + virReportOOMError(); + goto cleanup; + } + + tmp = virFileReadAll(filename, 1024, buf); + if (tmp < 0) + goto cleanup; + + if (tmp > 0 && (*buf)[tmp-1] == '\n') + (*buf)[tmp-1]=0; + + ret = 0; +cleanup: + VIR_FREE(filename); + return ret; +} + static int virUSBSysReadFile(const char *f_name, const char *d_name, int base, unsigned int *value) { @@ -120,6 +145,7 @@ cleanup: static virUSBDeviceListPtr virUSBDeviceSearch(unsigned int vendor, unsigned int product, + const char *serial, unsigned int bus, unsigned int devno, const char *vroot, @@ -128,6 +154,7 @@ virUSBDeviceSearch(unsigned int vendor, DIR *dir = NULL; bool found = false; char *ignore = NULL; + char *found_serial = NULL; struct dirent *de; virUSBDeviceListPtr list = NULL, ret = NULL; virUSBDevicePtr usb; @@ -158,6 +185,10 @@ virUSBDeviceSearch(unsigned int vendor, 16, &found_prod) < 0) goto cleanup; + VIR_FREE(found_serial); + found_serial=NULL; + virUSBSysReadFileString("serial", de->d_name, &found_serial); + if (STRPREFIX(de->d_name, "usb")) tmpstr += 3; @@ -173,7 +204,8 @@ virUSBDeviceSearch(unsigned int vendor, goto cleanup; if ((flags & USB_DEVICE_FIND_BY_VENDOR) && - (found_prod != product || found_vend != vendor)) + (found_prod != product || found_vend != vendor || + (serial != NULL && !STREQ_NULLABLE(found_serial, serial)))) continue; if (flags & USB_DEVICE_FIND_BY_BUS) { @@ -197,6 +229,8 @@ virUSBDeviceSearch(unsigned int vendor, ret = list; cleanup: + VIR_FREE(found_serial); + if (dir) { int saved_errno = errno; closedir(dir); @@ -211,6 +245,7 @@ cleanup: int virUSBDeviceFindByVendor(unsigned int vendor, unsigned int product, + const char *serial, const char *vroot, bool mandatory, virUSBDeviceListPtr *devices) @@ -218,7 +253,7 @@ virUSBDeviceFindByVendor(unsigned int vendor, virUSBDeviceListPtr list; int count; - if (!(list = virUSBDeviceSearch(vendor, product, 0 , 0, + if (!(list = virUSBDeviceSearch(vendor, product, serial, 0 , 0, vroot, USB_DEVICE_FIND_BY_VENDOR))) return -1; @@ -256,7 +291,7 @@ virUSBDeviceFindByBus(unsigned int bus, { virUSBDeviceListPtr list; - if (!(list = virUSBDeviceSearch(0, 0, bus, devno, + if (!(list = virUSBDeviceSearch(0, 0, NULL, bus, devno, vroot, USB_DEVICE_FIND_BY_BUS))) return -1; @@ -289,6 +324,7 @@ virUSBDeviceFindByBus(unsigned int bus, int virUSBDeviceFind(unsigned int vendor, unsigned int product, + const char *serial, unsigned int bus, unsigned int devno, const char *vroot, @@ -298,7 +334,7 @@ virUSBDeviceFind(unsigned int vendor, virUSBDeviceListPtr list; unsigned int flags = USB_DEVICE_FIND_BY_VENDOR|USB_DEVICE_FIND_BY_BUS; - if (!(list = virUSBDeviceSearch(vendor, product, bus, devno, + if (!(list = virUSBDeviceSearch(vendor, product, serial, bus, devno, vroot, flags))) return -1; @@ -368,6 +404,8 @@ virUSBDeviceNew(unsigned int bus, return NULL; } + /* XXX fixme. Read serial number if available */ + VIR_DEBUG("%s %s: initialized", dev->id, dev->name); return dev; @@ -379,6 +417,7 @@ virUSBDeviceFree(virUSBDevicePtr dev) if (!dev) return; VIR_DEBUG("%s %s: freeing", dev->id, dev->name); + VIR_FREE(dev->serial); VIR_FREE(dev->path); VIR_FREE(dev); } diff --git a/src/util/virusb.h b/src/util/virusb.h index aa59d12..014ae91 100644 --- a/src/util/virusb.h +++ b/src/util/virusb.h @@ -47,12 +47,14 @@ int virUSBDeviceFindByBus(unsigned int bus, int virUSBDeviceFindByVendor(unsigned int vendor, unsigned int product, + const char *serial, const char *vroot, bool mandatory, virUSBDeviceListPtr *devices); int virUSBDeviceFind(unsigned int vendor, unsigned int product, + const char *serial, unsigned int bus, unsigned int devno, const char *vroot, -- 1.8.1.4

On 06/14/2013 02:43 PM, david.waring@rd.bbc.co.uk wrote:
From: David Waring <davidjw@rd.bbc.co.uk>
The commit message is missing and the title is misleading since we knew how to find the devices by vendor and product before. Please put it here instead of the cover letter, since cover letters don't get into git.
--- docs/formatdomain.html.in | 21 +++++++++++--------- src/conf/domain_conf.c | 18 +++++++++++++++-- src/conf/domain_conf.h | 1 +
RNG schema in docs/schemas will need an update too. It would be nice to test the new element in tests/domainschematest too.
src/conf/node_device_conf.c | 5 +++++ src/conf/node_device_conf.h | 1 + src/lxc/lxc_hostdev.c | 5 +++-- src/qemu/qemu_hostdev.c | 5 +++-- src/util/virusb.c | 47 +++++++++++++++++++++++++++++++++++++++++---- src/util/virusb.h | 2 ++ 9 files changed, 86 insertions(+), 19 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2373397..1ef775d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3541,6 +3541,16 @@ virDomainHostdevSubsysUsbDefParseXML(const xmlNodePtr node, "%s", _("usb product needs id")); goto out; } + } else if (xmlStrEqual(cur->name, BAD_CAST "serial")) { + char* serial = virXMLPropString(cur, "id"); + + if (serial) { + def->source.subsys.u.usb.serial = serial; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("usb serial needs id")); + goto out; + }
The check later in this function should be extended to reject serial id if no vendor or product were specified. For vendor and product, there are numeric ids and string names (which cannot be used here), but for serial there is just the string. How about: <serial>012345678</serial> instead of <serial id='012345678'/>? virXMLPropString allocates memory that needs to be freed in virDomainHostdevDefClear.
} else if (xmlStrEqual(cur->name, BAD_CAST "address")) { char *bus, *device;
@@ -9532,9 +9542,10 @@ virDomainHostdevMatchSubsysUSB(virDomainHostdevDefPtr a, a->source.subsys.u.usb.device == b->source.subsys.u.usb.device) return 1; } else { - /* specified by product & vendor id */ + /* specified by product, vendor id and optionally serial number */ if (a->source.subsys.u.usb.product == b->source.subsys.u.usb.product && - a->source.subsys.u.usb.vendor == b->source.subsys.u.usb.vendor) + a->source.subsys.u.usb.vendor == b->source.subsys.u.usb.vendor && + STREQ_NULLABLE(a->source.subsys.u.usb.serial, b->source.subsys.u.usb.serial)) return 1; } return 0; @@ -14343,6 +14354,9 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, def->source.subsys.u.usb.vendor); virBufferAsprintf(buf, "<product id='0x%.4x'/>\n", def->source.subsys.u.usb.product); + if (def->source.subsys.u.usb.serial != NULL) + virBufferAsprintf(buf, "<serial id='%s' />\n", + def->source.subsys.u.usb.serial);
If you use virBufferEscapeString it will escape any characters that aren't XML-safe and you don't have to check it for NULL (virBufferEscapeString prints nothing if the string is NULL).
} if (def->source.subsys.u.usb.bus || def->source.subsys.u.usb.device) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 5b159ac..d1086f4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -413,6 +413,7 @@ struct _virDomainHostdevSubsys {
unsigned vendor; unsigned product; + char *serial; } usb; struct { virDevicePCIAddress addr; /* host address */ diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 4eeb3b3..7a56cc3 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -347,6 +347,9 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDefPtr def) data->usb_dev.vendor_name); else virBufferAddLit(&buf, " />\n"); + if (data->usb_dev.serial)
This if is not necessary.
+ virBufferEscapeString(&buf, " <serial id='%s' />\n", + data->usb_dev.serial); break; case VIR_NODE_DEV_CAP_USB_INTERFACE: virBufferAsprintf(&buf, " <number>%d</number>\n", @@ -952,6 +955,7 @@ virNodeDevCapUsbDevParseXML(xmlXPathContextPtr ctxt,
data->usb_dev.vendor_name = virXPathString("string(./vendor[1])", ctxt); data->usb_dev.product_name = virXPathString("string(./product[1])", ctxt); + data->usb_dev.serial = virXPathString("string(./serial[1]/@id)", ctxt);
ret = 0; out: @@ -1383,6 +1387,7 @@ void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) case VIR_NODE_DEV_CAP_USB_DEV: VIR_FREE(data->usb_dev.product_name); VIR_FREE(data->usb_dev.vendor_name); + VIR_FREE(data->usb_dev.serial); break; case VIR_NODE_DEV_CAP_USB_INTERFACE: VIR_FREE(data->usb_if.description); diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 1c5855c..2c14d63 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -118,6 +118,7 @@ struct _virNodeDevCapsDef { unsigned int vendor; char *product_name; char *vendor_name; + char *serial; } usb_dev; struct { unsigned int number; diff --git a/src/lxc/lxc_hostdev.c b/src/lxc/lxc_hostdev.c index 257e93b..ec931bc 100644 --- a/src/lxc/lxc_hostdev.c +++ b/src/lxc/lxc_hostdev.c @@ -127,6 +127,7 @@ virLXCFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev, { unsigned vendor = hostdev->source.subsys.u.usb.vendor; unsigned product = hostdev->source.subsys.u.usb.product; + const char *serial = hostdev->source.subsys.u.usb.serial; unsigned bus = hostdev->source.subsys.u.usb.bus; unsigned device = hostdev->source.subsys.u.usb.device; bool autoAddress = hostdev->source.subsys.u.usb.autoAddress; @@ -135,7 +136,7 @@ virLXCFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev, *usb = NULL;
if (vendor && bus) {
Now that we search by serial too, the debug and error messages need to include it as well.
- rc = virUSBDeviceFind(vendor, product, bus, device, + rc = virUSBDeviceFind(vendor, product, serial, bus, device, NULL, autoAddress ? false : mandatory, usb); @@ -157,7 +158,7 @@ virLXCFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev, if (vendor) { virUSBDeviceList *devs;
- rc = virUSBDeviceFindByVendor(vendor, product, + rc = virUSBDeviceFindByVendor(vendor, product, serial,
Trailing space at the end of line (this breaks 'make syntax-check')
NULL, mandatory, &devs); if (rc < 0) diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index 9013f60..d8582d7 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -722,6 +722,7 @@ qemuFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev, { unsigned vendor = hostdev->source.subsys.u.usb.vendor; unsigned product = hostdev->source.subsys.u.usb.product; + const char *serial = hostdev->source.subsys.u.usb.serial; unsigned bus = hostdev->source.subsys.u.usb.bus; unsigned device = hostdev->source.subsys.u.usb.device; bool autoAddress = hostdev->source.subsys.u.usb.autoAddress; @@ -730,7 +731,7 @@ qemuFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev, *usb = NULL;
if (vendor && bus) { - rc = virUSBDeviceFind(vendor, product, bus, device, + rc = virUSBDeviceFind(vendor, product, serial, bus, device, NULL, autoAddress ? false : mandatory, usb);
Same goes for the debug/error messages here.
@@ -752,7 +753,7 @@ qemuFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev, if (vendor) { virUSBDeviceListPtr devs;
- rc = virUSBDeviceFindByVendor(vendor, product, NULL, mandatory, &devs); + rc = virUSBDeviceFindByVendor(vendor, product, serial, NULL, mandatory, &devs); if (rc < 0) return -1;
diff --git a/src/util/virusb.c b/src/util/virusb.c index d34e44f..5ca396e 100644 --- a/src/util/virusb.c +++ b/src/util/virusb.c @@ -54,6 +54,7 @@ struct _virUSBDevice {
char name[USB_ADDR_LEN]; /* domain:bus:slot.function */ char id[USB_ID_LEN]; /* product vendor */ + char *serial; /* serial number */ char *path; const char *used_by; /* name of the domain using this dev */ }; @@ -87,6 +88,30 @@ static int virUSBOnceInit(void)
VIR_ONCE_GLOBAL_INIT(virUSB)
+static int virUSBSysReadFileString(const char *f_name, const char *d_name, char **buf) +{ + int ret = -1, tmp; + char *filename = NULL; + + tmp = virAsprintf(&filename, USB_SYSFS "/devices/%s/%s", d_name, f_name); + if (tmp < 0) {
This might look nicer without the tmp variable.
+ virReportOOMError(); + goto cleanup; + } + + tmp = virFileReadAll(filename, 1024, buf);
This fills my log with errors for all the USB devices without serial numbers. if (!virFileExists(filename)) { *buf = NULL; goto cleanup; }
+ if (tmp < 0) + goto cleanup; +
+ if (tmp > 0 && (*buf)[tmp-1] == '\n') + (*buf)[tmp-1]=0;
Missing spaces around '-' and '='. This might look nicer if we rename the 'buf' parameter to 'value', making buf the local variable for the file contents and then: if (VIR_STRNDUP(*value, buf, strchrnul(buf, '\n')) < 0) goto cleanup;
+ + ret = 0; +cleanup: + VIR_FREE(filename); + return ret; +} + static int virUSBSysReadFile(const char *f_name, const char *d_name, int base, unsigned int *value) { @@ -120,6 +145,7 @@ cleanup: static virUSBDeviceListPtr virUSBDeviceSearch(unsigned int vendor, unsigned int product, + const char *serial, unsigned int bus, unsigned int devno, const char *vroot, @@ -128,6 +154,7 @@ virUSBDeviceSearch(unsigned int vendor, DIR *dir = NULL; bool found = false; char *ignore = NULL; + char *found_serial = NULL; struct dirent *de; virUSBDeviceListPtr list = NULL, ret = NULL; virUSBDevicePtr usb; @@ -158,6 +185,10 @@ virUSBDeviceSearch(unsigned int vendor, 16, &found_prod) < 0) goto cleanup;
+ VIR_FREE(found_serial);
+ found_serial=NULL;
Redundant, VIR_FREE sets it to NULL already.
+ virUSBSysReadFileString("serial", de->d_name, &found_serial); + if (STRPREFIX(de->d_name, "usb")) tmpstr += 3;
@@ -173,7 +204,8 @@ virUSBDeviceSearch(unsigned int vendor, goto cleanup;
if ((flags & USB_DEVICE_FIND_BY_VENDOR) && - (found_prod != product || found_vend != vendor)) + (found_prod != product || found_vend != vendor || + (serial != NULL && !STREQ_NULLABLE(found_serial, serial)))) continue;
if (flags & USB_DEVICE_FIND_BY_BUS) { @@ -197,6 +229,8 @@ virUSBDeviceSearch(unsigned int vendor, ret = list;
cleanup: + VIR_FREE(found_serial); + if (dir) { int saved_errno = errno; closedir(dir); @@ -211,6 +245,7 @@ cleanup: int virUSBDeviceFindByVendor(unsigned int vendor, unsigned int product, + const char *serial,
All these functions should report the serial id in the debug/error messages.
const char *vroot, bool mandatory, virUSBDeviceListPtr *devices)
Jan
participants (2)
-
david.waring@rd.bbc.co.uk
-
Ján Tomko