[libvirt] [PATCH 0/2] libxl: support pvusb

This patch series is to support pvusb in libxl driver. Chunyan Liu (2): libxl: support creating guest with USB hostdev libxl: support hotplug USB host device src/libxl/libxl_conf.c | 71 +++++++++++++++++++++++++ src/libxl/libxl_conf.h | 3 ++ src/libxl/libxl_domain.c | 4 +- src/libxl/libxl_driver.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 206 insertions(+), 4 deletions(-) -- 2.1.4

Support creating guest with USB host device in config file. Currently libxl only supports xen PV guest, and only supports specifying USB host device by 'bus number' and 'device number'. Signed-off-by: Chunyan Liu <cyliu@suse.com> --- src/libxl/libxl_conf.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++ src/libxl/libxl_conf.h | 3 ++ src/libxl/libxl_domain.c | 4 +-- src/libxl/libxl_driver.c | 2 +- 4 files changed, 77 insertions(+), 3 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 30f2ce9..3b69cbf 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1848,6 +1848,74 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg, } int +libxlMakeUSB(virDomainHostdevDefPtr hostdev, libxl_device_usbdev *usbdev) +{ + virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; + + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + return -1; + if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) + return -1; + + if (usbsrc->bus <= 0 || usbsrc->device <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("libxenlight supports only USB device " + "specified by busnum:devnum")); + return -1; + } + + usbdev->u.hostdev.hostbus = usbsrc->bus; + usbdev->u.hostdev.hostaddr = usbsrc->device; + + return 0; +} + +static int +libxlMakeUSBList(virDomainDefPtr def, libxl_domain_config *d_config) +{ + virDomainHostdevDefPtr *l_hostdevs = def->hostdevs; + size_t nhostdevs = def->nhostdevs; + size_t nusbdevs = 0; + libxl_device_usbdev *x_usbdevs; + size_t i, j; + + if (nhostdevs == 0) + return 0; + + if (VIR_ALLOC_N(x_usbdevs, nhostdevs) < 0) + return -1; + + for (i = 0, j = 0; i < nhostdevs; i++) { + if (l_hostdevs[i]->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + continue; + if (l_hostdevs[i]->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) + continue; + + libxl_device_usbdev_init(&x_usbdevs[j]); + + if (libxlMakeUSB(l_hostdevs[i], &x_usbdevs[j]) < 0) + goto error; + + nusbdevs++; + j++; + } + + VIR_SHRINK_N(x_usbdevs, nhostdevs, nhostdevs - nusbdevs); + d_config->usbdevs = x_usbdevs; + d_config->num_usbdevs = nusbdevs; + + return 0; + + error: + for (i = 0; i < nhostdevs; i++) + libxl_device_usbdev_dispose(&x_usbdevs[i]); + + VIR_FREE(x_usbdevs); + return -1; +} + + +int libxlMakePCI(virDomainHostdevDefPtr hostdev, libxl_device_pci *pcidev) { virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci; @@ -2078,6 +2146,9 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, if (libxlMakePCIList(def, d_config) < 0) return -1; + if (libxlMakeUSBList(def, d_config) < 0) + return -1; + /* * Now that any potential VFBs are defined, update the build info with * the data of the primary display. Some day libxl might implicitely do diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index 24e2911..e3ccca1 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -192,6 +192,9 @@ libxlMakeVfb(virPortAllocatorPtr graphicsports, int libxlMakePCI(virDomainHostdevDefPtr hostdev, libxl_device_pci *pcidev); +int +libxlMakeUSB(virDomainHostdevDefPtr hostdev, libxl_device_usbdev *usbdev); + virDomainXMLOptionPtr libxlCreateXMLConf(void); diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 14a900c..4272dda 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -728,7 +728,7 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver, virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; virHostdevReAttachDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME, - vm->def, VIR_HOSTDEV_SP_PCI, NULL); + vm->def, VIR_HOSTDEV_SP_PCI | VIR_HOSTDEV_SP_USB, NULL); VIR_FREE(priv->lockState); if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0) @@ -1103,7 +1103,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, goto cleanup_dom; if (virHostdevPrepareDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME, - vm->def, VIR_HOSTDEV_SP_PCI) < 0) + vm->def, VIR_HOSTDEV_SP_PCI | VIR_HOSTDEV_SP_USB) < 0) goto cleanup_dom; /* Unlock virDomainObj while creating the domain */ diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index bf97c9c..18a0891 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -383,7 +383,7 @@ libxlReconnectDomain(virDomainObjPtr vm, /* Update hostdev state */ if (virHostdevUpdateActiveDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME, - vm->def, VIR_HOSTDEV_SP_PCI) < 0) + vm->def, VIR_HOSTDEV_SP_PCI | VIR_HOSTDEV_SP_USB) < 0) goto out; if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) -- 2.1.4

