>>> On 5/21/2016 at 05:25 AM, in message <573F80E3.4070905@xxxxxxxx>, Jim Fehlig
<jfehlig@xxxxxxxx> wrote:
> On 05/20/2016 02:25 PM, Jim Fehlig wrote:
> > On 05/20/2016 04:32 AM, Joao Martins wrote:
> >> On 05/19/2016 09:14 AM, 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'.
> >>>
> >>> For example:
> >>> usb.xml:
> >>> <hostdev mode='subsystem' type='usb' managed='no'>
> >>> <source>
> >>> <address bus='1' device='3'/>
> >>> </source>
> >>> </hostdev>
> >>> #xl attach-device dom usb.xml
> >>> #xl detach-device dom usb.xml
> >>>
> >>> Signed-off-by: Chunyan Liu <cyliu@xxxxxxxx>
> >>> ---
> >>> Changes:
> >>> * add LIBXL_HAVE_PVUSB check
> >>> * fix Jim's comments
> >>>
> >>> src/libxl/libxl_driver.c | 136
> ++++++++++++++++++++++++++++++++++++++++++++++-
> >>> 1 file changed, 135 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> >>> index 960673f..a171efe 100644
> >>> --- a/src/libxl/libxl_driver.c
> >>> +++ b/src/libxl/libxl_driver.c
> >>> @@ -3028,6 +3028,56 @@ libxlDomainAttachHostPCIDevice(libxlDriverPrivatePtr
> driver,
> >>> return ret;
> >>> }
> >>>
> >>> +#ifdef LIBXL_HAVE_PVUSB
> >>> +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);
> >>> +
> >>> + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
> >>> + hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
> >>> + goto cleanup;
> >>> +
> >>> + 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 reattach;
> >>> +
> >>> + 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 reattach;
> >>> + }
> >>> +
> >>> + vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
> >>> + ret = 0;
> >>> + goto cleanup;
> >>> +
> >>> + reattach:
> >>> + virHostdevReAttachUSBDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
> >>> + vm->def->name, &hostdev, 1);
> >>> +
> >>> + cleanup:
> >>> + virObjectUnref(cfg);
> >>> + libxl_device_usbdev_dispose(&usbdev);
> >>> + return ret;
> >>> +}
> >>> +#endif
> >>> +
> >>> static int
> >>> libxlDomainAttachHostDevice(libxlDriverPrivatePtr driver,
> >>> virDomainObjPtr vm,
> >>> @@ -3046,6 +3096,13 @@ libxlDomainAttachHostDevice(libxlDriverPrivatePtr
> driver,
> >>> return -1;
> >>> break;
> >>>
> >>> +#ifdef LIBXL_HAVE_PVUSB
> >>> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
> >>> + if (libxlDomainAttachHostUSBDevice(driver, vm, hostdev) < 0)
> >>> + return -1;
> >>> + break;
> >>> +#endif
> >>> +
> >>> default:
> >>> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> >>> _("hostdev subsys type '%s' not supported"),
> >>> @@ -3271,7 +3328,9 @@ 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_PCI ||
> >>> + VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB))
> >> Is this conditional correct when LIBXL_HAVE_PVUSB isn't there?
> > I think there are two problems here. First, (VIR_DOMAIN_DEVICE_DISK ||
> > VIR_DOMAIN_DEVICE_LEASE) evaluates to 1, so essentially this is equivalent
> to
> >
> > if (hostdev->source.subsys.type != 1)
Oh, my god. Your are right. Stupid mistake.
> >
> > Second, I agree that we don't want to add a USB device to the config if
> libxl
> > cant support it.
>
> Something like the below diff?
Yes, I think it's OK. And another place needs to be updated, since now it's
not only pci device, but also could be usb device, so the error message
should be updated.
if (virDomainHostdevFind(vmdef, hostdev, &found) >= 0) {
- pcisrc = &hostdev->source.subsys.u.pci;
virReportError(VIR_ERR_OPERATION_FAILED,
- _("target pci device %.4x:%.2x:%.2x.%.1x\
- already exists"),
- pcisrc->addr.domain, pcisrc->addr.bus,
- pcisrc->addr.slot, pcisrc->addr.function);
+ _("device already exists in domain configuration"));
return -1;
}
Sorry for not noticing that in time.
- Chunyan
>
> Regards,
> Jim
>
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 9001f34..5b6f87a 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -3326,10 +3326,14 @@ 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 ||
> - VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB))
> + switch (hostdev->source.subsys.type) {
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
> +#ifndef LIBXL_HAVE_PVUSB
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
> +#endif
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> return -1;
> + }
>
> if (virDomainHostdevFind(vmdef, hostdev, &found) >= 0) {
> pcisrc = &hostdev->source.subsys.u.pci;
> l
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list