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

[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 at 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