Chunyan Liu wrote:
Support creating guest with USB host device in config file. Currently libxl only supports xen PV guest, and only supports specifying USB host device by 'bus number' and 'device number'.
It would be nice to have an example of xl.cfg(5) usbdev= configuration and the corresponding domXML snippet. Actually, the example would be better placed in the commit message for the new patch needed for converting domXML <-> xl.cfg(5) USB config.
Signed-off-by: Chunyan Liu <cyliu@suse.com> --- src/libxl/libxl_conf.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++ src/libxl/libxl_conf.h | 3 ++ src/libxl/libxl_domain.c | 4 +-- src/libxl/libxl_driver.c | 2 +- 4 files changed, 77 insertions(+), 3 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 30f2ce9..3b69cbf 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1848,6 +1848,74 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg, }
int +libxlMakeUSB(virDomainHostdevDefPtr hostdev, libxl_device_usbdev *usbdev) +{ + virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; + + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + return -1; + if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) + return -1; + + if (usbsrc->bus <= 0 || usbsrc->device <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("libxenlight supports only USB device " + "specified by busnum:devnum")); + return -1; + } + + usbdev->u.hostdev.hostbus = usbsrc->bus; + usbdev->u.hostdev.hostaddr = usbsrc->device; + + return 0; +} + +static int +libxlMakeUSBList(virDomainDefPtr def, libxl_domain_config *d_config) +{ + virDomainHostdevDefPtr *l_hostdevs = def->hostdevs; + size_t nhostdevs = def->nhostdevs; + size_t nusbdevs = 0; + libxl_device_usbdev *x_usbdevs; + size_t i, j; + + if (nhostdevs == 0) + return 0; + + if (VIR_ALLOC_N(x_usbdevs, nhostdevs) < 0) + return -1; + + for (i = 0, j = 0; i < nhostdevs; i++) { + if (l_hostdevs[i]->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + continue; + if (l_hostdevs[i]->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) + continue; + + libxl_device_usbdev_init(&x_usbdevs[j]); + + if (libxlMakeUSB(l_hostdevs[i], &x_usbdevs[j]) < 0) + goto error; + + nusbdevs++; + j++; + } + + VIR_SHRINK_N(x_usbdevs, nhostdevs, nhostdevs - nusbdevs); + d_config->usbdevs = x_usbdevs; + d_config->num_usbdevs = nusbdevs; + + return 0; + + error: + for (i = 0; i < nhostdevs; i++) + libxl_device_usbdev_dispose(&x_usbdevs[i]);
It looks like it is possible to call dispose on elements that did not have a corresponding init.
+ + VIR_FREE(x_usbdevs); + return -1; +} + + +int libxlMakePCI(virDomainHostdevDefPtr hostdev, libxl_device_pci *pcidev) { virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci; @@ -2078,6 +2146,9 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, if (libxlMakePCIList(def, d_config) < 0) return -1;
+ if (libxlMakeUSBList(def, d_config) < 0) + return -1; + /* * Now that any potential VFBs are defined, update the build info with * the data of the primary display. Some day libxl might implicitely do diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index 24e2911..e3ccca1 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -192,6 +192,9 @@ libxlMakeVfb(virPortAllocatorPtr graphicsports, int libxlMakePCI(virDomainHostdevDefPtr hostdev, libxl_device_pci *pcidev);
+int +libxlMakeUSB(virDomainHostdevDefPtr hostdev, libxl_device_usbdev *usbdev); + virDomainXMLOptionPtr libxlCreateXMLConf(void);
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 14a900c..4272dda 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -728,7 +728,7 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver, virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
virHostdevReAttachDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME, - vm->def, VIR_HOSTDEV_SP_PCI, NULL); + vm->def, VIR_HOSTDEV_SP_PCI | VIR_HOSTDEV_SP_USB, NULL);
Long line that could be split to fit within 80 columns.
VIR_FREE(priv->lockState); if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0) @@ -1103,7 +1103,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, goto cleanup_dom;
if (virHostdevPrepareDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME, - vm->def, VIR_HOSTDEV_SP_PCI) < 0) + vm->def, VIR_HOSTDEV_SP_PCI | VIR_HOSTDEV_SP_USB) < 0)
Same here, although this one isn't as easy to split.
goto cleanup_dom;
/* Unlock virDomainObj while creating the domain */ diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index bf97c9c..18a0891 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -383,7 +383,7 @@ libxlReconnectDomain(virDomainObjPtr vm,
/* Update hostdev state */ if (virHostdevUpdateActiveDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME, - vm->def, VIR_HOSTDEV_SP_PCI) < 0) + vm->def, VIR_HOSTDEV_SP_PCI | VIR_HOSTDEV_SP_USB) < 0)
Odd indentation for libvirt, but this too is harder to nicely split for 80 columns. Regards, Jim

