Re: [libvirt] [PATCH 1/2] libxl: support syntax <interface type="hostdev">
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!
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;