Re: [libvirt] [PATCH 1/2] libxl: support creating guest with USB hostdev

>> On 5/13/2016 at 06:56 AM, in message <57350A36.3070707(a)suse.com&gt;, Jim Fehlig

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(a)suse.com&gt; > --- > 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