On 5/13/2016 at 06:56 AM, in message <57350A36.3070707@suse.com>, Jim Fehlig <jfehlig@suse.com> wrote: Chunyan Liu wrote: Support creating guest with USB host device in config file. Currently libxl only supports xen PV guest, and only supports specifying USB host device by 'bus number' and 'device number'.
It would be nice to have an example of xl.cfg(5) usbdev= configuration and the corresponding domXML snippet. Actually, the example would be better placed in the commit message for the new patch needed for converting domXML <-> xl.cfg(5) USB config.
Signed-off-by: Chunyan Liu <cyliu@suse.com> --- src/libxl/libxl_conf.c | 71
++++++++++++++++++++++++++++++++++++++++++++++++
src/libxl/libxl_conf.h | 3 ++ src/libxl/libxl_domain.c | 4 +-- src/libxl/libxl_driver.c | 2 +- 4 files changed, 77 insertions(+), 3 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 30f2ce9..3b69cbf 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1848,6 +1848,74 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg, }
int +libxlMakeUSB(virDomainHostdevDefPtr hostdev, libxl_device_usbdev *usbdev) +{ + virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; + + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + return -1; + if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) + return -1; + + if (usbsrc->bus <= 0 || usbsrc->device <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("libxenlight supports only USB device " + "specified by busnum:devnum")); + return -1; + } + + usbdev->u.hostdev.hostbus = usbsrc->bus; + usbdev->u.hostdev.hostaddr = usbsrc->device; + + return 0; +} + +static int +libxlMakeUSBList(virDomainDefPtr def, libxl_domain_config *d_config) +{ + virDomainHostdevDefPtr *l_hostdevs = def->hostdevs; + size_t nhostdevs = def->nhostdevs; + size_t nusbdevs = 0; + libxl_device_usbdev *x_usbdevs; + size_t i, j; + + if (nhostdevs == 0) + return 0; + + if (VIR_ALLOC_N(x_usbdevs, nhostdevs) < 0) + return -1; + + for (i = 0, j = 0; i < nhostdevs; i++) { + if (l_hostdevs[i]->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + continue; + if (l_hostdevs[i]->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) + continue; + + libxl_device_usbdev_init(&x_usbdevs[j]); + + if (libxlMakeUSB(l_hostdevs[i], &x_usbdevs[j]) < 0) + goto error; + + nusbdevs++; + j++; + } + + VIR_SHRINK_N(x_usbdevs, nhostdevs, nhostdevs - nusbdevs); + d_config->usbdevs = x_usbdevs; + d_config->num_usbdevs = nusbdevs; + + return 0; + + error: + for (i = 0; i < nhostdevs; i++) + libxl_device_usbdev_dispose(&x_usbdevs[i]);
It looks like it is possible to call dispose on elements that did not have a corresponding init.
Oh, my fault, should be "i <nusbdevs".
+ + VIR_FREE(x_usbdevs); + return -1; +} + + +int libxlMakePCI(virDomainHostdevDefPtr hostdev, libxl_device_pci *pcidev) { virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci; @@ -2078,6 +2146,9 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, if (libxlMakePCIList(def, d_config) < 0) return -1;
+ if (libxlMakeUSBList(def, d_config) < 0) + return -1; + /* * Now that any potential VFBs are defined, update the build info with * the data of the primary display. Some day libxl might implicitely do diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index 24e2911..e3ccca1 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -192,6 +192,9 @@ libxlMakeVfb(virPortAllocatorPtr graphicsports, int libxlMakePCI(virDomainHostdevDefPtr hostdev, libxl_device_pci *pcidev);
+int +libxlMakeUSB(virDomainHostdevDefPtr hostdev, libxl_device_usbdev *usbdev); + virDomainXMLOptionPtr libxlCreateXMLConf(void);
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 14a900c..4272dda 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -728,7 +728,7 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver, virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
virHostdevReAttachDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME, - vm->def, VIR_HOSTDEV_SP_PCI, NULL); + vm->def, VIR_HOSTDEV_SP_PCI | VIR_HOSTDEV_SP_USB, NULL);
Long line that could be split to fit within 80 columns.
Yes, this and the following has problem to fit for both within 80 columns and correct indention. Change indention to let it fit within 80 columns?
VIR_FREE(priv->lockState); if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) <
0)
@@ -1103,7 +1103,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, goto cleanup_dom;
if (virHostdevPrepareDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME, - vm->def, VIR_HOSTDEV_SP_PCI) < 0) + vm->def, VIR_HOSTDEV_SP_PCI | VIR_HOSTDEV_SP_USB) < 0)
Same here, although this one isn't as easy to split.
goto cleanup_dom;
/* Unlock virDomainObj while creating the domain */ diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index bf97c9c..18a0891 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -383,7 +383,7 @@ libxlReconnectDomain(virDomainObjPtr vm,
/* Update hostdev state */ if (virHostdevUpdateActiveDomainDevices(hostdev_mgr,
LIBXL_DRIVER_NAME,
- vm->def, VIR_HOSTDEV_SP_PCI) < 0) + vm->def, VIR_HOSTDEV_SP_PCI | VIR_HOSTDEV_SP_USB) < 0)
Odd indentation for libvirt, but this too is harder to nicely split for 80 columns.
Regards, Jim

Support hot attach/detach a USB host device to guest. Curretnly libxl only supports xen PV guest, and only supports specifying USB host device by 'bus number' and 'device number'. Signed-off-by: Chunyan Liu <cyliu@suse.com> --- 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); + + 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: + 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)) 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"), + 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"), + 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"), + 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); + + 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); -- 2.1.4

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@suse.com> --- 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.
+ + 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.
+ 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.
+ 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? 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);

