Re: [libvirt] [PATCH V2 2/2] libxl: allow vendor/product addressing for USB hostdevs

On Wed, Aug 10, 2016 at 06:39:12PM -0600, Jim Fehlig wrote: > > From: Cédric Bosdonnat <cbosdonnat(a)suse.com&gt; > > 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(a)suse.com&gt; > --- > > 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.