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

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

 



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!

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]