[PATCH] feat(usb-hostdev): add ability to filter by USB serial strings too

This patch adds the ability to select a specific USB device when multiple are available with the same vendor and product IDs. As an example, I'm using this to pass through a specific USB mouse to my guest VM. I happen to have two of the exact same model (Logitech MX518), differing only in their serial number, and only wish to pass through one of them to my VM. An example of the config snippet I'm using with this: <hostdev mode='subsystem' type='usb' managed='yes'> <source> <vendor id='0x046d'/> <product id='0xc08e'/> <serial>0D3087538362</serial> </source> </hostdev> Without this patch I would either get an error message on attempted startup, complaining about having found multiple devices with the same product or vendor id. Or alternatively I'd have to set the bus number and device number. Unfortunately the latter tends to change with every reboot, and in my case whenever my monitor comes out of standby (I'm using its builtin USB hub). --- docs/formatdomain.rst | 6 ++ docs/schemas/domaincommon.rng | 10 +++ src/conf/domain_conf.c | 15 ++++ src/conf/domain_conf.h | 3 +- src/hypervisor/virhostdev.c | 14 ++-- src/util/virusb.c | 66 ++++++++++++----- src/util/virusb.h | 2 + tests/virusbtest.c | 71 +++++++++++-------- .../sys_bus_usb/devices/1-1.5.4/serial | 1 + .../sys_bus_usb/devices/1-1.5.5/serial | 1 + .../sys_bus_usb/devices/1-1.5.6/serial | 1 + 11 files changed, 137 insertions(+), 53 deletions(-) create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/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 diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 2587106191..6d011cc789 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3864,6 +3864,7 @@ for PCI (KVM only) and 1.0.6 for SCSI (KVM only)` : <source startupPolicy='optional'> <vendor id='0x1234'/> <product id='0xbeef'/> + <serial>TANT14-B4732</serial> </source> <boot order='2'/> </hostdev> @@ -4041,6 +4042,11 @@ or: optional drop if missing at any start attempt ========= ===================================================================== + :since:`Since 7.1.0`, the ``source`` element of USB devices may contain + the ``serial`` element which can be used in addition to ``vendor`` and + ``product`` to select a specific USB device if multiple devices with the + same vendor and product id are present. + ``pci`` PCI devices can only be described by their ``address``. :since:`Since 6.8.0 (Xen only)` , the ``source`` element of a PCI device diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index e6de934456..d6eb29a9a8 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -5355,6 +5355,11 @@ <ref name="usbId"/> </attribute> </element> + <optional> + <element name="serial"> + <ref name="usbSerial"/> + </element> + </optional> </define> <define name="usbaddress"> <element name="address"> @@ -7053,6 +7058,11 @@ <param name="pattern">(0x)?[0-9a-fA-F]{1,4}</param> </data> </define> + <define name="usbSerial"> + <data type="string"> + <param name="pattern">[A-Za-z0-9_\.\+\- ]+</param> + </data> + </define> <define name="usbVersion"> <data type="string"> <param name="pattern">[0-9]{1,2}.[0-9]{1,2}</param> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b731744f04..4967d62989 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3078,6 +3078,8 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def) VIR_FREE(def->source.subsys.u.scsi_host.wwpn); break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: + VIR_FREE(def->source.subsys.u.usb.serial); + break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST: @@ -6703,6 +6705,16 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, "%s", _("usb product needs id")); return -1; } + } else if (!usbsrc->serial && + virXMLNodeNameEqual(cur, "serial")) { + g_autofree char *serial = virXMLNodeContentString(cur); + if (!serial) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("usb serial needs content")); + return -1; + } + + usbsrc->serial = g_steal_pointer(&serial); } else if (virXMLNodeNameEqual(cur, "address")) { g_autofree char *bus = NULL; g_autofree char *device = NULL; @@ -25089,6 +25101,9 @@ virDomainHostdevDefFormatSubsysUSB(virBufferPtr buf, if (usbsrc->vendor) { virBufferAsprintf(&sourceChildBuf, "<vendor id='0x%.4x'/>\n", usbsrc->vendor); virBufferAsprintf(&sourceChildBuf, "<product id='0x%.4x'/>\n", usbsrc->product); + if (usbsrc->serial) { + virBufferEscapeString(&sourceChildBuf, "<serial>%s</serial>\n", usbsrc->serial); + } } if (usbsrc->bus || usbsrc->device) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 930eed60de..a66b815901 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -227,12 +227,13 @@ VIR_ENUM_DECL(virDomainHostdevSubsysSCSIProtocol); struct _virDomainHostdevSubsysUSB { bool autoAddress; /* bus/device were filled automatically based - on vendor/product */ + on vendor/product (optionally serial) */ unsigned bus; unsigned device; unsigned vendor; unsigned product; + char *serial; }; struct _virDomainHostdevSubsysPCI { diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c index 743aaa84d6..f9fd42b24d 100644 --- a/src/hypervisor/virhostdev.c +++ b/src/hypervisor/virhostdev.c @@ -1324,6 +1324,7 @@ virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev, virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; unsigned vendor = usbsrc->vendor; unsigned product = usbsrc->product; + const char *serial = usbsrc->serial; unsigned bus = usbsrc->bus; unsigned device = usbsrc->device; bool autoAddress = usbsrc->autoAddress; @@ -1332,7 +1333,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); @@ -1354,7 +1355,8 @@ virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev, if (vendor) { g_autoptr(virUSBDeviceList) devs = NULL; - rc = virUSBDeviceFindByVendor(vendor, product, NULL, mandatory, &devs); + rc = virUSBDeviceFindByVendor(vendor, product, serial, NULL, mandatory, + &devs); if (rc < 0) { return -1; } else if (rc == 0) { @@ -1362,9 +1364,11 @@ virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev, } else if (rc > 1) { if (autoAddress) { virReportError(VIR_ERR_OPERATION_FAILED, - _("Multiple USB devices for %x:%x were found," - " but none of them is at bus:%u device:%u"), - vendor, product, bus, device); + _("Multiple USB devices for %x:%x (serial: %s)" + " were found, but none of them is at bus:%u" + " device:%u"), + vendor, product, serial ? serial : "<none>", bus, + device); } else { virReportError(VIR_ERR_OPERATION_FAILED, _("Multiple USB devices for %x:%x, " diff --git a/src/util/virusb.c b/src/util/virusb.c index 6d897600e5..d945d71b98 100644 --- a/src/util/virusb.c +++ b/src/util/virusb.c @@ -81,21 +81,31 @@ static int virUSBOnceInit(void) VIR_ONCE_GLOBAL_INIT(virUSB); -static int virUSBSysReadFile(const char *f_name, const char *d_name, - int base, unsigned int *value) +static char* virUSBSysReadFile(const char *f_name, const char *d_name) { - g_autofree char *buf = NULL; g_autofree char *filename = NULL; - char *ignore = NULL; + char* buf; filename = g_strdup_printf(USB_SYSFS "/devices/%s/%s", d_name, f_name); if (virFileReadAll(filename, 1024, &buf) < 0) + return NULL; + + return buf; +} + +static int virUSBSysReadIntFile(const char *f_name, const char *d_name, + int base, unsigned int *value) +{ + g_autofree char *buf = NULL; + char *ignore = NULL; + + if (!(buf = virUSBSysReadFile(f_name, d_name))) return -1; if (virStrToLong_ui(buf, &ignore, base, value) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Could not parse usb file %s"), filename); + _("Could not parse usb file %s/%s"), d_name, f_name); return -1; } @@ -105,6 +115,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, @@ -132,11 +143,11 @@ virUSBDeviceSearch(unsigned int vendor, if (strchr(de->d_name, ':')) continue; - if (virUSBSysReadFile("idVendor", de->d_name, + if (virUSBSysReadIntFile("idVendor", de->d_name, 16, &found_vend) < 0) goto cleanup; - if (virUSBSysReadFile("idProduct", de->d_name, + if (virUSBSysReadIntFile("idProduct", de->d_name, 16, &found_prod) < 0) goto cleanup; @@ -150,13 +161,31 @@ virUSBDeviceSearch(unsigned int vendor, goto cleanup; } - if (virUSBSysReadFile("devnum", de->d_name, + if (virUSBSysReadIntFile("devnum", de->d_name, 10, &found_devno) < 0) goto cleanup; - if ((flags & USB_DEVICE_FIND_BY_VENDOR) && - (found_prod != product || found_vend != vendor)) - continue; + if (flags & USB_DEVICE_FIND_BY_VENDOR) { + if (found_prod != product || found_vend != vendor) + continue; + + if (serial) { + g_autofree char *found_serial = virUSBSysReadFile("serial", de->d_name); + size_t len; + + if (!found_serial) + continue; + len = strlen(found_serial); + if (!len) + continue; + + if (found_serial[len - 1] == '\n') + found_serial[len - 1] = '\0'; + + if (strcmp(found_serial, serial)) + continue; + } + } if (flags & USB_DEVICE_FIND_BY_BUS) { if (found_bus != bus || found_devno != devno) @@ -188,6 +217,7 @@ virUSBDeviceSearch(unsigned int vendor, int virUSBDeviceFindByVendor(unsigned int vendor, unsigned int product, + const char *serial, const char *vroot, bool mandatory, virUSBDeviceListPtr *devices) @@ -195,7 +225,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; @@ -203,15 +233,16 @@ virUSBDeviceFindByVendor(unsigned int vendor, if (list->count == 0) { virObjectUnref(list); if (!mandatory) { - VIR_DEBUG("Did not find USB device %04x:%04x", - vendor, product); + VIR_DEBUG("Did not find USB device %04x:%04x (serial %s)", + vendor, product, serial ? serial : "<none>"); if (devices) *devices = NULL; return 0; } virReportError(VIR_ERR_INTERNAL_ERROR, - _("Did not find USB device %04x:%04x"), vendor, product); + _("Did not find USB device %04x:%04x (serial %s)"), + vendor, product, serial ? serial : "<none>"); return -1; } @@ -233,7 +264,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; @@ -266,6 +297,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, @@ -275,7 +307,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; diff --git a/src/util/virusb.h b/src/util/virusb.h index 42a3303952..c9d5330fac 100644 --- a/src/util/virusb.h +++ b/src/util/virusb.h @@ -45,12 +45,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 7df4e3aec3..cba4d57d9a 100644 --- a/tests/virusbtest.c +++ b/tests/virusbtest.c @@ -37,12 +37,14 @@ struct findTestInfo { const char *name; unsigned int vendor; unsigned int product; + const char *serial; unsigned int bus; unsigned int devno; const char *vroot; bool mandatory; int how; bool expectFailure; + unsigned int expectResults; }; static int testDeviceFileActor(virUSBDevicePtr dev, @@ -76,12 +78,12 @@ static int testDeviceFind(const void *opaque) switch (info->how) { case FIND_BY_ALL: - rv = virUSBDeviceFind(info->vendor, info->product, + 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, + rv = virUSBDeviceFindByVendor(info->vendor, info->product, info->serial, info->vroot, info->mandatory, &devs); break; case FIND_BY_BUS: @@ -117,6 +119,13 @@ static int testDeviceFind(const void *opaque) if (virUSBDeviceFileIterate(device, testDeviceFileActor, NULL) < 0) goto cleanup; } + + if (ndevs != info->expectResults) { + virReportError(VIR_ERR_INTERNAL_ERROR, "different amount of results" + " than expected (%zu != %u)", ndevs, + info->expectResults); + goto cleanup; + } break; } @@ -157,7 +166,7 @@ testUSBList(const void *opaque G_GNUC_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); @@ -181,7 +190,7 @@ testUSBList(const void *opaque G_GNUC_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); @@ -201,7 +210,7 @@ testUSBList(const void *opaque G_GNUC_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)) { @@ -235,48 +244,50 @@ 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, expected) \ do { \ - struct findTestInfo data = { name, vend, prod, bus, \ - devno, vroot, mand, how, fail \ + struct findTestInfo data = { name, vend, prod, serial, bus, \ + devno, vroot, mand, how, fail, expected \ }; \ if (virTestRun("USBDeviceFind " name, testDeviceFind, &data) < 0) \ rv = -1; \ } while (0) -#define DO_TEST_FIND(name, vend, prod, bus, devno) \ - DO_TEST_FIND_FULL(name, vend, prod, 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, \ - FIND_BY_ALL, true) +#define DO_TEST_FIND(name, vend, prod, serial, bus, devno, expected) \ + DO_TEST_FIND_FULL(name, vend, prod, serial, bus, devno, NULL, true, \ + FIND_BY_ALL, false, expected) +#define DO_TEST_FIND_FAIL(name, vend, prod, serial, bus, devno) \ + DO_TEST_FIND_FULL(name, vend, prod, serial, bus, devno, NULL, true, \ + FIND_BY_ALL, true, 0) #define DO_TEST_FIND_BY_BUS(name, bus, devno) \ - DO_TEST_FIND_FULL(name, 101, 202, bus, devno, NULL, true, \ - FIND_BY_BUS, false) + DO_TEST_FIND_FULL(name, 101, 202, NULL, bus, devno, NULL, true, \ + FIND_BY_BUS, false, 1) #define DO_TEST_FIND_BY_BUS_FAIL(name, bus, devno) \ - DO_TEST_FIND_FULL(name, 101, 202, bus, devno, NULL, true, \ - FIND_BY_BUS, true) + DO_TEST_FIND_FULL(name, 101, 202, NULL, bus, devno, NULL, true, \ + FIND_BY_BUS, true, 0) -#define DO_TEST_FIND_BY_VENDOR(name, vend, prod) \ - DO_TEST_FIND_FULL(name, vend, prod, 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, \ - FIND_BY_VENDOR, true) +#define DO_TEST_FIND_BY_VENDOR(name, vend, prod, serial, expected) \ + DO_TEST_FIND_FULL(name, vend, prod, serial, 123, 456, NULL, true, \ + FIND_BY_VENDOR, false, expected) +#define DO_TEST_FIND_BY_VENDOR_FAIL(name, vend, prod, serial) \ + DO_TEST_FIND_FULL(name, vend, prod, serial, 123, 456, NULL, true, \ + FIND_BY_VENDOR, true, 0) - DO_TEST_FIND("Nexus", 0x18d1, 0x4e22, 1, 20); - DO_TEST_FIND_FAIL("Nexus wrong devnum", 0x18d1, 0x4e22, 1, 25); - DO_TEST_FIND_FAIL("Bogus", 0xf00d, 0xbeef, 1024, 768); + DO_TEST_FIND("Nexus", 0x18d1, 0x4e22, NULL, 1, 20, 1); + DO_TEST_FIND_FAIL("Nexus wrong devnum", 0x18d1, 0x4e22, NULL, 1, 25); + DO_TEST_FIND_FAIL("Bogus", 0xf00d, 0xbeef, NULL, 1024, 768); DO_TEST_FIND_BY_BUS("integrated camera", 1, 5); DO_TEST_FIND_BY_BUS_FAIL("wrong bus/devno combination", 2, 20); DO_TEST_FIND_BY_BUS_FAIL("missing bus", 5, 20); DO_TEST_FIND_BY_BUS_FAIL("missing devnum", 1, 158); - DO_TEST_FIND_BY_VENDOR("Nexus (multiple results)", 0x18d1, 0x4e22); - 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_VENDOR("Nexus (multiple results)", 0x18d1, 0x4e22, NULL, 3); + DO_TEST_FIND_BY_VENDOR("Nexus (with serial)", 0x18d1, 0x4e22, "TANT14-B4732", 1); + DO_TEST_FIND_BY_VENDOR_FAIL("Bogus vendor and product", 0xf00d, 0xbeef, NULL); + DO_TEST_FIND_BY_VENDOR_FAIL("Valid vendor", 0x1d6b, 0xbeef, NULL); + DO_TEST_FIND_BY_VENDOR_FAIL("Bogus serial", 0x18d1, 0x4e22, "BOGUS"); if (virTestRun("USB List test", testUSBList, NULL) < 0) rv = -1; diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/serial b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/serial new file mode 100644 index 0000000000..6bee213c08 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/serial @@ -0,0 +1 @@ +20200222113321 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 0000000000..14764cdc37 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/serial @@ -0,0 +1 @@ +0123:4b:01.7 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 0000000000..b20f2081b7 --- /dev/null +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/serial @@ -0,0 +1 @@ +TANT14-B4732 -- 2.27.0