>> On 5/10/2014 at 06:18 AM, in message
<536D541D.5040107(a)suse.com>, Jim Fehlig
<jfehlig(a)suse.com> wrote:
Chunyan Liu wrote:
> Signed-off-by: Chunyan Liu <cyliu(a)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;
>
>