From: Maximilian Martin <maximilian_martin@gmx.de>
This patch implements USB bus/port matching. In addition
to vendor/product and bus/device matching, bus/port
matching is implemented. This involves additions and
refactorings in the domain configuration, host device
handling, and USB helpers. Also, test methods are
refactored and extended.
Signed-off-by: Maximilian Martin <maximilian_martin@gmx.de>
---
src/conf/domain_conf.c | 58 +++++++++++--
src/conf/domain_conf.h | 1 +
src/hypervisor/virhostdev.c | 131 +++++++++++++++++------------
src/libvirt_private.syms | 2 -
src/util/virusb.c | 160 ++++++++++++------------------------
src/util/virusb.h | 22 ++---
tests/virusbtest.c | 149 +++++++++++++++++++++++----------
7 files changed, 298 insertions(+), 225 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7766e302ec..63f3032892 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2680,6 +2680,15 @@ virDomainHostdevSubsysSCSIClear(virDomainHostdevSubsysSCSI *scsisrc)
}
}
+static void
+virDomainHostdevSubsysUSBClear(virDomainHostdevSubsysUSB *usbsrc)
+{
+ if (!usbsrc)
+ return;
+
+ VIR_FREE(usbsrc->port);
+}
+
static void
virDomainHostdevDefClear(virDomainHostdevDef *def)
@@ -2723,6 +2732,8 @@ virDomainHostdevDefClear(virDomainHostdevDef *def)
g_clear_pointer(&def->source.subsys.u.pci.origstates, virBitmapFree);
break;
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
+ virDomainHostdevSubsysUSBClear(&def->source.subsys.u.usb);
+ break;
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
break;
@@ -5961,13 +5972,38 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
}
if ((addressNode = virXPathNode("./address", ctxt))) {
+ bool foundDevice = false;
+ bool foundPort = false;
+ g_autofree char *port = NULL;
+ int rc = -1;
+
if (virXMLPropUInt(addressNode, "bus", 0,
- VIR_XML_PROP_REQUIRED, &usbsrc->bus) < 0)
+ VIR_XML_PROP_REQUIRED, &usbsrc->bus) < 0) {
return -1;
+ }
- if (virXMLPropUInt(addressNode, "device", 0,
- VIR_XML_PROP_REQUIRED, &usbsrc->device) < 0)
+ rc = virXMLPropUInt(addressNode, "device", 0,
+ VIR_XML_PROP_NONE, &usbsrc->device);
+ if (rc < 0)
return -1;
+ else if (rc > 0)
+ foundDevice = true;
+
+ port = virXMLPropString(addressNode, "port");
+ if (port && *port) {
+ usbsrc->port = g_steal_pointer(&port);
+ foundPort = true;
+ }
+
+ if (!foundDevice && !foundPort) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("usb address needs either device id or port"));
+ return -1;
+ } else if (foundDevice && foundPort) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("found both device id and port in usb address (ambiguous setting)"));
+ return -1;
+ }
}
return 0;
@@ -14851,8 +14887,13 @@ virDomainHostdevMatchSubsysUSB(virDomainHostdevDef *first,
virDomainHostdevSubsysUSB *first_usbsrc = &first->source.subsys.u.usb;
virDomainHostdevSubsysUSB *second_usbsrc = &second->source.subsys.u.usb;
- if (first_usbsrc->bus && first_usbsrc->device) {
- /* specified by bus location on host */
+ if (first_usbsrc->bus && first_usbsrc->port) {
+ /* specified by bus and port on host */
+ if (first_usbsrc->bus == second_usbsrc->bus &&
+ STREQ_NULLABLE(first_usbsrc->port, second_usbsrc->port))
+ return 1;
+ } else if (first_usbsrc->bus && first_usbsrc->device) {
+ /* specified by bus and device id on host */
if (first_usbsrc->bus == second_usbsrc->bus &&
first_usbsrc->device == second_usbsrc->device)
return 1;
@@ -24511,10 +24552,15 @@ virDomainHostdevDefFormatSubsysUSB(virBuffer *buf,
virBufferAsprintf(&sourceChildBuf, "<product id='0x%.4x'/>\n", usbsrc->product);
}
- if (usbsrc->bus || usbsrc->device)
+ if (usbsrc->bus && usbsrc->port) {
+ virBufferAsprintf(&sourceChildBuf, "<address %sbus='%d' port='%s'/>\n",
+ includeTypeInAddr ? "type='usb' " : "",
+ usbsrc->bus, usbsrc->port);
+ } else if (usbsrc->bus || usbsrc->device) {
virBufferAsprintf(&sourceChildBuf, "<address %sbus='%d' device='%d'/>\n",
includeTypeInAddr ? "type='usb' " : "",
usbsrc->bus, usbsrc->device);
+ }
virXMLFormatElement(buf, "source", &sourceAttrBuf, &sourceChildBuf);
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index eca820892e..a655dc57a8 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -228,6 +228,7 @@ struct _virDomainHostdevSubsysUSB {
on vendor/product */
unsigned bus;
unsigned device;
+ char *port;
unsigned vendor;
unsigned product;
diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c
index 7d7df4418d..19907c76ba 100644
--- a/src/hypervisor/virhostdev.c
+++ b/src/hypervisor/virhostdev.c
@@ -1359,6 +1359,41 @@ virHostdevMarkUSBDevices(virHostdevManager *mgr,
return -1;
}
+static int
+virHostdevFindUSBDeviceWithFlags(virDomainHostdevDef *hostdev,
+ bool mandatory,
+ unsigned int flags,
+ virUSBDevice **usb)
+{
+ virDomainHostdevSubsysUSB *usbsrc = &hostdev->source.subsys.u.usb;
+ unsigned vendor = usbsrc->vendor;
+ unsigned product = usbsrc->product;
+ unsigned bus = usbsrc->bus;
+ const char *port = usbsrc->port;
+ unsigned device = usbsrc->device;
+ g_autoptr(virUSBDeviceList) devs = NULL;
+ int rc;
+
+ rc = virUSBDeviceFind(vendor, product, bus, device, port, NULL,
+ mandatory, flags, &devs);
+ if (rc < 0)
+ return -1;
+
+ if (rc == 1) {
+ *usb = virUSBDeviceListGet(devs, 0);
+ virUSBDeviceListSteal(devs, *usb);
+ }
+
+ if (rc > 1) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Multiple USB devices for %1$x:%2$x, use <address> to specify one"),
+ vendor, product);
+ return -1;
+ }
+
+ return rc;
+}
+
int
virHostdevFindUSBDevice(virDomainHostdevDef *hostdev,
@@ -1367,77 +1402,65 @@ virHostdevFindUSBDevice(virDomainHostdevDef *hostdev,
{
virDomainHostdevSubsysUSB *usbsrc = &hostdev->source.subsys.u.usb;
unsigned vendor = usbsrc->vendor;
- unsigned product = usbsrc->product;
unsigned bus = usbsrc->bus;
unsigned device = usbsrc->device;
+ const char *port = usbsrc->port;
bool autoAddress = usbsrc->autoAddress;
+ unsigned int flags = 0;
int rc;
*usb = NULL;
- if (vendor && bus) {
- rc = virUSBDeviceFind(vendor, product, bus, device,
- NULL,
- autoAddress ? false : mandatory,
- usb);
- if (rc < 0) {
- return -1;
- } 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);
- }
+ if (vendor)
+ flags |= USB_DEVICE_FIND_BY_VENDOR;
+ if (device)
+ flags |= USB_DEVICE_FIND_BY_DEVICE;
+ if (port)
+ flags |= USB_DEVICE_FIND_BY_PORT;
+
+ /* Rule out invalid cases. */
+ if (vendor && device && port) {
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("Cannot match USB device on vendor/product, bus/device, and bus/port at once."));
+ } else if (device && port) {
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("Cannot match USB device on bus/device and bus/port at once."));
+ } else if (!vendor && !device && !port) {
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("No matching fields for USB device found. Vendor/product, bus/device, or bus/port required."));
+ return -1;
}
- /* When vendor is specified, its USB address is either unspecified or the
- * device could not be found at the USB device where it had been
- * automatically found before.
- */
- if (vendor) {
- g_autoptr(virUSBDeviceList) devs = NULL;
+ /* First attempt, matching on all valid fields. */
+ rc = virHostdevFindUSBDeviceWithFlags(hostdev,
+ autoAddress ? false : mandatory,
+ flags, usb);
+ if (rc < 0)
+ return -1;
- rc = virUSBDeviceFindByVendor(vendor, product, NULL, mandatory, &devs);
- if (rc < 0) {
- return -1;
- } else if (rc == 0) {
- goto out;
- } else if (rc > 1) {
- if (autoAddress) {
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("Multiple USB devices for %1$x:%2$x were found, but none of them is at bus:%3$u device:%4$u"),
- vendor, product, bus, device);
- } else {
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("Multiple USB devices for %1$x:%2$x, use <address> to specify one"),
- vendor, product);
- }
+ if (rc != 1 && autoAddress && device) {
+ VIR_INFO("USB device could not be found at previous address "
+ "(bus:%u device:%u)", bus, device);
+
+ /* Second attempt, for when the device number has changed. */
+ flags &= ~USB_DEVICE_FIND_BY_DEVICE;
+ usbsrc->device = 0;
+
+ rc = virHostdevFindUSBDeviceWithFlags(hostdev, mandatory,
+ flags, usb);
+ if (rc < 0)
return -1;
- }
- *usb = virUSBDeviceListGet(devs, 0);
- virUSBDeviceListSteal(devs, *usb);
+ usbsrc->autoAddress = true;
+ }
+ if (!*usb) {
+ hostdev->missing = true;
+ } else if (!usbsrc->bus || !usbsrc->device) {
usbsrc->bus = virUSBDeviceGetBus(*usb);
usbsrc->device = virUSBDeviceGetDevno(*usb);
- usbsrc->autoAddress = true;
-
- if (autoAddress) {
- VIR_INFO("USB device %x:%x found at bus:%u device:%u (moved"
- " from bus:%u device:%u)",
- vendor, product,
- usbsrc->bus, usbsrc->device,
- bus, device);
- }
- } else if (bus) {
- if (virUSBDeviceFindByBus(bus, device, NULL, mandatory, usb) < 0)
- return -1;
}
- out:
- if (!*usb)
- hostdev->missing = true;
return 0;
}
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b846011f0f..bc01f25d4c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3680,8 +3680,6 @@ virURIResolveAlias;
# util/virusb.h
virUSBDeviceFileIterate;
virUSBDeviceFind;
-virUSBDeviceFindByBus;
-virUSBDeviceFindByVendor;
virUSBDeviceFree;
virUSBDeviceGetBus;
virUSBDeviceGetDevno;
diff --git a/src/util/virusb.c b/src/util/virusb.c
index cf3461f80a..85ad3d58ce 100644
--- a/src/util/virusb.c
+++ b/src/util/virusb.c
@@ -61,12 +61,6 @@ struct _virUSBDeviceList {
virUSBDevice **devs;
};
-typedef enum {
- USB_DEVICE_ALL = 0,
- USB_DEVICE_FIND_BY_VENDOR = 1 << 0,
- USB_DEVICE_FIND_BY_BUS = 1 << 1,
-} virUSBDeviceFindFlags;
-
static virClass *virUSBDeviceListClass;
static void virUSBDeviceListDispose(void *obj);
@@ -102,11 +96,29 @@ static int virUSBSysReadFile(const char *f_name, const char *d_name,
return 0;
}
+static int
+virUSBSysReadFileStr(const char *f_name,
+ const char *d_name,
+ char **value)
+{
+ char *buf = NULL;
+ g_autofree char *filename = NULL;
+
+ filename = g_strdup_printf(USB_SYSFS "/devices/%s/%s", d_name, f_name);
+
+ if (virFileReadAll(filename, 1024, &buf) < 0)
+ return -1;
+
+ *value = buf;
+ return 0;
+}
+
static virUSBDeviceList *
virUSBDeviceSearch(unsigned int vendor,
unsigned int product,
unsigned int bus,
unsigned int devno,
+ const char *port,
const char *vroot,
unsigned int flags)
{
@@ -127,6 +139,8 @@ virUSBDeviceSearch(unsigned int vendor,
while ((direrr = virDirRead(dir, &de, USB_SYSFS "/devices")) > 0) {
unsigned int found_prod, found_vend, found_bus, found_devno;
+ g_autofree char *found_port = NULL;
+ bool port_matches;
char *tmpstr = de->d_name;
if (strchr(de->d_name, ':'))
@@ -154,16 +168,31 @@ virUSBDeviceSearch(unsigned int vendor,
10, &found_devno) < 0)
goto cleanup;
- if ((flags & USB_DEVICE_FIND_BY_VENDOR) &&
- (found_prod != product || found_vend != vendor))
- continue;
+ if (virUSBSysReadFileStr("devpath", de->d_name,
+ &found_port) < 0) {
+ goto cleanup;
+ } else {
+ virStringTrimOptionalNewline(found_port);
+ port_matches = STREQ_NULLABLE(found_port, port);
+ }
- if (flags & USB_DEVICE_FIND_BY_BUS) {
+ if (flags & USB_DEVICE_FIND_BY_VENDOR) {
+ if (found_prod != product || found_vend != vendor)
+ continue;
+ }
+
+ if (flags & USB_DEVICE_FIND_BY_DEVICE) {
if (found_bus != bus || found_devno != devno)
continue;
found = true;
}
+ if (flags & USB_DEVICE_FIND_BY_PORT) {
+ if (found_bus != bus || !port_matches)
+ continue;
+ found = true;
+ }
+
usb = virUSBDeviceNew(found_bus, found_devno, vroot);
if (!usb)
@@ -185,123 +214,42 @@ virUSBDeviceSearch(unsigned int vendor,
return ret;
}
-int
-virUSBDeviceFindByVendor(unsigned int vendor,
- unsigned int product,
- const char *vroot,
- bool mandatory,
- virUSBDeviceList **devices)
-{
- virUSBDeviceList *list;
- int count;
-
- if (!(list = virUSBDeviceSearch(vendor, product, 0, 0,
- vroot,
- USB_DEVICE_FIND_BY_VENDOR)))
- return -1;
-
- if (list->count == 0) {
- virObjectUnref(list);
- if (!mandatory) {
- VIR_DEBUG("Did not find USB device %04x:%04x",
- vendor, product);
- if (devices)
- *devices = NULL;
- return 0;
- }
-
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Did not find USB device %1$04x:%2$04x"), vendor, product);
- return -1;
- }
-
- count = list->count;
- if (devices)
- *devices = list;
- else
- virObjectUnref(list);
-
- return count;
-}
-
-int
-virUSBDeviceFindByBus(unsigned int bus,
- unsigned int devno,
- const char *vroot,
- bool mandatory,
- virUSBDevice **usb)
-{
- virUSBDeviceList *list;
-
- if (!(list = virUSBDeviceSearch(0, 0, bus, devno,
- vroot,
- USB_DEVICE_FIND_BY_BUS)))
- return -1;
-
- if (list->count == 0) {
- virObjectUnref(list);
- if (!mandatory) {
- VIR_DEBUG("Did not find USB device bus:%u device:%u",
- bus, devno);
- if (usb)
- *usb = NULL;
- return 0;
- }
-
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Did not find USB device bus:%1$u device:%2$u"),
- bus, devno);
- return -1;
- }
-
- if (usb) {
- *usb = virUSBDeviceListGet(list, 0);
- virUSBDeviceListSteal(list, *usb);
- }
- virObjectUnref(list);
-
- return 0;
-}
-
int
virUSBDeviceFind(unsigned int vendor,
unsigned int product,
unsigned int bus,
unsigned int devno,
+ const char *port,
const char *vroot,
bool mandatory,
- virUSBDevice **usb)
+ unsigned int flags,
+ virUSBDeviceList **devices)
{
- virUSBDeviceList *list;
+ g_autoptr(virUSBDeviceList) list = NULL;
+ int count;
- 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, bus, devno, port,
vroot, flags)))
return -1;
- if (list->count == 0) {
- virObjectUnref(list);
+ count = list->count;
+ if (count == 0) {
if (!mandatory) {
- VIR_DEBUG("Did not find USB device %04x:%04x bus:%u device:%u",
- vendor, product, bus, devno);
- if (usb)
- *usb = NULL;
+ if (devices)
+ *devices = NULL;
return 0;
}
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Did not find USB device %1$04x:%2$04x bus:%3$u device:%4$u"),
- vendor, product, bus, devno);
+ _("Did not find matching USB device: vid:%1$04x, pid:%2$04x, bus:%3$u, device:%4$u, port:%5$s"),
+ vendor, product, bus, devno, port ? port : "");
return -1;
}
- if (usb) {
- *usb = virUSBDeviceListGet(list, 0);
- virUSBDeviceListSteal(list, *usb);
- }
- virObjectUnref(list);
+ if (devices)
+ *devices = g_steal_pointer(&list);
- return 0;
+ return count;
}
virUSBDevice *
diff --git a/src/util/virusb.h b/src/util/virusb.h
index d2b3f69942..86cc0a9d3d 100644
--- a/src/util/virusb.h
+++ b/src/util/virusb.h
@@ -30,30 +30,26 @@ typedef struct _virUSBDeviceList virUSBDeviceList;
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virUSBDeviceList, virObjectUnref);
+typedef enum {
+ USB_DEVICE_ALL = 0,
+ USB_DEVICE_FIND_BY_VENDOR = 1 << 0,
+ USB_DEVICE_FIND_BY_DEVICE = 1 << 1,
+ USB_DEVICE_FIND_BY_PORT = 1 << 2,
+} virUSBDeviceFindFlags;
virUSBDevice *virUSBDeviceNew(unsigned int bus,
unsigned int devno,
const char *vroot);
-int virUSBDeviceFindByBus(unsigned int bus,
- unsigned int devno,
- const char *vroot,
- bool mandatory,
- virUSBDevice **usb);
-
-int virUSBDeviceFindByVendor(unsigned int vendor,
- unsigned int product,
- const char *vroot,
- bool mandatory,
- virUSBDeviceList **devices);
-
int virUSBDeviceFind(unsigned int vendor,
unsigned int product,
unsigned int bus,
unsigned int devno,
+ const char *port,
const char *vroot,
bool mandatory,
- virUSBDevice **usb);
+ unsigned int flags,
+ virUSBDeviceList **devices);
void virUSBDeviceFree(virUSBDevice *dev);
int virUSBDeviceSetUsedBy(virUSBDevice *dev,
diff --git a/tests/virusbtest.c b/tests/virusbtest.c
index 870e136321..12ac338df9 100644
--- a/tests/virusbtest.c
+++ b/tests/virusbtest.c
@@ -26,9 +26,11 @@
#define VIR_FROM_THIS VIR_FROM_NONE
typedef enum {
- FIND_BY_ALL,
FIND_BY_VENDOR,
- FIND_BY_BUS
+ FIND_BY_DEVICE,
+ FIND_BY_PORT,
+ FIND_BY_VENDOR_AND_DEVICE,
+ FIND_BY_VENDOR_AND_PORT
} testUSBFindFlags;
struct findTestInfo {
@@ -37,6 +39,7 @@ struct findTestInfo {
unsigned int product;
unsigned int bus;
unsigned int devno;
+ const char *port;
const char *vroot;
bool mandatory;
int how;
@@ -70,25 +73,34 @@ static int testDeviceFind(const void *opaque)
g_autoptr(virUSBDeviceList) devs = NULL;
int rv = 0;
size_t i, ndevs = 0;
+ unsigned int flags = 0;
switch (info->how) {
- case FIND_BY_ALL:
- rv = virUSBDeviceFind(info->vendor, info->product,
- info->bus, info->devno,
- info->vroot, info->mandatory, &dev);
- break;
case FIND_BY_VENDOR:
- rv = virUSBDeviceFindByVendor(info->vendor, info->product,
- info->vroot, info->mandatory, &devs);
+ flags = USB_DEVICE_FIND_BY_VENDOR;
+ break;
+ case FIND_BY_DEVICE:
+ flags = USB_DEVICE_FIND_BY_DEVICE;
+ break;
+ case FIND_BY_PORT:
+ flags = USB_DEVICE_FIND_BY_PORT;
break;
- case FIND_BY_BUS:
- rv = virUSBDeviceFindByBus(info->bus, info->devno,
- info->vroot, info->mandatory, &dev);
+ case FIND_BY_VENDOR_AND_DEVICE:
+ flags = USB_DEVICE_FIND_BY_VENDOR |
+ USB_DEVICE_FIND_BY_DEVICE;
+ break;
+ case FIND_BY_VENDOR_AND_PORT:
+ flags = USB_DEVICE_FIND_BY_VENDOR |
+ USB_DEVICE_FIND_BY_PORT;
break;
}
+ rv = virUSBDeviceFind(info->vendor, info->product,
+ info->bus, info->devno, info->port,
+ info->vroot, info->mandatory, flags, &devs);
+
if (info->expectFailure) {
- if (rv == 0) {
+ if (rv >= 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
"unexpected success");
} else {
@@ -99,9 +111,20 @@ static int testDeviceFind(const void *opaque)
goto cleanup;
}
+ if (info->how != FIND_BY_VENDOR) {
+ if (rv == 1) {
+ dev = virUSBDeviceListGet(devs, 0);
+ virUSBDeviceListSteal(devs, dev);
+ } else {
+ goto cleanup;
+ }
+ }
+
switch (info->how) {
- case FIND_BY_ALL:
- case FIND_BY_BUS:
+ case FIND_BY_DEVICE:
+ case FIND_BY_PORT:
+ case FIND_BY_VENDOR_AND_DEVICE:
+ case FIND_BY_VENDOR_AND_PORT:
if (virUSBDeviceFileIterate(dev, testDeviceFileActor, NULL) < 0)
goto cleanup;
break;
@@ -146,14 +169,17 @@ testUSBList(const void *opaque G_GNUC_UNUSED)
virUSBDeviceList *list = NULL;
virUSBDeviceList *devlist = NULL;
virUSBDevice *dev = NULL;
+ virUSBDeviceList *devs = NULL;
int ret = -1;
+ int rv;
size_t i, ndevs;
if (!(list = virUSBDeviceListNew()))
goto cleanup;
#define EXPECTED_NDEVS_ONE 3
- if (virUSBDeviceFindByVendor(0x1d6b, 0x0002, NULL, true, &devlist) < 0)
+ if (virUSBDeviceFind(0x1d6b, 0x0002, 0, 0, NULL, NULL, true,
+ USB_DEVICE_FIND_BY_VENDOR, &devlist) < 0)
goto cleanup;
ndevs = virUSBDeviceListCount(devlist);
@@ -176,7 +202,8 @@ testUSBList(const void *opaque G_GNUC_UNUSED)
goto cleanup;
#define EXPECTED_NDEVS_TWO 3
- if (virUSBDeviceFindByVendor(0x18d1, 0x4e22, NULL, true, &devlist) < 0)
+ if (virUSBDeviceFind(0x18d1, 0x4e22, 0, 0, NULL, NULL, true,
+ USB_DEVICE_FIND_BY_VENDOR, &devlist) < 0)
goto cleanup;
ndevs = virUSBDeviceListCount(devlist);
@@ -196,8 +223,16 @@ 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)
+ rv = virUSBDeviceFind(0x18d1, 0x4e22, 1, 20, NULL, NULL, true,
+ USB_DEVICE_FIND_BY_VENDOR |
+ USB_DEVICE_FIND_BY_DEVICE, &devs);
+ if (rv != 1) {
goto cleanup;
+ } else {
+ dev = virUSBDeviceListGet(devs, 0);
+ virUSBDeviceListSteal(devs, dev);
+ }
+ virObjectUnref(devs);
if (!virUSBDeviceListFind(list, dev)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -229,49 +264,75 @@ 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, bus, devno, \
+ port, vroot, mand, how, fail) \
do { \
struct findTestInfo data = { name, vend, prod, bus, \
- devno, vroot, mand, how, fail \
+ devno, port, vroot, mand, how, fail \
}; \
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_BY_BUS(name, bus, devno) \
- DO_TEST_FIND_FULL(name, 101, 202, 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, \
- 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, 123, 456, NULL, 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, 123, 456, NULL, NULL, true, \
FIND_BY_VENDOR, true)
- 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_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);
+#define DO_TEST_FIND_BY_DEVICE(name, bus, devno) \
+ DO_TEST_FIND_FULL(name, 0x1010, 0x2020, bus, devno, NULL, NULL, true, \
+ FIND_BY_DEVICE, false)
+#define DO_TEST_FIND_BY_DEVICE_FAIL(name, bus, devno) \
+ DO_TEST_FIND_FULL(name, 0x1010, 0x2020, bus, devno, NULL, NULL, true, \
+ FIND_BY_DEVICE, true)
+
+#define DO_TEST_FIND_BY_PORT(name, bus, port) \
+ DO_TEST_FIND_FULL(name, 0x1010, 0x2020, bus, 456, port, NULL, true, \
+ FIND_BY_PORT, false)
+#define DO_TEST_FIND_BY_PORT_FAIL(name, bus, port) \
+ DO_TEST_FIND_FULL(name, 0x1010, 0x2020, bus, 456, port, NULL, true, \
+ FIND_BY_PORT, true)
+
+#define DO_TEST_FIND_BY_VENDOR_AND_DEVICE(name, vend, prod, bus, devno) \
+ DO_TEST_FIND_FULL(name, vend, prod, bus, devno, NULL, NULL, true, \
+ FIND_BY_VENDOR_AND_DEVICE, false)
+#define DO_TEST_FIND_BY_VENDOR_AND_DEVICE_FAIL(name, vend, prod, bus, devno) \
+ DO_TEST_FIND_FULL(name, vend, prod, bus, devno, NULL, NULL, true, \
+ FIND_BY_VENDOR_AND_DEVICE, true)
+
+#define DO_TEST_FIND_BY_VENDOR_AND_PORT(name, vend, prod, bus, port) \
+ DO_TEST_FIND_FULL(name, vend, prod, bus, 456, port, NULL, true, \
+ FIND_BY_VENDOR_AND_PORT, false)
+#define DO_TEST_FIND_BY_VENDOR_AND_PORT_FAIL(name, vend, prod, bus, port) \
+ DO_TEST_FIND_FULL(name, vend, prod, bus, 456, port, NULL, true, \
+ FIND_BY_VENDOR_AND_PORT, true)
+
+ DO_TEST_FIND_BY_DEVICE("integrated camera", 1, 5);
+ DO_TEST_FIND_BY_DEVICE_FAIL("wrong bus/devno combination", 2, 20);
+ DO_TEST_FIND_BY_DEVICE_FAIL("missing bus", 5, 20);
+ DO_TEST_FIND_BY_DEVICE_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_PORT("Logitech mouse", 1, "1.5.3.3");
+ DO_TEST_FIND_BY_PORT_FAIL("wrong bus/port combination", 2, "1.5.3.3");
+ DO_TEST_FIND_BY_PORT_FAIL("missing bus", 5, "1.5.3.3");
+ DO_TEST_FIND_BY_PORT_FAIL("missing port", 1, "8.2.5");
+
+ DO_TEST_FIND_BY_VENDOR_AND_DEVICE("Nexus", 0x18d1, 0x4e22, 1, 20);
+ DO_TEST_FIND_BY_VENDOR_AND_DEVICE_FAIL("Bogus vendor and product", 0xf00d, 0xbeef, 1, 25);
+ DO_TEST_FIND_BY_VENDOR_AND_DEVICE_FAIL("Nexus wrong devnum", 0x18d1, 0x4e22, 1, 25);
+ DO_TEST_FIND_BY_VENDOR_AND_DEVICE_FAIL("Bogus", 0xf00d, 0xbeef, 1024, 768);
+
+ DO_TEST_FIND_BY_VENDOR_AND_PORT("Nexus", 0x046d, 0xc069, 1, "1.5.3.3");
+ DO_TEST_FIND_BY_VENDOR_AND_PORT_FAIL("Bogus vendor and product", 0xf00d, 0xbeef, 1, "1.5.3.3");
+ DO_TEST_FIND_BY_VENDOR_AND_PORT_FAIL("Nexus wrong port", 0x18d1, 0x4e22, 1, "8.2.5");
+ DO_TEST_FIND_BY_VENDOR_AND_PORT_FAIL("Bogus", 0xf00d, 0xbeef, 1024, "1.1.1.1");
+
if (virTestRun("USB List test", testUSBList, NULL) < 0)
rv = -1;
--
2.39.5On Mon, Aug 18, 2025 at 04:34:13PM +0200, Maximilian Martin via Devel wrote:
> From: Maximilian Martin <maximilian_martin@gmx.de>
>
> This patch implements USB bus/port matching. In addition
> to vendor/product and bus/device matching, bus/port
> matching is implemented. This involves additions and
> refactorings in the domain configuration, host device
> handling, and USB helpers. Also, test methods are
> refactored and extended.
>
> Signed-off-by: Maximilian Martin <maximilian_martin@gmx.de>
> ---
> src/conf/domain_conf.c | 58 +++++++++++--
> src/conf/domain_conf.h | 1 +
> src/hypervisor/virhostdev.c | 131 +++++++++++++++++------------
> src/libvirt_private.syms | 2 -
> src/util/virusb.c | 160 ++++++++++++------------------------
> src/util/virusb.h | 22 ++---
> tests/virusbtest.c | 149 +++++++++++++++++++++++----------
> 7 files changed, 298 insertions(+), 225 deletions(-)
This patch is too big and needs splitting up into three pieces
* Refactoring the virUSBDeviceFind APIs to generalize them
* Extending virUSBDeviceFind to support 'port' searches
* Adding 'port' to the XML schema / hostdev code
Since this is the only comment I have on this series, I've
taken the liberty to do that split myself so no need to resend.
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 7766e302ec..63f3032892 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2680,6 +2680,15 @@ virDomainHostdevSubsysSCSIClear(virDomainHostdevSubsysSCSI *scsisrc)
> }
> }
>
> +static void
> +virDomainHostdevSubsysUSBClear(virDomainHostdevSubsysUSB *usbsrc)
> +{
> + if (!usbsrc)
> + return;
> +
> + VIR_FREE(usbsrc->port);
> +}
> +
>
> static void
> virDomainHostdevDefClear(virDomainHostdevDef *def)
> @@ -2723,6 +2732,8 @@ virDomainHostdevDefClear(virDomainHostdevDef *def)
> g_clear_pointer(&def->source.subsys.u.pci.origstates, virBitmapFree);
> break;
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
> + virDomainHostdevSubsysUSBClear(&def->source.subsys.u.usb);
> + break;
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> break;
> @@ -5961,13 +5972,38 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
> }
>
> if ((addressNode = virXPathNode("./address", ctxt))) {
> + bool foundDevice = false;
> + bool foundPort = false;
> + g_autofree char *port = NULL;
> + int rc = -1;
> +
> if (virXMLPropUInt(addressNode, "bus", 0,
> - VIR_XML_PROP_REQUIRED, &usbsrc->bus) < 0)
> + VIR_XML_PROP_REQUIRED, &usbsrc->bus) < 0) {
> return -1;
> + }
>
> - if (virXMLPropUInt(addressNode, "device", 0,
> - VIR_XML_PROP_REQUIRED, &usbsrc->device) < 0)
> + rc = virXMLPropUInt(addressNode, "device", 0,
> + VIR_XML_PROP_NONE, &usbsrc->device);
> + if (rc < 0)
> return -1;
> + else if (rc > 0)
> + foundDevice = true;
> +
> + port = virXMLPropString(addressNode, "port");
> + if (port && *port) {
> + usbsrc->port = g_steal_pointer(&port);
> + foundPort = true;
> + }
> +
> + if (!foundDevice && !foundPort) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("usb address needs either device id or port"));
> + return -1;
> + } else if (foundDevice && foundPort) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("found both device id and port in usb address (ambiguous setting)"));
> + return -1;
> + }
> }
>
> return 0;
> @@ -14851,8 +14887,13 @@ virDomainHostdevMatchSubsysUSB(virDomainHostdevDef *first,
> virDomainHostdevSubsysUSB *first_usbsrc = &first->source.subsys.u.usb;
> virDomainHostdevSubsysUSB *second_usbsrc = &second->source.subsys.u.usb;
>
> - if (first_usbsrc->bus && first_usbsrc->device) {
> - /* specified by bus location on host */
> + if (first_usbsrc->bus && first_usbsrc->port) {
> + /* specified by bus and port on host */
> + if (first_usbsrc->bus == second_usbsrc->bus &&
> + STREQ_NULLABLE(first_usbsrc->port, second_usbsrc->port))
> + return 1;
> + } else if (first_usbsrc->bus && first_usbsrc->device) {
> + /* specified by bus and device id on host */
> if (first_usbsrc->bus == second_usbsrc->bus &&
> first_usbsrc->device == second_usbsrc->device)
> return 1;
> @@ -24511,10 +24552,15 @@ virDomainHostdevDefFormatSubsysUSB(virBuffer *buf,
> virBufferAsprintf(&sourceChildBuf, "<product id='0x%.4x'/>\n", usbsrc->product);
> }
>
> - if (usbsrc->bus || usbsrc->device)
> + if (usbsrc->bus && usbsrc->port) {
> + virBufferAsprintf(&sourceChildBuf, "<address %sbus='%d' port='%s'/>\n",
> + includeTypeInAddr ? "type='usb' " : "",
> + usbsrc->bus, usbsrc->port);
> + } else if (usbsrc->bus || usbsrc->device) {
> virBufferAsprintf(&sourceChildBuf, "<address %sbus='%d' device='%d'/>\n",
> includeTypeInAddr ? "type='usb' " : "",
> usbsrc->bus, usbsrc->device);
> + }
>
> virXMLFormatElement(buf, "source", &sourceAttrBuf, &sourceChildBuf);
> }
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index eca820892e..a655dc57a8 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -228,6 +228,7 @@ struct _virDomainHostdevSubsysUSB {
> on vendor/product */
> unsigned bus;
> unsigned device;
> + char *port;
>
> unsigned vendor;
> unsigned product;
> diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c
> index 7d7df4418d..19907c76ba 100644
> --- a/src/hypervisor/virhostdev.c
> +++ b/src/hypervisor/virhostdev.c
> @@ -1359,6 +1359,41 @@ virHostdevMarkUSBDevices(virHostdevManager *mgr,
> return -1;
> }
>
> +static int
> +virHostdevFindUSBDeviceWithFlags(virDomainHostdevDef *hostdev,
> + bool mandatory,
> + unsigned int flags,
> + virUSBDevice **usb)
> +{
> + virDomainHostdevSubsysUSB *usbsrc = &hostdev->source.subsys.u.usb;
> + unsigned vendor = usbsrc->vendor;
> + unsigned product = usbsrc->product;
> + unsigned bus = usbsrc->bus;
> + const char *port = usbsrc->port;
> + unsigned device = usbsrc->device;
> + g_autoptr(virUSBDeviceList) devs = NULL;
> + int rc;
> +
> + rc = virUSBDeviceFind(vendor, product, bus, device, port, NULL,
> + mandatory, flags, &devs);
> + if (rc < 0)
> + return -1;
> +
> + if (rc == 1) {
> + *usb = virUSBDeviceListGet(devs, 0);
> + virUSBDeviceListSteal(devs, *usb);
> + }
> +
> + if (rc > 1) {
> + virReportError(VIR_ERR_OPERATION_FAILED,
> + _("Multiple USB devices for %1$x:%2$x, use <address> to specify one"),
> + vendor, product);
> + return -1;
> + }
> +
> + return rc;
> +}
> +
>
> int
> virHostdevFindUSBDevice(virDomainHostdevDef *hostdev,
> @@ -1367,77 +1402,65 @@ virHostdevFindUSBDevice(virDomainHostdevDef *hostdev,
> {
> virDomainHostdevSubsysUSB *usbsrc = &hostdev->source.subsys.u.usb;
> unsigned vendor = usbsrc->vendor;
> - unsigned product = usbsrc->product;
> unsigned bus = usbsrc->bus;
> unsigned device = usbsrc->device;
> + const char *port = usbsrc->port;
> bool autoAddress = usbsrc->autoAddress;
> + unsigned int flags = 0;
> int rc;
>
> *usb = NULL;
>
> - if (vendor && bus) {
> - rc = virUSBDeviceFind(vendor, product, bus, device,
> - NULL,
> - autoAddress ? false : mandatory,
> - usb);
> - if (rc < 0) {
> - return -1;
> - } 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);
> - }
> + if (vendor)
> + flags |= USB_DEVICE_FIND_BY_VENDOR;
> + if (device)
> + flags |= USB_DEVICE_FIND_BY_DEVICE;
> + if (port)
> + flags |= USB_DEVICE_FIND_BY_PORT;
> +
> + /* Rule out invalid cases. */
> + if (vendor && device && port) {
> + virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> + _("Cannot match USB device on vendor/product, bus/device, and bus/port at once."));
> + } else if (device && port) {
> + virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> + _("Cannot match USB device on bus/device and bus/port at once."));
> + } else if (!vendor && !device && !port) {
> + virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> + _("No matching fields for USB device found. Vendor/product, bus/device, or bus/port required."));
> + return -1;
> }
>
> - /* When vendor is specified, its USB address is either unspecified or the
> - * device could not be found at the USB device where it had been
> - * automatically found before.
> - */
> - if (vendor) {
> - g_autoptr(virUSBDeviceList) devs = NULL;
> + /* First attempt, matching on all valid fields. */
> + rc = virHostdevFindUSBDeviceWithFlags(hostdev,
> + autoAddress ? false : mandatory,
> + flags, usb);
> + if (rc < 0)
> + return -1;
>
> - rc = virUSBDeviceFindByVendor(vendor, product, NULL, mandatory, &devs);
> - if (rc < 0) {
> - return -1;
> - } else if (rc == 0) {
> - goto out;
> - } else if (rc > 1) {
> - if (autoAddress) {
> - virReportError(VIR_ERR_OPERATION_FAILED,
> - _("Multiple USB devices for %1$x:%2$x were found, but none of them is at bus:%3$u device:%4$u"),
> - vendor, product, bus, device);
> - } else {
> - virReportError(VIR_ERR_OPERATION_FAILED,
> - _("Multiple USB devices for %1$x:%2$x, use <address> to specify one"),
> - vendor, product);
> - }
> + if (rc != 1 && autoAddress && device) {
> + VIR_INFO("USB device could not be found at previous address "
> + "(bus:%u device:%u)", bus, device);
> +
> + /* Second attempt, for when the device number has changed. */
> + flags &= ~USB_DEVICE_FIND_BY_DEVICE;
> + usbsrc->device = 0;
> +
> + rc = virHostdevFindUSBDeviceWithFlags(hostdev, mandatory,
> + flags, usb);
> + if (rc < 0)
> return -1;
> - }
>
> - *usb = virUSBDeviceListGet(devs, 0);
> - virUSBDeviceListSteal(devs, *usb);
> + usbsrc->autoAddress = true;
> + }
>
> + if (!*usb) {
> + hostdev->missing = true;
> + } else if (!usbsrc->bus || !usbsrc->device) {
> usbsrc->bus = virUSBDeviceGetBus(*usb);
> usbsrc->device = virUSBDeviceGetDevno(*usb);
> - usbsrc->autoAddress = true;
> -
> - if (autoAddress) {
> - VIR_INFO("USB device %x:%x found at bus:%u device:%u (moved"
> - " from bus:%u device:%u)",
> - vendor, product,
> - usbsrc->bus, usbsrc->device,
> - bus, device);
> - }
> - } else if (bus) {
> - if (virUSBDeviceFindByBus(bus, device, NULL, mandatory, usb) < 0)
> - return -1;
> }
>
> - out:
> - if (!*usb)
> - hostdev->missing = true;
> return 0;
> }
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index b846011f0f..bc01f25d4c 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -3680,8 +3680,6 @@ virURIResolveAlias;
> # util/virusb.h
> virUSBDeviceFileIterate;
> virUSBDeviceFind;
> -virUSBDeviceFindByBus;
> -virUSBDeviceFindByVendor;
> virUSBDeviceFree;
> virUSBDeviceGetBus;
> virUSBDeviceGetDevno;
> diff --git a/src/util/virusb.c b/src/util/virusb.c
> index cf3461f80a..85ad3d58ce 100644
> --- a/src/util/virusb.c
> +++ b/src/util/virusb.c
> @@ -61,12 +61,6 @@ struct _virUSBDeviceList {
> virUSBDevice **devs;
> };
>
> -typedef enum {
> - USB_DEVICE_ALL = 0,
> - USB_DEVICE_FIND_BY_VENDOR = 1 << 0,
> - USB_DEVICE_FIND_BY_BUS = 1 << 1,
> -} virUSBDeviceFindFlags;
> -
> static virClass *virUSBDeviceListClass;
>
> static void virUSBDeviceListDispose(void *obj);
> @@ -102,11 +96,29 @@ static int virUSBSysReadFile(const char *f_name, const char *d_name,
> return 0;
> }
>
> +static int
> +virUSBSysReadFileStr(const char *f_name,
> + const char *d_name,
> + char **value)
> +{
> + char *buf = NULL;
> + g_autofree char *filename = NULL;
> +
> + filename = g_strdup_printf(USB_SYSFS "/devices/%s/%s", d_name, f_name);
> +
> + if (virFileReadAll(filename, 1024, &buf) < 0)
> + return -1;
> +
> + *value = buf;
> + return 0;
> +}
> +
> static virUSBDeviceList *
> virUSBDeviceSearch(unsigned int vendor,
> unsigned int product,
> unsigned int bus,
> unsigned int devno,
> + const char *port,
> const char *vroot,
> unsigned int flags)
> {
> @@ -127,6 +139,8 @@ virUSBDeviceSearch(unsigned int vendor,
>
> while ((direrr = virDirRead(dir, &de, USB_SYSFS "/devices")) > 0) {
> unsigned int found_prod, found_vend, found_bus, found_devno;
> + g_autofree char *found_port = NULL;
> + bool port_matches;
> char *tmpstr = de->d_name;
>
> if (strchr(de->d_name, ':'))
> @@ -154,16 +168,31 @@ virUSBDeviceSearch(unsigned int vendor,
> 10, &found_devno) < 0)
> goto cleanup;
>
> - if ((flags & USB_DEVICE_FIND_BY_VENDOR) &&
> - (found_prod != product || found_vend != vendor))
> - continue;
> + if (virUSBSysReadFileStr("devpath", de->d_name,
> + &found_port) < 0) {
> + goto cleanup;
> + } else {
> + virStringTrimOptionalNewline(found_port);
> + port_matches = STREQ_NULLABLE(found_port, port);
> + }
>
> - if (flags & USB_DEVICE_FIND_BY_BUS) {
> + if (flags & USB_DEVICE_FIND_BY_VENDOR) {
> + if (found_prod != product || found_vend != vendor)
> + continue;
> + }
> +
> + if (flags & USB_DEVICE_FIND_BY_DEVICE) {
> if (found_bus != bus || found_devno != devno)
> continue;
> found = true;
> }
>
> + if (flags & USB_DEVICE_FIND_BY_PORT) {
> + if (found_bus != bus || !port_matches)
> + continue;
> + found = true;
> + }
> +
> usb = virUSBDeviceNew(found_bus, found_devno, vroot);
>
> if (!usb)
> @@ -185,123 +214,42 @@ virUSBDeviceSearch(unsigned int vendor,
> return ret;
> }
>
> -int
> -virUSBDeviceFindByVendor(unsigned int vendor,
> - unsigned int product,
> - const char *vroot,
> - bool mandatory,
> - virUSBDeviceList **devices)
> -{
> - virUSBDeviceList *list;
> - int count;
> -
> - if (!(list = virUSBDeviceSearch(vendor, product, 0, 0,
> - vroot,
> - USB_DEVICE_FIND_BY_VENDOR)))
> - return -1;
> -
> - if (list->count == 0) {
> - virObjectUnref(list);
> - if (!mandatory) {
> - VIR_DEBUG("Did not find USB device %04x:%04x",
> - vendor, product);
> - if (devices)
> - *devices = NULL;
> - return 0;
> - }
> -
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("Did not find USB device %1$04x:%2$04x"), vendor, product);
> - return -1;
> - }
> -
> - count = list->count;
> - if (devices)
> - *devices = list;
> - else
> - virObjectUnref(list);
> -
> - return count;
> -}
> -
> -int
> -virUSBDeviceFindByBus(unsigned int bus,
> - unsigned int devno,
> - const char *vroot,
> - bool mandatory,
> - virUSBDevice **usb)
> -{
> - virUSBDeviceList *list;
> -
> - if (!(list = virUSBDeviceSearch(0, 0, bus, devno,
> - vroot,
> - USB_DEVICE_FIND_BY_BUS)))
> - return -1;
> -
> - if (list->count == 0) {
> - virObjectUnref(list);
> - if (!mandatory) {
> - VIR_DEBUG("Did not find USB device bus:%u device:%u",
> - bus, devno);
> - if (usb)
> - *usb = NULL;
> - return 0;
> - }
> -
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("Did not find USB device bus:%1$u device:%2$u"),
> - bus, devno);
> - return -1;
> - }
> -
> - if (usb) {
> - *usb = virUSBDeviceListGet(list, 0);
> - virUSBDeviceListSteal(list, *usb);
> - }
> - virObjectUnref(list);
> -
> - return 0;
> -}
> -
> int
> virUSBDeviceFind(unsigned int vendor,
> unsigned int product,
> unsigned int bus,
> unsigned int devno,
> + const char *port,
> const char *vroot,
> bool mandatory,
> - virUSBDevice **usb)
> + unsigned int flags,
> + virUSBDeviceList **devices)
> {
> - virUSBDeviceList *list;
> + g_autoptr(virUSBDeviceList) list = NULL;
> + int count;
>
> - 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, bus, devno, port,
> vroot, flags)))
> return -1;
>
> - if (list->count == 0) {
> - virObjectUnref(list);
> + count = list->count;
> + if (count == 0) {
> if (!mandatory) {
> - VIR_DEBUG("Did not find USB device %04x:%04x bus:%u device:%u",
> - vendor, product, bus, devno);
> - if (usb)
> - *usb = NULL;
> + if (devices)
> + *devices = NULL;
> return 0;
> }
>
> virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("Did not find USB device %1$04x:%2$04x bus:%3$u device:%4$u"),
> - vendor, product, bus, devno);
> + _("Did not find matching USB device: vid:%1$04x, pid:%2$04x, bus:%3$u, device:%4$u, port:%5$s"),
> + vendor, product, bus, devno, port ? port : "");
> return -1;
> }
>
> - if (usb) {
> - *usb = virUSBDeviceListGet(list, 0);
> - virUSBDeviceListSteal(list, *usb);
> - }
> - virObjectUnref(list);
> + if (devices)
> + *devices = g_steal_pointer(&list);
>
> - return 0;
> + return count;
> }
>
> virUSBDevice *
> diff --git a/src/util/virusb.h b/src/util/virusb.h
> index d2b3f69942..86cc0a9d3d 100644
> --- a/src/util/virusb.h
> +++ b/src/util/virusb.h
> @@ -30,30 +30,26 @@ typedef struct _virUSBDeviceList virUSBDeviceList;
>
> G_DEFINE_AUTOPTR_CLEANUP_FUNC(virUSBDeviceList, virObjectUnref);
>
> +typedef enum {
> + USB_DEVICE_ALL = 0,
> + USB_DEVICE_FIND_BY_VENDOR = 1 << 0,
> + USB_DEVICE_FIND_BY_DEVICE = 1 << 1,
> + USB_DEVICE_FIND_BY_PORT = 1 << 2,
> +} virUSBDeviceFindFlags;
>
> virUSBDevice *virUSBDeviceNew(unsigned int bus,
> unsigned int devno,
> const char *vroot);
>
> -int virUSBDeviceFindByBus(unsigned int bus,
> - unsigned int devno,
> - const char *vroot,
> - bool mandatory,
> - virUSBDevice **usb);
> -
> -int virUSBDeviceFindByVendor(unsigned int vendor,
> - unsigned int product,
> - const char *vroot,
> - bool mandatory,
> - virUSBDeviceList **devices);
> -
> int virUSBDeviceFind(unsigned int vendor,
> unsigned int product,
> unsigned int bus,
> unsigned int devno,
> + const char *port,
> const char *vroot,
> bool mandatory,
> - virUSBDevice **usb);
> + unsigned int flags,
> + virUSBDeviceList **devices);
>
> void virUSBDeviceFree(virUSBDevice *dev);
> int virUSBDeviceSetUsedBy(virUSBDevice *dev,
> diff --git a/tests/virusbtest.c b/tests/virusbtest.c
> index 870e136321..12ac338df9 100644
> --- a/tests/virusbtest.c
> +++ b/tests/virusbtest.c
> @@ -26,9 +26,11 @@
> #define VIR_FROM_THIS VIR_FROM_NONE
>
> typedef enum {
> - FIND_BY_ALL,
> FIND_BY_VENDOR,
> - FIND_BY_BUS
> + FIND_BY_DEVICE,
> + FIND_BY_PORT,
> + FIND_BY_VENDOR_AND_DEVICE,
> + FIND_BY_VENDOR_AND_PORT
> } testUSBFindFlags;
>
> struct findTestInfo {
> @@ -37,6 +39,7 @@ struct findTestInfo {
> unsigned int product;
> unsigned int bus;
> unsigned int devno;
> + const char *port;
> const char *vroot;
> bool mandatory;
> int how;
> @@ -70,25 +73,34 @@ static int testDeviceFind(const void *opaque)
> g_autoptr(virUSBDeviceList) devs = NULL;
> int rv = 0;
> size_t i, ndevs = 0;
> + unsigned int flags = 0;
>
> switch (info->how) {
> - case FIND_BY_ALL:
> - rv = virUSBDeviceFind(info->vendor, info->product,
> - info->bus, info->devno,
> - info->vroot, info->mandatory, &dev);
> - break;
> case FIND_BY_VENDOR:
> - rv = virUSBDeviceFindByVendor(info->vendor, info->product,
> - info->vroot, info->mandatory, &devs);
> + flags = USB_DEVICE_FIND_BY_VENDOR;
> + break;
> + case FIND_BY_DEVICE:
> + flags = USB_DEVICE_FIND_BY_DEVICE;
> + break;
> + case FIND_BY_PORT:
> + flags = USB_DEVICE_FIND_BY_PORT;
> break;
> - case FIND_BY_BUS:
> - rv = virUSBDeviceFindByBus(info->bus, info->devno,
> - info->vroot, info->mandatory, &dev);
> + case FIND_BY_VENDOR_AND_DEVICE:
> + flags = USB_DEVICE_FIND_BY_VENDOR |
> + USB_DEVICE_FIND_BY_DEVICE;
> + break;
> + case FIND_BY_VENDOR_AND_PORT:
> + flags = USB_DEVICE_FIND_BY_VENDOR |
> + USB_DEVICE_FIND_BY_PORT;
> break;
> }
>
> + rv = virUSBDeviceFind(info->vendor, info->product,
> + info->bus, info->devno, info->port,
> + info->vroot, info->mandatory, flags, &devs);
> +
> if (info->expectFailure) {
> - if (rv == 0) {
> + if (rv >= 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> "unexpected success");
> } else {
> @@ -99,9 +111,20 @@ static int testDeviceFind(const void *opaque)
> goto cleanup;
> }
>
> + if (info->how != FIND_BY_VENDOR) {
> + if (rv == 1) {
> + dev = virUSBDeviceListGet(devs, 0);
> + virUSBDeviceListSteal(devs, dev);
> + } else {
> + goto cleanup;
> + }
> + }
> +
> switch (info->how) {
> - case FIND_BY_ALL:
> - case FIND_BY_BUS:
> + case FIND_BY_DEVICE:
> + case FIND_BY_PORT:
> + case FIND_BY_VENDOR_AND_DEVICE:
> + case FIND_BY_VENDOR_AND_PORT:
> if (virUSBDeviceFileIterate(dev, testDeviceFileActor, NULL) < 0)
> goto cleanup;
> break;
> @@ -146,14 +169,17 @@ testUSBList(const void *opaque G_GNUC_UNUSED)
> virUSBDeviceList *list = NULL;
> virUSBDeviceList *devlist = NULL;
> virUSBDevice *dev = NULL;
> + virUSBDeviceList *devs = NULL;
> int ret = -1;
> + int rv;
> size_t i, ndevs;
>
> if (!(list = virUSBDeviceListNew()))
> goto cleanup;
>
> #define EXPECTED_NDEVS_ONE 3
> - if (virUSBDeviceFindByVendor(0x1d6b, 0x0002, NULL, true, &devlist) < 0)
> + if (virUSBDeviceFind(0x1d6b, 0x0002, 0, 0, NULL, NULL, true,
> + USB_DEVICE_FIND_BY_VENDOR, &devlist) < 0)
> goto cleanup;
>
> ndevs = virUSBDeviceListCount(devlist);
> @@ -176,7 +202,8 @@ testUSBList(const void *opaque G_GNUC_UNUSED)
> goto cleanup;
>
> #define EXPECTED_NDEVS_TWO 3
> - if (virUSBDeviceFindByVendor(0x18d1, 0x4e22, NULL, true, &devlist) < 0)
> + if (virUSBDeviceFind(0x18d1, 0x4e22, 0, 0, NULL, NULL, true,
> + USB_DEVICE_FIND_BY_VENDOR, &devlist) < 0)
> goto cleanup;
>
> ndevs = virUSBDeviceListCount(devlist);
> @@ -196,8 +223,16 @@ 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)
> + rv = virUSBDeviceFind(0x18d1, 0x4e22, 1, 20, NULL, NULL, true,
> + USB_DEVICE_FIND_BY_VENDOR |
> + USB_DEVICE_FIND_BY_DEVICE, &devs);
> + if (rv != 1) {
> goto cleanup;
> + } else {
> + dev = virUSBDeviceListGet(devs, 0);
> + virUSBDeviceListSteal(devs, dev);
> + }
> + virObjectUnref(devs);
>
> if (!virUSBDeviceListFind(list, dev)) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -229,49 +264,75 @@ 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, bus, devno, \
> + port, vroot, mand, how, fail) \
> do { \
> struct findTestInfo data = { name, vend, prod, bus, \
> - devno, vroot, mand, how, fail \
> + devno, port, vroot, mand, how, fail \
> }; \
> 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_BY_BUS(name, bus, devno) \
> - DO_TEST_FIND_FULL(name, 101, 202, 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, \
> - 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, 123, 456, NULL, 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, 123, 456, NULL, NULL, true, \
> FIND_BY_VENDOR, true)
>
> - 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_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);
> +#define DO_TEST_FIND_BY_DEVICE(name, bus, devno) \
> + DO_TEST_FIND_FULL(name, 0x1010, 0x2020, bus, devno, NULL, NULL, true, \
> + FIND_BY_DEVICE, false)
> +#define DO_TEST_FIND_BY_DEVICE_FAIL(name, bus, devno) \
> + DO_TEST_FIND_FULL(name, 0x1010, 0x2020, bus, devno, NULL, NULL, true, \
> + FIND_BY_DEVICE, true)
> +
> +#define DO_TEST_FIND_BY_PORT(name, bus, port) \
> + DO_TEST_FIND_FULL(name, 0x1010, 0x2020, bus, 456, port, NULL, true, \
> + FIND_BY_PORT, false)
> +#define DO_TEST_FIND_BY_PORT_FAIL(name, bus, port) \
> + DO_TEST_FIND_FULL(name, 0x1010, 0x2020, bus, 456, port, NULL, true, \
> + FIND_BY_PORT, true)
> +
> +#define DO_TEST_FIND_BY_VENDOR_AND_DEVICE(name, vend, prod, bus, devno) \
> + DO_TEST_FIND_FULL(name, vend, prod, bus, devno, NULL, NULL, true, \
> + FIND_BY_VENDOR_AND_DEVICE, false)
> +#define DO_TEST_FIND_BY_VENDOR_AND_DEVICE_FAIL(name, vend, prod, bus, devno) \
> + DO_TEST_FIND_FULL(name, vend, prod, bus, devno, NULL, NULL, true, \
> + FIND_BY_VENDOR_AND_DEVICE, true)
> +
> +#define DO_TEST_FIND_BY_VENDOR_AND_PORT(name, vend, prod, bus, port) \
> + DO_TEST_FIND_FULL(name, vend, prod, bus, 456, port, NULL, true, \
> + FIND_BY_VENDOR_AND_PORT, false)
> +#define DO_TEST_FIND_BY_VENDOR_AND_PORT_FAIL(name, vend, prod, bus, port) \
> + DO_TEST_FIND_FULL(name, vend, prod, bus, 456, port, NULL, true, \
> + FIND_BY_VENDOR_AND_PORT, true)
> +
> + DO_TEST_FIND_BY_DEVICE("integrated camera", 1, 5);
> + DO_TEST_FIND_BY_DEVICE_FAIL("wrong bus/devno combination", 2, 20);
> + DO_TEST_FIND_BY_DEVICE_FAIL("missing bus", 5, 20);
> + DO_TEST_FIND_BY_DEVICE_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_PORT("Logitech mouse", 1, "1.5.3.3");
> + DO_TEST_FIND_BY_PORT_FAIL("wrong bus/port combination", 2, "1.5.3.3");
> + DO_TEST_FIND_BY_PORT_FAIL("missing bus", 5, "1.5.3.3");
> + DO_TEST_FIND_BY_PORT_FAIL("missing port", 1, "8.2.5");
> +
> + DO_TEST_FIND_BY_VENDOR_AND_DEVICE("Nexus", 0x18d1, 0x4e22, 1, 20);
> + DO_TEST_FIND_BY_VENDOR_AND_DEVICE_FAIL("Bogus vendor and product", 0xf00d, 0xbeef, 1, 25);
> + DO_TEST_FIND_BY_VENDOR_AND_DEVICE_FAIL("Nexus wrong devnum", 0x18d1, 0x4e22, 1, 25);
> + DO_TEST_FIND_BY_VENDOR_AND_DEVICE_FAIL("Bogus", 0xf00d, 0xbeef, 1024, 768);
> +
> + DO_TEST_FIND_BY_VENDOR_AND_PORT("Nexus", 0x046d, 0xc069, 1, "1.5.3.3");
> + DO_TEST_FIND_BY_VENDOR_AND_PORT_FAIL("Bogus vendor and product", 0xf00d, 0xbeef, 1, "1.5.3.3");
> + DO_TEST_FIND_BY_VENDOR_AND_PORT_FAIL("Nexus wrong port", 0x18d1, 0x4e22, 1, "8.2.5");
> + DO_TEST_FIND_BY_VENDOR_AND_PORT_FAIL("Bogus", 0xf00d, 0xbeef, 1024, "1.1.1.1");
> +
> if (virTestRun("USB List test", testUSBList, NULL) < 0)
> rv = -1;
>
> --
> 2.39.5
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
© 2016 - 2026 Red Hat, Inc.