[libvirt] [PATCH V2 0/2] libxl USB vendor/product support
V2 of Cedric's patches to allow specifying USB hostdevs by vendor/product in the libxl driver https://www.redhat.com/archives/libvir-list/2016-August/msg00305.html Cédric Bosdonnat (2): Add virHostdevFindUSBDevice to private symbols libxl: allow vendor/product addressing for USB hostdevs src/libvirt_private.syms | 1 + src/libxl/libxl_conf.c | 33 +++++++++++++++++++++++---------- src/util/virhostdev.c | 2 +- src/util/virhostdev.h | 6 ++++++ 4 files changed, 31 insertions(+), 11 deletions(-) -- 2.1.4
From: Cédric Bosdonnat <cbosdonnat@suse.com> Finding an USB device from the vendor/device values will be needed by libxl driver to convert from vendor/device to bus/dev addresses. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- V2: Remove unnecessary ATTRIBUTE_NONNULL 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..f2f51bd 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(3); int virHostdevPrepareUSBDevices(virHostdevManagerPtr hostdev_mgr, const char *drv_name, -- 2.1.4
On Wed, Aug 10, 2016 at 06:39:11PM -0600, Jim Fehlig wrote:
From: Cédric Bosdonnat <cbosdonnat@suse.com>
Finding an USB device from the vendor/device values will be needed by libxl driver to convert from vendor/device to bus/dev addresses.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> ---
ACK
From: Cédric Bosdonnat <cbosdonnat@suse.com> 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. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- V2: - Initialize local 'usb' variable - Use 'bus' and 'device' from virUSBDevice retrieved from virHostdevFindUSBDevice instead of relying on the function's side-affects src/libxl/libxl_conf.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 5202ca1..06cbc2c 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1559,23 +1559,36 @@ int libxlMakeUSB(virDomainHostdevDefPtr hostdev, libxl_device_usbdev *usbdev) { virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; + virUSBDevicePtr usb = NULL; + 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) { + usbdev->u.hostdev.hostbus = usbsrc->bus; + usbdev->u.hostdev.hostaddr = usbsrc->device; + } else { + if (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 = virUSBDeviceGetBus(usb); + usbdev->u.hostdev.hostaddr = virUSBDeviceGetDevno(usb); } - usbdev->u.hostdev.hostbus = usbsrc->bus; - usbdev->u.hostdev.hostaddr = usbsrc->device; + ret = 0; - return 0; + cleanup: + virUSBDeviceFree(usb); + + return ret; } static int -- 2.1.4
On Wed, Aug 10, 2016 at 06:39:12PM -0600, Jim Fehlig wrote:
From: Cédric Bosdonnat <cbosdonnat@suse.com>
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.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> ---
V2: - Initialize local 'usb' variable - Use 'bus' and 'device' from virUSBDevice retrieved from virHostdevFindUSBDevice instead of relying on the function's side-affects
src/libxl/libxl_conf.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 5202ca1..06cbc2c 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1559,23 +1559,36 @@ int libxlMakeUSB(virDomainHostdevDefPtr hostdev, libxl_device_usbdev *usbdev) { virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; + virUSBDevicePtr usb = NULL; + int ret = -1;
if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) - return -1; + goto cleanup;
There's no need to have 'ret' and cleanup section, you can still return -1 here, since...
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) { + usbdev->u.hostdev.hostbus = usbsrc->bus; + usbdev->u.hostdev.hostaddr = usbsrc->device; + } else { + if (virHostdevFindUSBDevice(hostdev, true, &usb) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("failed to find USB device busnum:devnum " + "for %x:%x"), + usbsrc->vendor, usbsrc->product);
... this and ...
+ goto cleanup; + } + + usbdev->u.hostdev.hostbus = virUSBDeviceGetBus(usb); + usbdev->u.hostdev.hostaddr = virUSBDeviceGetDevno(usb);
... this is the only place you need to clean stuff up. But whatever floats your boat, this is just a suggestion ;) ACK, preferably after release, unless this fixes something for you guys. Martin
On Fri, 2016-09-02 at 15:24 +0200, Martin Kletzander wrote:
On Wed, Aug 10, 2016 at 06:39:12PM -0600, Jim Fehlig wrote:
From: Cédric Bosdonnat <cbosdonnat@suse.com>
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.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> ---
V2: - Initialize local 'usb' variable - Use 'bus' and 'device' from virUSBDevice retrieved from virHostdevFindUSBDevice instead of relying on the function's side-affects
src/libxl/libxl_conf.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 5202ca1..06cbc2c 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1559,23 +1559,36 @@ int libxlMakeUSB(virDomainHostdevDefPtr hostdev, libxl_device_usbdev *usbdev) { virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; + virUSBDevicePtr usb = NULL; + int ret = -1;
if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) - return -1; + goto cleanup;
There's no need to have 'ret' and cleanup section, you can still return -1 here, since...
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) { + usbdev->u.hostdev.hostbus = usbsrc->bus; + usbdev->u.hostdev.hostaddr = usbsrc->device; + } else { + if (virHostdevFindUSBDevice(hostdev, true, &usb) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("failed to find USB device busnum:devnum " + "for %x:%x"), + usbsrc->vendor, usbsrc->product);
... this and ...
+ goto cleanup; + } + + usbdev->u.hostdev.hostbus = virUSBDeviceGetBus(usb); + usbdev->u.hostdev.hostaddr = virUSBDeviceGetDevno(usb);
... this is the only place you need to clean stuff up. But whatever floats your boat, this is just a suggestion ;)
ACK, preferably after release, unless this fixes something for you guys. Thanks for the review, I'll push that with your suggestion.
-- Cedric
I'ld ACK this v2 of my patch series, but not sure I'm the right guy to do it. Anyone else can review it? -- Cedric On Wed, 2016-08-10 at 18:39 -0600, Jim Fehlig wrote:
V2 of Cedric's patches to allow specifying USB hostdevs by vendor/product in the libxl driver
https://www.redhat.com/archives/libvir-list/2016-August/msg00305.html
Cédric Bosdonnat (2): Add virHostdevFindUSBDevice to private symbols libxl: allow vendor/product addressing for USB hostdevs
src/libvirt_private.syms | 1 + src/libxl/libxl_conf.c | 33 +++++++++++++++++++++++---------- src/util/virhostdev.c | 2 +- src/util/virhostdev.h | 6 ++++++ 4 files changed, 31 insertions(+), 11 deletions(-)
participants (3)
-
Cedric Bosdonnat -
Jim Fehlig -
Martin Kletzander