On 08/05/2016 01:10 AM, Cédric Bosdonnat wrote:

Finding an USB device from the vendor/device values will be needed by libxl driver to convert from vendor/device to bus/dev addresses. --- src/libvirt_private.syms | 1 + src/util/virhostdev.c | 2 +- src/util/virhostdev.h | 6 ++++++ 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 419c33d..de8b1fe 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1646,6 +1646,7 @@ virHookPresent;

# util/virhostdev.h +virHostdevFindUSBDevice; virHostdevManagerGetDefault; virHostdevPCINodeDeviceDetach; virHostdevPCINodeDeviceReAttach; diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 9b5ca6f..9c2262e 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -1178,7 +1178,7 @@ virHostdevMarkUSBDevices(virHostdevManagerPtr mgr, }

-static int +int virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev, bool mandatory, virUSBDevicePtr *usb) diff --git a/src/util/virhostdev.h b/src/util/virhostdev.h index c451981..9bb582b 100644 --- a/src/util/virhostdev.h +++ b/src/util/virhostdev.h @@ -66,6 +66,12 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr, unsigned int flags) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); + +int +virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev, + bool mandatory, + virUSBDevicePtr *usb) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);

ATTRIBUTE_NONNULL(2) is not needed. I'm fine with exporting this function from virhostdev, but I'd like to hear what others think since it has some interesting side-affects such as modifying hostdev->source.subsys.u.usb if the device is found by vendor. My first thought is that such behavior is internal to the virhostdev module, but populating hostdev->source.subsys.u.usb->{bus,device} when called externally may be fine. Regards, Jim

On 08/05/2016 01:10 AM, Cédric Bosdonnat wrote:

libxl only has API to address the host USB devices by bus/device. Find the bus/device if the user only provided the vendor/product of the USB device. --- src/libxl/libxl_conf.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 5202ca1..4b758f1 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1559,23 +1559,32 @@ int libxlMakeUSB(virDomainHostdevDefPtr hostdev, libxl_device_usbdev *usbdev) { virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; + virUSBDevicePtr usb;

Needs initialized to NULL. I encountered a libvirtd segfault without it.

+ int ret = -1;

if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) - return -1; + goto cleanup; if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) - return -1; + goto cleanup;

- if (usbsrc->bus <= 0 || usbsrc->device <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("libxenlight supports only USB device " - "specified by busnum:devnum")); - return -1; + if ((usbsrc->bus <= 0 || usbsrc->device <= 0) && + (virHostdevFindUSBDevice(hostdev, true, &usb) < 0)) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("failed to find USB device busnum:devnum " + "for %x:%x"), + usbsrc->vendor, usbsrc->product); + goto cleanup; }

usbdev->u.hostdev.hostbus = usbsrc->bus; usbdev->u.hostdev.hostaddr = usbsrc->device;

I had to read virHostdevFindUSBDevice to understand how this would work :-). If the device is found by vendor/product, virHostdevFindUSBDevice sets hostdev->source.subsys.u.usb->{bus,device}. I don't think we should rely on that internal behavior and instead set hostbus and hostaddr from usb->{bus,dev}. Regards, Jim

- return 0; + ret = 0; + + cleanup: + virUSBDeviceFree(usb); + + return ret; }

static int