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

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




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



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