[libvirt] [PATCH] libxl: Fix initialization of nictype in libxl_device_nic
As pointed out by the Xen folks [1], HVM nics should always be set to type LIBXL_NIC_TYPE_VIF_IOEMU unless the user explicity requests LIBXL_NIC_TYPE_VIF via model='netfront'. The current logic in libxlMakeNic() only sets the nictype to LIBXL_NIC_TYPE_VIF_IOEMU if a model is specified that is not 'netfront', which breaks PXE booting configurations where no model is specified (i.e. use the hypervisor default). Reported-by: Stefan Bader <stefan.bader@canonical.com> [1] https://www.redhat.com/archives/libvir-list/2013-December/msg01156.html Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- I toyed with detecting whether to use an IOEMU nic in libxlMakeNicList() and passing the bool to libxlMakeNic(), but in the end left the detection to libxlMakeNic(). src/libxl/libxl_conf.c | 20 ++++++++++++++------ src/libxl/libxl_conf.h | 4 +++- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index aaeb00e..04d01af 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -855,8 +855,12 @@ error: } int -libxlMakeNic(virDomainNetDefPtr l_nic, libxl_device_nic *x_nic) +libxlMakeNic(virDomainDefPtr def, + virDomainNetDefPtr l_nic, + libxl_device_nic *x_nic) { + bool ioemu_nic = STREQ(def->os.type, "hvm"); + /* TODO: Where is mtu stored? * * x_nics[i].mtu = 1492; @@ -866,12 +870,16 @@ libxlMakeNic(virDomainNetDefPtr l_nic, libxl_device_nic *x_nic) virMacAddrGetRaw(&l_nic->mac, x_nic->mac); - if (l_nic->model && !STREQ(l_nic->model, "netfront")) { - if (VIR_STRDUP(x_nic->model, l_nic->model) < 0) - return -1; + if (ioemu_nic) x_nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU; - } else { + else x_nic->nictype = LIBXL_NIC_TYPE_VIF; + + if (l_nic->model) { + if (VIR_STRDUP(x_nic->model, l_nic->model) < 0) + return -1; + if (STREQ(l_nic->model, "netfront")) + x_nic->nictype = LIBXL_NIC_TYPE_VIF; } if (VIR_STRDUP(x_nic->ifname, l_nic->ifname) < 0) @@ -908,7 +916,7 @@ libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config) return -1; for (i = 0; i < nnics; i++) { - if (libxlMakeNic(l_nics[i], &x_nics[i])) + if (libxlMakeNic(def, l_nics[i], &x_nics[i])) goto error; } diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index ffa93bd..90d590f 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -142,7 +142,9 @@ libxlMakeCapabilities(libxl_ctx *ctx); int libxlMakeDisk(virDomainDiskDefPtr l_dev, libxl_device_disk *x_dev); int -libxlMakeNic(virDomainNetDefPtr l_nic, libxl_device_nic *x_nic); +libxlMakeNic(virDomainDefPtr def, + virDomainNetDefPtr l_nic, + libxl_device_nic *x_nic); int libxlMakeVfb(libxlDriverPrivatePtr driver, virDomainGraphicsDefPtr l_vfb, libxl_device_vfb *x_vfb); -- 1.8.1.4
On 01/06/2014 11:59 AM, Jim Fehlig wrote:
As pointed out by the Xen folks [1], HVM nics should always be set to type LIBXL_NIC_TYPE_VIF_IOEMU unless the user explicity requests LIBXL_NIC_TYPE_VIF via model='netfront'. The current logic in libxlMakeNic() only sets the nictype to LIBXL_NIC_TYPE_VIF_IOEMU if a model is specified that is not 'netfront', which breaks PXE booting configurations where no model is specified (i.e. use the hypervisor default).
Reported-by: Stefan Bader <stefan.bader@canonical.com>
[1] https://www.redhat.com/archives/libvir-list/2013-December/msg01156.html
Signed-off-by: Jim Fehlig <jfehlig@suse.com> ---
I toyed with detecting whether to use an IOEMU nic in libxlMakeNicList() and passing the bool to libxlMakeNic(), but in the end left the detection to libxlMakeNic().
Seems okay to me with the approach you took.
src/libxl/libxl_conf.c | 20 ++++++++++++++------ src/libxl/libxl_conf.h | 4 +++- 2 files changed, 17 insertions(+), 7 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Eric Blake wrote:
On 01/06/2014 11:59 AM, Jim Fehlig wrote:
As pointed out by the Xen folks [1], HVM nics should always be set to type LIBXL_NIC_TYPE_VIF_IOEMU unless the user explicity requests LIBXL_NIC_TYPE_VIF via model='netfront'. The current logic in libxlMakeNic() only sets the nictype to LIBXL_NIC_TYPE_VIF_IOEMU if a model is specified that is not 'netfront', which breaks PXE booting configurations where no model is specified (i.e. use the hypervisor default).
Reported-by: Stefan Bader <stefan.bader@canonical.com>
[1] https://www.redhat.com/archives/libvir-list/2013-December/msg01156.html
Signed-off-by: Jim Fehlig <jfehlig@suse.com> ---
I toyed with detecting whether to use an IOEMU nic in libxlMakeNicList() and passing the bool to libxlMakeNic(), but in the end left the detection to libxlMakeNic().
Seems okay to me with the approach you took.
src/libxl/libxl_conf.c | 20 ++++++++++++++------ src/libxl/libxl_conf.h | 4 +++- 2 files changed, 17 insertions(+), 7 deletions(-)
ACK.
Thanks, pushed. Regards, Jim
On Mon, 2014-01-06 at 11:59 -0700, Jim Fehlig wrote:
+ bool ioemu_nic = STREQ(def->os.type, "hvm"); [...] - if (l_nic->model && !STREQ(l_nic->model, "netfront")) { - if (VIR_STRDUP(x_nic->model, l_nic->model) < 0) - return -1; + if (ioemu_nic) x_nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU; - } else { + else x_nic->nictype = LIBXL_NIC_TYPE_VIF;
It's up to you but you could just leave nictype set to the default (as initialised by libxl_device_nic_init and let the library do the right thing based on the guest type.
+ + if (l_nic->model) { + if (VIR_STRDUP(x_nic->model, l_nic->model) < 0) + return -1; + if (STREQ(l_nic->model, "netfront")) + x_nic->nictype = LIBXL_NIC_TYPE_VIF;
This bit would remain valid whether or not you did the above. Ultimately up to you whether you want to precisely control things or just follow the defaults. Ian.
participants (3)
-
Eric Blake -
Ian Campbell -
Jim Fehlig