Chun Yan Liu wrote:

...

...

...

On 5/10/2014 at 06:18 AM, in message <536D541D.5040107@suse.com>, Jim Fehlig

<jfehlig@suse.com> wrote:

...

Chunyan Liu wrote:

...

Signed-off-by: Chunyan Liu <cyliu@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.

Yep, understood.

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

I fixed up the commit message and pushed the patch. Thanks Chunyan! Regards, Jim