[libvirt] [PATCH 1/2] libxl: support syntax <interface type="hostdev">

[Chun Yan Liu]

>>> On 5/10/2014 at 06:18 AM, in message <536D541D.5040107 at suse.com>, Jim Fehlig
<jfehlig at suse.com> wrote: 
> Chunyan Liu wrote: 
> > Signed-off-by: Chunyan Liu <cyliu at suse.com> 
> >    
>  
> A while back when testing Chunyan's "common hostdev library" series, I 
> mentioned that <interface type='hostdev'> was not working with the libxl 
> driver.  Chunyan later privately sent a "v1" of this patch for testing 
> in my setup.  In addition to testing, I provided some private comments.  
> I see those have been incorporated in this patch and functionally it 
> looks good, but I do have one additional question about the commit 
> message... 
>  
> > --- 
> >  src/libxl/libxl_conf.c | 16 +++++++++++----- 
> >  1 file changed, 11 insertions(+), 5 deletions(-) 
> > 
> > diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c 
> > index 298c8a1..b7fed7f 100644 
> > --- a/src/libxl/libxl_conf.c 
> > +++ b/src/libxl/libxl_conf.c 
> > @@ -921,25 +921,31 @@ static int 
> >  libxlMakeNicList(virDomainDefPtr def,  libxl_domain_config *d_config) 
> >  { 
> >      virDomainNetDefPtr *l_nics = def->nets; 
> > -    int nnics = def->nnets; 
> > +    size_t nnics = def->nnets; 
> >      libxl_device_nic *x_nics; 
> > -    size_t i; 
> > +    size_t i, nvnics = 0; 
> >   
> >      if (VIR_ALLOC_N(x_nics, nnics) < 0) 
> >          return -1; 
> >   
> >      for (i = 0; i < nnics; i++) { 
> > -        if (libxlMakeNic(def, l_nics[i], &x_nics[i])) 
> > +        if (l_nics[i]->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) 
> > +            continue; 
> >    
>  
> After looking at this again, it seems we are really *fixing* <interface 
> type='hostdev'>.  The driver already supports creating the hostdev 
> device, but without this patch a libxl_device_nic is created too.  Is 
> that a fair statement?  If so, the commit message should be changed to 
> reflect this.  Thanks! 

A NET_TYPE_HOSTDEV device is really a hostdev device, the driver will create
a hostdev device for it, so no need to create a libxl_device_nic again. Before 
this patch, it tried to call libxlMakeNic to create a libxl_device_nic for it but
failed since NET_TYPE_HOSTDEV is not supported there.

I'll add this to commit message. Is that OK?

- Chunyan

>  
> Regards, 
> Jim 
>  
> > + 
> > +        if (libxlMakeNic(def, l_nics[i], &x_nics[nvnics])) 
> >              goto error; 
> >          /* 
> >           * The devid (at least right now) will not get initialized by 
> >           * libxl in the setup case but is required for starting the 
> >           * device-model. 
> >           */ 
> > -        if (x_nics[i].devid < 0) 
> > -            x_nics[i].devid = i; 
> > +        if (x_nics[nvnics].devid < 0) 
> > +            x_nics[nvnics].devid = nvnics; 
> > + 
> > +        nvnics++; 
> >      } 
> >   
> > +    VIR_SHRINK_N(x_nics, nnics, nnics - nvnics); 
> >      d_config->nics = x_nics; 
> >      d_config->num_nics = nnics; 
> >   
> >    
>