>>> On 5/13/2016 at 07:20 AM, in message <57350FA5.3070307@xxxxxxxx>, Jim Fehlig
<jfehlig@xxxxxxxx> wrote:
> Chunyan Liu wrote:
> > Support hot attach/detach a USB host device to guest.
> > Curretnly libxl only supports xen PV guest, and only
>
> s/Curretnly/Currently/
>
> > supports specifying USB host device by 'bus number'
> > and 'device number'.
> >
> > Signed-off-by: Chunyan Liu <cyliu@xxxxxxxx>
> > ---
> > src/libxl/libxl_driver.c | 130
> ++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 129 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> > index 18a0891..8900644 100644
> > --- a/src/libxl/libxl_driver.c
> > +++ b/src/libxl/libxl_driver.c
> > @@ -3024,6 +3024,55 @@ libxlDomainAttachHostPCIDevice(libxlDriverPrivatePtr
> driver,
> > }
> >
> > static int
> > +libxlDomainAttachHostUSBDevice(libxlDriverPrivatePtr driver,
> > + virDomainObjPtr vm,
> > + virDomainHostdevDefPtr hostdev)
> > +{
> > + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
> > + libxl_device_usbdev usbdev;
> > + virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
> > + int ret = -1;
> > +
> > + libxl_device_usbdev_init(&usbdev);
>
> Move the init below the check for hostdev->mode and hostdev->source.subsys.type,
> otherwise it is not disposed if the check fails.
Yes, will update.
>
> > +
> > + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
> > + hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
> > + return ret;
> > +
> > + if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0)
> > + goto cleanup;
> > +
> > + if (virHostdevPrepareUSBDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
> > + vm->def->name, &hostdev, 1, 0) < 0)
> > + goto cleanup;
> > +
> > + if (libxlMakeUSB(hostdev, &usbdev) < 0)
> > + goto error;
> > +
> > + if (libxl_device_usbdev_add(cfg->ctx, vm->def->id, &usbdev, 0) < 0) {
> > + virReportError(VIR_ERR_INTERNAL_ERROR,
> > + _("libxenlight failed to attach usb device
> Busnum:%3x, Devnum:%3x"),
> > + hostdev->source.subsys.u.usb.bus,
> > + hostdev->source.subsys.u.usb.device);
> > + goto error;
> > + }
> > +
> > + vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
> > + ret = 0;
> > + goto cleanup;
> > +
> > + error:
>
> I think 'reattach' is a better name for this label.
Will update.
>
> > + virHostdevReAttachUSBDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
> > + vm->def->name, &hostdev, 1);
> > +
> > + cleanup:
> > + virObjectUnref(cfg);
> > + libxl_device_usbdev_dispose(&usbdev);
> > + return ret;
> > +}
> > +
> > +
> > +static int
> > libxlDomainAttachHostDevice(libxlDriverPrivatePtr driver,
> > virDomainObjPtr vm,
> > virDomainHostdevDefPtr hostdev)
> > @@ -3041,6 +3090,11 @@ libxlDomainAttachHostDevice(libxlDriverPrivatePtr
> driver,
> > return -1;
> > break;
> >
> > + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
> > + if (libxlDomainAttachHostUSBDevice(driver, vm, hostdev) < 0)
> > + return -1;
> > + break;
> > +
> > default:
> > virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > _("hostdev subsys type '%s' not supported"),
> > @@ -3266,7 +3320,8 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef,
> virDomainDeviceDefPtr dev)
> > case VIR_DOMAIN_DEVICE_HOSTDEV:
> > hostdev = dev->data.hostdev;
> >
> > - if (hostdev->source.subsys.type !=
> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
> > + if (hostdev->source.subsys.type !=
> > + (VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB ||
> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI))
>
> if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB
> ||
> hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
>
> > return -1;
> >
> > if (virDomainHostdevFind(vmdef, hostdev, &found) >= 0) {
> > @@ -3385,6 +3440,76 @@ libxlDomainDetachHostPCIDevice(libxlDriverPrivatePtr
> driver,
> > }
> >
> > static int
> > +libxlDomainDetachHostUSBDevice(libxlDriverPrivatePtr driver,
> > + virDomainObjPtr vm,
> > + virDomainHostdevDefPtr hostdev)
> > +{
> > + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
> > + virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys;
> > + virDomainHostdevSubsysUSBPtr usbsrc = &subsys->u.usb;
> > + virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
> > + libxl_device_usbdev usbdev;
> > + libxl_device_usbdev *usbdevs = NULL;
> > + int num = 0;
> > + virDomainHostdevDefPtr detach;
> > + int idx;
> > + size_t i;
> > + bool found = false;
> > + int ret = -1;
> > +
> > + libxl_device_usbdev_init(&usbdev);
> > +
> > + idx = virDomainHostdevFind(vm->def, hostdev, &detach);
> > + if (idx < 0) {
> > + virReportError(VIR_ERR_OPERATION_FAILED,
> > + _("host USB device Busnum: %3x, Devnum:%3x not
> found"),
>
> Space after Busnum, but not Devnum.
Will update.
>
> > + usbsrc->bus, usbsrc->device);
> > + goto cleanup;
> > + }
> > +
> > + usbdevs = libxl_device_usbdev_list(cfg->ctx, vm->def->id, &num);
> > + for (i = 0; i < num; i++) {
> > + if (usbdevs[i].u.hostdev.hostbus == usbsrc->bus &&
> > + usbdevs[i].u.hostdev.hostaddr == usbsrc->device) {
> > + libxl_device_usbdev_copy(cfg->ctx, &usbdev, &usbdevs[i]);
> > + found = true;
> > + break;
> > + }
> > + }
> > + libxl_device_usbdev_list_free(usbdevs, num);
> > +
> > + if (!found) {
> > + virReportError(VIR_ERR_OPERATION_FAILED,
> > + _("host USB device Busnum: %3x, Devnum:%3x not
> found"),
>
> Same here.
>
> > + usbsrc->bus, usbsrc->device);
> > + goto cleanup;
> > + }
> > +
> > + if (libxl_device_usbdev_remove(cfg->ctx, vm->def->id, &usbdev, 0) < 0) {
> > + virReportError(VIR_ERR_INTERNAL_ERROR,
> > + _("libxenlight failed to detach USB device\
> > + Busnum: %3x, Devnum:%3x"),
>
> And here.
>
> > + usbsrc->bus, usbsrc->device);
> > + goto error;
> > + }
> > +
> > + virDomainHostdevRemove(vm->def, idx);
> > +
> > + virHostdevReAttachUSBDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
> > + vm->def->name, &hostdev, 1);
> > +
> > + ret = 0;
> > +
> > + error:
> > + virDomainHostdevDefFree(detach);
>
> The error label will be hit on success of the function. Should
> virDomainHostdevDefFree() be called in 'cleanup' and 'error' dropped?
Yes, that's better. Will update.
Thanks,
Chunyan
>
> Regards,
> Jim
>
> > +
> > + cleanup:
> > + virObjectUnref(cfg);
> > + libxl_device_usbdev_dispose(&usbdev);
> > + return ret;
> > +}
> > +
> > +static int
> > libxlDomainDetachHostDevice(libxlDriverPrivatePtr driver,
> > virDomainObjPtr vm,
> > virDomainHostdevDefPtr hostdev)
> > @@ -3402,6 +3527,9 @@ libxlDomainDetachHostDevice(libxlDriverPrivatePtr
> driver,
> > case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
> > return libxlDomainDetachHostPCIDevice(driver, vm, hostdev);
> >
> > + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
> > + return libxlDomainDetachHostUSBDevice(driver, vm, hostdev);
> > +
> > default:
> > virReportError(VIR_ERR_INTERNAL_ERROR,
> > _("unexpected hostdev type %d"), subsys->type);
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list