>>> On 5/10/2014 at 06:18 AM, in message <536D541D.5040107@xxxxxxxx>, Jim Fehlig
<jfehlig@xxxxxxxx> wrote:
> Chunyan Liu wrote:
> > Signed-off-by: Chunyan Liu <cyliu@xxxxxxxx>
> >
>
> 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;
> >
> >
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list