[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 05/19/2016 09:14 AM, 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',
> for example:
>     <hostdev mode='subsystem' type='usb' managed='no'>
>       <source>
>         <address bus='1' device='3'/>
>       </source>
>     </hostdev>
> 
> Signed-off-by: Chunyan Liu <cyliu suse com>
There was only a nitpick that I spotted but other than that looks good to me:

Reviewed-by: Joao Martins <joao m martins oracle com>

> ---
> Changes:
>   * add LIBXL_HAVE_PVUSB check
>   * address Jim's comments
> 
>  src/libxl/libxl_conf.c   | 74 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/libxl/libxl_conf.h   |  5 ++++
>  src/libxl/libxl_domain.c | 16 +++++++++--
>  src/libxl/libxl_driver.c |  8 +++++-
>  4 files changed, 100 insertions(+), 3 deletions(-)
> 
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index c399f5c..c6c76c4 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -1861,6 +1861,75 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg,
>  
>  }
>  
> +#ifdef LIBXL_HAVE_PVUSB
> +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 < nusbdevs; i++)
> +        libxl_device_usbdev_dispose(&x_usbdevs[i]);
> +
> +    VIR_FREE(x_usbdevs);
> +    return -1;
> +}
> +#endif
> +
>  int
>  libxlMakePCI(virDomainHostdevDefPtr hostdev, libxl_device_pci *pcidev)
>  {
> @@ -2092,6 +2161,11 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
>      if (libxlMakePCIList(def, d_config) < 0)
>          return -1;
>  
> +#ifdef LIBXL_HAVE_PVUSB
> +    if (libxlMakeUSBList(def, d_config) < 0)
> +        return -1;
> +#endif
> +
>      /*
>       * 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 c5b9429..df318f4 100644
> --- a/src/libxl/libxl_conf.h
> +++ b/src/libxl/libxl_conf.h
> @@ -196,6 +196,11 @@ libxlMakeVfb(virPortAllocatorPtr graphicsports,
>  int
>  libxlMakePCI(virDomainHostdevDefPtr hostdev, libxl_device_pci *pcidev);
>  
> +#ifdef LIBXL_HAVE_PVUSB
> +int
> +libxlMakeUSB(virDomainHostdevDefPtr hostdev, libxl_device_usbdev *usbdev);
> +#endif
> +
The #ifdef and #endif aren't indented properly. In this case the code style requires
it to be "# ifdef" and "# endif" otherwise make syntax-check will fail.

>  virDomainXMLOptionPtr
>  libxlCreateXMLConf(void);
>  
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index 5fa1bd9..3dda34d 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -726,9 +726,15 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver,
>      int vnc_port;
>      char *file;
>      virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
> +    unsigned int hostdev_flags;
> +
> +    hostdev_flags = VIR_HOSTDEV_SP_PCI;
> +#ifdef LIBXL_HAVE_PVUSB
> +    hostdev_flags |= VIR_HOSTDEV_SP_USB;
> +#endif
Just one suggestion: this chunk (and the ones after) could probably be made simpler
by having hostdev_flags initialized beforehand with VIR_HOSTDEV_SP_PCI instead of
having in separate:

    unsigned int hostdev_flags = VIR_HOSTDEV_SP_PCI;

#ifdef LIBXL_HAVE_PVUSB
    hostdev_flags |= VIR_HOSTDEV_SP_USB;
#endif

>  
>      virHostdevReAttachDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
> -                                    vm->def, VIR_HOSTDEV_SP_PCI, NULL);
> +                                    vm->def, hostdev_flags, NULL);
>  
>      VIR_FREE(priv->lockState);
>      if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0)
> @@ -1038,6 +1044,12 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
>      virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
>      libxl_asyncprogress_how aop_console_how;
>      libxl_domain_restore_params params;
> +    unsigned int hostdev_flags;
> +
> +    hostdev_flags = VIR_HOSTDEV_SP_PCI;
> +#ifdef LIBXL_HAVE_PVUSB
> +    hostdev_flags |= VIR_HOSTDEV_SP_USB;
> +#endif
>  
>      libxl_domain_config_init(&d_config);
>  
> @@ -1114,7 +1126,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
>          goto cleanup_dom;
>  
>      if (virHostdevPrepareDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
> -                                       vm->def, VIR_HOSTDEV_SP_PCI) < 0)
> +                                       vm->def, hostdev_flags) < 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 2c19ddb..960673f 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -352,6 +352,12 @@ libxlReconnectDomain(virDomainObjPtr vm,
>      int len;
>      uint8_t *data = NULL;
>      virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
> +    unsigned int hostdev_flags;
> +
> +    hostdev_flags = VIR_HOSTDEV_SP_PCI;
> +#ifdef LIBXL_HAVE_PVUSB
> +    hostdev_flags |= VIR_HOSTDEV_SP_USB;
> +#endif
>  
>      virObjectLock(vm);
>  
> @@ -379,7 +385,7 @@ libxlReconnectDomain(virDomainObjPtr vm,
>  
>      /* Update hostdev state */
>      if (virHostdevUpdateActiveDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
> -                                            vm->def, VIR_HOSTDEV_SP_PCI) < 0)
> +                                            vm->def, hostdev_flags) < 0)
>          goto out;
>  
>      if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback)
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]