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@xxxxxxxx>
There was only a nitpick that I spotted but other than that looks good to me:
Reviewed-by: Joao Martins <joao.m.martins@xxxxxxxxxx>
> ---
> 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)
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list