[libvirt] [PATCH 0/3] Attach USB hostdevs using serial id
Originally posted a year ago: https://www.redhat.com/archives/libvir-list/2013-June/msg00624.html Rebased, with tests and nodedev 'implementation' and some nits fixed. David Waring (2): Add serials to USB hostdevs in domain XML Add ability to attach host USB devices using serial id Ján Tomko (1): Add USB serials to node device driver docs/formatdomain.html.in | 22 +++++---- docs/schemas/domaincommon.rng | 8 ++++ docs/schemas/nodedev.rng | 9 ++++ src/conf/domain_conf.c | 31 ++++++++++-- src/conf/domain_conf.h | 1 + src/conf/node_device_conf.c | 4 ++ src/conf/node_device_conf.h | 1 + src/node_device/node_device_hal.c | 1 + src/node_device/node_device_udev.c | 6 +++ src/util/virhostdev.c | 17 +++---- src/util/virusb.c | 56 +++++++++++++++++++--- src/util/virusb.h | 2 + tests/nodedevschemadata/usb_1_1_5_3.xml | 11 +++++ tests/nodedevxml2xmltest.c | 1 + .../qemuxml2argvdata/qemuxml2argv-usb-hostdev.xml | 42 ++++++++++++++++ tests/qemuxml2xmltest.c | 2 + tests/virusbtest.c | 40 +++++++++++----- .../sys_bus_usb/devices/1-1.5.3.1/serial | 1 + .../sys_bus_usb/devices/1-1.5.3.3/serial | 1 + .../sys_bus_usb/devices/1-1.5.5/serial | 1 + .../sys_bus_usb/devices/1-1.5.6/serial | 1 + .../sys_bus_usb/devices/1-1.5/serial | 1 + .../sys_bus_usb/devices/1-1.6/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/1-1/serial | 1 + .../sys_bus_usb/devices/2-1.2/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/2-1/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/usb1/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/usb2/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/usb3/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/usb4/serial | 1 + 30 files changed, 227 insertions(+), 40 deletions(-) create mode 100644 tests/nodedevschemadata/usb_1_1_5_3.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hostdev.xml create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.6/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/2-1.2/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/2-1/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb1/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb2/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb3/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb4/serial -- 1.8.3.2
From: David Waring <davidjw@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. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- docs/formatdomain.html.in | 22 +++++++----- docs/schemas/domaincommon.rng | 8 +++++ src/conf/domain_conf.c | 31 +++++++++++++--- src/conf/domain_conf.h | 1 + .../qemuxml2argvdata/qemuxml2argv-usb-hostdev.xml | 42 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 ++ 6 files changed, 92 insertions(+), 14 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hostdev.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 691a451..5bb2531 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2711,6 +2711,7 @@ <source startupPolicy='optional'> <vendor id='0x1234'/> <product id='0xbeef'/> + <serial>SERIAL123456</serial> </source> <boot order='2'/> </hostdev> @@ -2773,10 +2774,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. @@ -2800,11 +2802,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/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index af67123..8595771 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3546,6 +3546,14 @@ <ref name="usbId"/> </attribute> </element> + <optional> + <element name="serial"> + <choice> + <text/> + <empty/> + </choice> + </element> + </optional> </define> <define name="usbaddress"> <element name="address"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fe06921..4c1d02e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1719,8 +1719,14 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def) } break; case VIR_DOMAIN_HOSTDEV_MODE_SUBSYS: - if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) + switch (def->source.subsys.type) { + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: + VIR_FREE(def->source.subsys.u.usb.serial); + break; + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: VIR_FREE(def->source.subsys.u.scsi.adapter); + break; + } break; } } @@ -3852,6 +3858,18 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, "%s", _("usb product needs id")); goto out; } + } else if (xmlStrEqual(cur->name, BAD_CAST "serial")) { + char *serial; + + if (VIR_STRDUP(serial, (const char *)xmlNodeGetContent(cur)) < 0) + goto out; + + if (!serial) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("usb serial needs id")); + goto out; + } + def->source.subsys.u.usb.serial = serial; } else if (xmlStrEqual(cur->name, BAD_CAST "address")) { char *bus, *device; @@ -3903,12 +3921,12 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, goto out; } - if (!got_vendor && got_product) { + if (!got_vendor && (got_product || def->source.subsys.u.usb.serial)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing vendor")); goto out; } - if (got_vendor && !got_product) { + if ((got_vendor || def->source.subsys.u.usb.serial) && !got_product) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing product")); goto out; @@ -10095,9 +10113,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; @@ -15479,6 +15498,8 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, def->source.subsys.u.usb.vendor); virBufferAsprintf(buf, "<product id='0x%.4x'/>\n", def->source.subsys.u.usb.product); + virBufferEscapeString(buf, "<serial>%s</serial>\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 2de807d..cba3733 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -403,6 +403,7 @@ struct _virDomainHostdevSubsys { unsigned vendor; unsigned product; + char *serial; } usb; struct { virDevicePCIAddress addr; /* host address */ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-hostdev.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-hostdev.xml new file mode 100644 index 0000000..10c4bbf --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-hostdev.xml @@ -0,0 +1,42 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0' model='ich9-ehci1'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x7'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci1'> + <master startport='0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0' multifunction='on'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci2'> + <master startport='2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x1'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci3'> + <master startport='4'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <hostdev mode='subsystem' type='usb' managed='no'> + <source startupPolicy='optional'> + <vendor id='0x1234'/> + <product id='0xbeef'/> + <serial>S123456</serial> + </source> + <boot order='1'/> + </hostdev> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index da528da..a130034 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -364,6 +364,8 @@ mymain(void) DO_TEST("chardev-label"); + DO_TEST("usb-hostdev"); + virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); -- 1.8.3.2
From: David Waring <davidjw@rd.bbc.co.uk> 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. https://bugzilla.redhat.com/show_bug.cgi?id=914883 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/virhostdev.c | 17 +++---- src/util/virusb.c | 56 +++++++++++++++++++--- src/util/virusb.h | 2 + tests/virusbtest.c | 40 +++++++++++----- .../sys_bus_usb/devices/1-1.5.3.1/serial | 1 + .../sys_bus_usb/devices/1-1.5.3.3/serial | 1 + .../sys_bus_usb/devices/1-1.5.5/serial | 1 + .../sys_bus_usb/devices/1-1.5.6/serial | 1 + .../sys_bus_usb/devices/1-1.5/serial | 1 + .../sys_bus_usb/devices/1-1.6/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/1-1/serial | 1 + .../sys_bus_usb/devices/2-1.2/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/2-1/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/usb1/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/usb2/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/usb3/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/usb4/serial | 1 + 17 files changed, 102 insertions(+), 26 deletions(-) create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.6/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/2-1.2/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/2-1/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb1/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb2/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb3/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb4/serial diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 9dd1df2..ad652c7 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -1049,6 +1049,7 @@ virHostdevFindUSBDevice(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; @@ -1057,7 +1058,7 @@ virHostdevFindUSBDevice(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); @@ -1066,9 +1067,9 @@ virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev, } else if (!autoAddress) { goto out; } else { - VIR_INFO("USB device %x:%x could not be found at previous" - " address (bus:%u device:%u)", - vendor, product, bus, device); + VIR_INFO("USB device %x:%x (serial: %s) could not be found" + " at previous address (bus:%u device:%u)", + vendor, product, serial ? serial : _("none"), bus, device); } } @@ -1079,7 +1080,7 @@ virHostdevFindUSBDevice(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; @@ -1100,7 +1101,7 @@ virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev, } else { virReportError(VIR_ERR_OPERATION_FAILED, _("Multiple USB devices for %x:%x, " - "use <address> to specify one"), + "use <address> or <serial> to specify one"), vendor, product); } return -1; @@ -1111,9 +1112,9 @@ virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev, hostdev->source.subsys.u.usb.autoAddress = true; if (autoAddress) { - VIR_INFO("USB device %x:%x found at bus:%u device:%u (moved" + VIR_INFO("USB device %x:%x (serial: %s) found at bus:%u device:%u (moved" " from bus:%u device:%u)", - vendor, product, + vendor, product, serial ? serial : _("none"), hostdev->source.subsys.u.usb.bus, hostdev->source.subsys.u.usb.device, bus, device); diff --git a/src/util/virusb.c b/src/util/virusb.c index 8244771..5dc24b5 100644 --- a/src/util/virusb.c +++ b/src/util/virusb.c @@ -56,6 +56,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; /* driver:domain using this dev */ @@ -92,6 +93,34 @@ 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, len; + char *filename = NULL; + + *buf = NULL; + + if (virAsprintf(&filename, USB_SYSFS "/devices/%s/%s", d_name, f_name) < 0) + return -1; + + if (!virFileExists(filename)) { + ret = 0; + goto cleanup; + } + + if ((len = virFileReadAll(filename, 1024, buf)) < 0) + goto cleanup; + + if (len > 0 && (*buf)[len - 1] == '\n') + (*buf)[len - 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) { @@ -123,6 +152,7 @@ static int virUSBSysReadFile(const char *f_name, const char *d_name, static virUSBDeviceListPtr virUSBDeviceSearch(unsigned int vendor, unsigned int product, + const char *serial, unsigned int bus, unsigned int devno, const char *vroot, @@ -131,6 +161,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; @@ -162,6 +193,10 @@ virUSBDeviceSearch(unsigned int vendor, 16, &found_prod) < 0) goto cleanup; + VIR_FREE(found_serial); + if (virUSBSysReadFileString("serial", de->d_name, &found_serial) < 0) + goto cleanup; + if (STRPREFIX(de->d_name, "usb")) tmpstr += 3; @@ -177,7 +212,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) { @@ -203,6 +239,8 @@ virUSBDeviceSearch(unsigned int vendor, ret = list; cleanup: + VIR_FREE(found_serial); + if (dir) { int saved_errno = errno; closedir(dir); @@ -217,6 +255,7 @@ virUSBDeviceSearch(unsigned int vendor, int virUSBDeviceFindByVendor(unsigned int vendor, unsigned int product, + const char *serial, const char *vroot, bool mandatory, virUSBDeviceListPtr *devices) @@ -224,7 +263,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; @@ -232,15 +271,16 @@ virUSBDeviceFindByVendor(unsigned int vendor, if (list->count == 0) { virObjectUnref(list); if (!mandatory) { - VIR_DEBUG("Did not find USB device %x:%x", - vendor, product); + VIR_DEBUG("Did not find USB device %x:%x (serial: %s)", + vendor, product, NULLSTR(serial)); if (devices) *devices = NULL; return 0; } virReportError(VIR_ERR_INTERNAL_ERROR, - _("Did not find USB device %x:%x"), vendor, product); + _("Did not find USB device %x:%x (serial: %s)"), + vendor, product, serial ? serial : _("none")); return -1; } @@ -262,7 +302,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; @@ -295,6 +335,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, @@ -304,7 +345,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; @@ -382,6 +423,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->used_by_drvname); VIR_FREE(dev->used_by_domname); diff --git a/src/util/virusb.h b/src/util/virusb.h index f98ea21..bb7644f 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, diff --git a/tests/virusbtest.c b/tests/virusbtest.c index d08e03b..c1a07a1 100644 --- a/tests/virusbtest.c +++ b/tests/virusbtest.c @@ -40,6 +40,7 @@ struct findTestInfo { const char *name; unsigned int vendor; unsigned int product; + char *serial; unsigned int bus; unsigned int devno; const char *vroot; @@ -82,11 +83,13 @@ static int testDeviceFind(const void *opaque) switch (info->how) { case FIND_BY_ALL: rv = virUSBDeviceFind(info->vendor, info->product, + info->serial, info->bus, info->devno, info->vroot, info->mandatory, &dev); break; case FIND_BY_VENDOR: rv = virUSBDeviceFindByVendor(info->vendor, info->product, + info->serial, info->vroot, info->mandatory, &devs); break; case FIND_BY_BUS: @@ -162,7 +165,7 @@ testUSBList(const void *opaque ATTRIBUTE_UNUSED) goto cleanup; #define EXPECTED_NDEVS_ONE 3 - if (virUSBDeviceFindByVendor(0x1d6b, 0x0002, NULL, true, &devlist) < 0) + if (virUSBDeviceFindByVendor(0x1d6b, 0x0002, NULL, NULL, true, &devlist) < 0) goto cleanup; ndevs = virUSBDeviceListCount(devlist); @@ -186,7 +189,7 @@ testUSBList(const void *opaque ATTRIBUTE_UNUSED) goto cleanup; #define EXPECTED_NDEVS_TWO 3 - if (virUSBDeviceFindByVendor(0x18d1, 0x4e22, NULL, true, &devlist) < 0) + if (virUSBDeviceFindByVendor(0x18d1, 0x4e22, NULL, NULL, true, &devlist) < 0) goto cleanup; ndevs = virUSBDeviceListCount(devlist); @@ -206,7 +209,7 @@ testUSBList(const void *opaque ATTRIBUTE_UNUSED) EXPECTED_NDEVS_ONE + EXPECTED_NDEVS_TWO) < 0) goto cleanup; - if (virUSBDeviceFind(0x18d1, 0x4e22, 1, 20, NULL, true, &dev) < 0) + if (virUSBDeviceFind(0x18d1, 0x4e22, NULL, 1, 20, NULL, true, &dev) < 0) goto cleanup; if (!virUSBDeviceListFind(list, dev)) { @@ -239,9 +242,10 @@ mymain(void) { int rv = 0; -#define DO_TEST_FIND_FULL(name, vend, prod, bus, devno, vroot, mand, how, fail) \ +#define DO_TEST_FIND_FULL(name, vend, prod, serial, bus, devno, vroot, \ + mand, how, fail) \ do { \ - struct findTestInfo data = { name, vend, prod, bus, \ + struct findTestInfo data = { name, vend, prod, serial, bus, \ devno, vroot, mand, how, fail \ }; \ if (virtTestRun("USBDeviceFind " name, testDeviceFind, &data) < 0) \ @@ -249,24 +253,31 @@ mymain(void) } while (0) #define DO_TEST_FIND(name, vend, prod, bus, devno) \ - DO_TEST_FIND_FULL(name, vend, prod, bus, devno, NULL, true, \ + DO_TEST_FIND_FULL(name, vend, prod, NULL, bus, devno, NULL, true, \ FIND_BY_ALL, false) #define DO_TEST_FIND_FAIL(name, vend, prod, bus, devno) \ - DO_TEST_FIND_FULL(name, vend, prod, bus, devno, NULL, true, \ + DO_TEST_FIND_FULL(name, vend, prod, NULL, bus, devno, NULL, true, \ FIND_BY_ALL, true) #define DO_TEST_FIND_BY_BUS(name, bus, devno) \ - DO_TEST_FIND_FULL(name, 101, 202, bus, devno, NULL, true, \ + DO_TEST_FIND_FULL(name, 101, 202, NULL, bus, devno, NULL, true, \ FIND_BY_BUS, false) #define DO_TEST_FIND_BY_BUS_FAIL(name, bus, devno) \ - DO_TEST_FIND_FULL(name, 101, 202, bus, devno, NULL, true, \ + DO_TEST_FIND_FULL(name, 101, 202, NULL, bus, devno, NULL, true, \ FIND_BY_BUS, true) #define DO_TEST_FIND_BY_VENDOR(name, vend, prod) \ - DO_TEST_FIND_FULL(name, vend, prod, 123, 456, NULL, true, \ + DO_TEST_FIND_FULL(name, vend, prod, NULL, 123, 456, NULL, true, \ FIND_BY_VENDOR, false) #define DO_TEST_FIND_BY_VENDOR_FAIL(name, vend, prod) \ - DO_TEST_FIND_FULL(name, vend, prod, 123, 456, NULL, true, \ + DO_TEST_FIND_FULL(name, vend, prod, NULL, 123, 456, NULL, true, \ + FIND_BY_VENDOR, true) + +#define DO_TEST_FIND_BY_SERIAL(name, vend, prod, serial) \ + DO_TEST_FIND_FULL(name, vend, prod, NULL, 123, 456, NULL, true, \ + FIND_BY_VENDOR, false) +#define DO_TEST_FIND_BY_SERIAL_FAIL(name, vend, prod, serial) \ + DO_TEST_FIND_FULL(name, vend, prod, NULL, 123, 456, NULL, true, \ FIND_BY_VENDOR, true) DO_TEST_FIND("Nexus", 0x18d1, 0x4e22, 1, 20); @@ -282,6 +293,13 @@ mymain(void) DO_TEST_FIND_BY_VENDOR_FAIL("Bogus vendor and product", 0xf00d, 0xbeef); DO_TEST_FIND_BY_VENDOR_FAIL("Valid vendor", 0x1d6b, 0xbeef); + DO_TEST_FIND_BY_SERIAL("Nexus (serial string)", 0x18d1, 0x4e22, + "something something"); + DO_TEST_FIND_BY_SERIAL("Nexus (serial number)", 0x18d1, 0x4e22, + "0118999881999119725 3"); + DO_TEST_FIND_BY_SERIAL_FAIL("Nexus (wrong serial)", 0x18d1, 0x4e22, + "0118999881999119725 3"); + if (virtTestRun("USB List test", testUSBList, NULL) < 0) rv = -1; diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/serial b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/serial new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/serial @@ -0,0 +1 @@ + diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/serial b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/serial new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/serial @@ -0,0 +1 @@ + diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/serial b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/serial new file mode 100644 index 0000000..a8aae02 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/serial @@ -0,0 +1 @@ +something something diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/serial b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/serial new file mode 100644 index 0000000..5b4cae7 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/serial @@ -0,0 +1 @@ +0118999881999119725 3 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5/serial b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5/serial new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5/serial @@ -0,0 +1 @@ + diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.6/serial b/tests/virusbtestdata/sys_bus_usb/devices/1-1.6/serial new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.6/serial @@ -0,0 +1 @@ + diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1/serial b/tests/virusbtestdata/sys_bus_usb/devices/1-1/serial new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1/serial @@ -0,0 +1 @@ + diff --git a/tests/virusbtestdata/sys_bus_usb/devices/2-1.2/serial b/tests/virusbtestdata/sys_bus_usb/devices/2-1.2/serial new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/2-1.2/serial @@ -0,0 +1 @@ + diff --git a/tests/virusbtestdata/sys_bus_usb/devices/2-1/serial b/tests/virusbtestdata/sys_bus_usb/devices/2-1/serial new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/2-1/serial @@ -0,0 +1 @@ + diff --git a/tests/virusbtestdata/sys_bus_usb/devices/usb1/serial b/tests/virusbtestdata/sys_bus_usb/devices/usb1/serial new file mode 100644 index 0000000..7f434da --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/usb1/serial @@ -0,0 +1 @@ +0000:00:1a.0 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/usb2/serial b/tests/virusbtestdata/sys_bus_usb/devices/usb2/serial new file mode 100644 index 0000000..3fed695 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/usb2/serial @@ -0,0 +1 @@ +0000:00:1d.0 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/usb3/serial b/tests/virusbtestdata/sys_bus_usb/devices/usb3/serial new file mode 100644 index 0000000..0d45813 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/usb3/serial @@ -0,0 +1 @@ +0000:0d:00.0 diff --git a/tests/virusbtestdata/sys_bus_usb/devices/usb4/serial b/tests/virusbtestdata/sys_bus_usb/devices/usb4/serial new file mode 100644 index 0000000..0d45813 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/usb4/serial @@ -0,0 +1 @@ +0000:0d:00.0 -- 1.8.3.2
Also extend the XML schema and add a test for XML parsing. --- docs/schemas/nodedev.rng | 9 +++++++++ src/conf/node_device_conf.c | 4 ++++ src/conf/node_device_conf.h | 1 + src/node_device/node_device_hal.c | 1 + src/node_device/node_device_udev.c | 6 ++++++ tests/nodedevschemadata/usb_1_1_5_3.xml | 11 +++++++++++ tests/nodedevxml2xmltest.c | 1 + 7 files changed, 33 insertions(+) create mode 100644 tests/nodedevschemadata/usb_1_1_5_3.xml diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 81ab4d4..6e3da6c 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -193,6 +193,15 @@ <empty/> </choice> </element> + + <optional> + <element name='serial'> + <choice> + <text/> + <empty/> + </choice> + </element> + </optional> </define> <define name='capusbinterface'> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index e65b5e4..7926e60 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -365,6 +365,8 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDef *def) data->usb_dev.vendor_name); else virBufferAddLit(&buf, " />\n"); + virBufferEscapeString(&buf, "<serial>%s</serial>\n", + data->usb_dev.serial); break; case VIR_NODE_DEV_CAP_USB_INTERFACE: virBufferAsprintf(&buf, "<number>%d</number>\n", @@ -980,6 +982,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])", ctxt); ret = 0; out: @@ -1476,6 +1479,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 50e6805..51467c1 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -123,6 +123,7 @@ struct _virNodeDevCapsDef { unsigned int vendor; char *product_name; char *vendor_name; + char *serial; } usb_dev; struct { unsigned int number; diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 8656b5d..2f85d45 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -221,6 +221,7 @@ gather_usb_device_cap(LibHalContext *ctx, const char *udi, if (get_str_prop(ctx, udi, "usb_device.product", &d->usb_dev.product_name) != 0) (void)get_str_prop(ctx, udi, "info.product", &d->usb_dev.product_name); + (void)get_str_prop(ctx, udi, "usb_device.serial", &d->usb_dev.serial); return 0; } diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 9a951d9..b3af192 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -575,6 +575,12 @@ static int udevProcessUSBDevice(struct udev_device *device, goto out; } + if (udevGetStringSysfsAttr(device, + "serial", + &data->usb_dev.serial) == PROPERTY_ERROR) { + goto out; + } + if (udevGenerateDeviceName(device, def, NULL) != 0) { goto out; } diff --git a/tests/nodedevschemadata/usb_1_1_5_3.xml b/tests/nodedevschemadata/usb_1_1_5_3.xml new file mode 100644 index 0000000..9a8ee33 --- /dev/null +++ b/tests/nodedevschemadata/usb_1_1_5_3.xml @@ -0,0 +1,11 @@ +<device> + <name>usb_1_1_5_3</name> + <parent>usb_1_1_5</parent> + <capability type='usb_device'> + <bus>1</bus> + <device>36</device> + <product id='0xc069'>USB Laser Mouse</product> + <vendor id='0x046d'>Logitech</vendor> + <serial>ABCDEFGH123456789</serial> + </capability> +</device> diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c index 9390bf5..770af72 100644 --- a/tests/nodedevxml2xmltest.c +++ b/tests/nodedevxml2xmltest.c @@ -88,6 +88,7 @@ mymain(void) DO_TEST("storage_serial_SATA_HTS721010G9SA00_MPCZ12Y0GNGWSE"); DO_TEST("usb_device_1d6b_1_0000_00_1d_0_if0"); DO_TEST("usb_device_1d6b_1_0000_00_1d_0"); + DO_TEST("usb_1_1_5_3"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.8.3.2
On Thu, Jun 05, 2014 at 08:59:41PM +0200, Ján Tomko wrote:
Originally posted a year ago: https://www.redhat.com/archives/libvir-list/2013-June/msg00624.html
Rebased, with tests and nodedev 'implementation' and some nits fixed.
I'm in two minds about this idea really. I can understand the motivation behind it, but this is a bit of a divergance from how we identify & address devices generally. For USB I'd be more interested in seeing us have the ability to pick devices based on the USB port they're plugged into. I think when you start identifying devices based on arbitrary metadata attributes of the device like its serial number, then this is getting into something that the external mgmt app should do. IMHO libvirt should focus on identifying devices based on the device addressing schemes, and providing the metadata via the node device APIs for apps to consume. The patch adding serial number/string to the node device driver XML does make sense though. We should also add the USB port number to that, since that's a stable attribute which doesn't change across plug/unplug, unlike the device number we currently use. And then allow the <address> element to specify the port number too. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange -
Ján Tomko