On 5/13/2016 at 07:20 AM, in message <57350FA5.3070307@suse.com>, Jim Fehlig <jfehlig@suse.com> 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@suse.com> --- 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);

Chunyan Liu wrote:
This patch series is to support pvusb in libxl driver.
Chunyan Liu (2): libxl: support creating guest with USB hostdev libxl: support hotplug USB host device
This series should also include support for converting domXML USB config to/from xl.cfg(5) in src/xenconfig/xen_xl.c, along with a test. FYI, the series fails to build with Xen < 4.7, where LIBXL_HAVE_PVUSB is not defined :-). Regards, Jim

On 05/12/2016 11:43 PM, Jim Fehlig wrote:
Chunyan Liu wrote:
This patch series is to support pvusb in libxl driver.
Chunyan Liu (2): libxl: support creating guest with USB hostdev libxl: support hotplug USB host device
This series should also include support for converting domXML USB config to/from xl.cfg(5) in src/xenconfig/xen_xl.c, along with a test.
FYI, the series fails to build with Xen < 4.7, where LIBXL_HAVE_PVUSB is not defined :-). Probably a nice follow-up after this series is in, would be to add the USB subsys type to domain capabilities (that you address in the OVMF series)
Joao
Regards, Jim

On 05/13/2016 07:14 AM, Joao Martins wrote:
On 05/12/2016 11:43 PM, Jim Fehlig wrote:
Chunyan Liu wrote:
This patch series is to support pvusb in libxl driver.
Chunyan Liu (2): libxl: support creating guest with USB hostdev libxl: support hotplug USB host device This series should also include support for converting domXML USB config to/from xl.cfg(5) in src/xenconfig/xen_xl.c, along with a test.
FYI, the series fails to build with Xen < 4.7, where LIBXL_HAVE_PVUSB is not defined :-). Probably a nice follow-up after this series is in, would be to add the USB subsys type to domain capabilities (that you address in the OVMF series)
Yes, good point. I'll add it to that series if this one makes it in first. Speaking of the OVMF series, I've fixed a few things after more testing and will rebase and resend a V2. Thanks in advance if either of you have time to look at it. Regards, Jim

On 05/13/2016 11:57 PM, Jim Fehlig wrote:
On 05/13/2016 07:14 AM, Joao Martins wrote:
On 05/12/2016 11:43 PM, Jim Fehlig wrote:
Chunyan Liu wrote:
This patch series is to support pvusb in libxl driver.
Chunyan Liu (2): libxl: support creating guest with USB hostdev libxl: support hotplug USB host device This series should also include support for converting domXML USB config to/from xl.cfg(5) in src/xenconfig/xen_xl.c, along with a test.
FYI, the series fails to build with Xen < 4.7, where LIBXL_HAVE_PVUSB is not defined :-). Probably a nice follow-up after this series is in, would be to add the USB subsys type to domain capabilities (that you address in the OVMF series)
Yes, good point. I'll add it to that series if this one makes it in first.
Speaking of the OVMF series, I've fixed a few things after more testing and will rebase and resend a V2. Thanks in advance if either of you have time to look at it. I will certainly have a look at it. I actually started but didn't wanted to comment without testing a bit first too - I will wait for the V2. I specially like the nice cleanup you make on the capabilities side plus getting domain capabilities implementated :)
Joao
participants (4)
-
Chun Yan Liu
-
Chunyan Liu
-
Jim Fehlig
-
Joao Martins