Re: [libvirt] [PATCH v2] libvirt: xen: do not use ioemu type for any emulated NIC

On 04/13/2012 09:14 AM, Stefan Bader wrote: >> I think it would be better if we just centralized this logic, as in, >> only set that (type ioemu) bit in conditional rather than 2. Should >> be pretty straightforward. > > Did you have something like below in mind? > > -Stefan > > >From a0e214fa6dea9bd66616f37246067a2c631f4184 Mon Sep 17 00:00:00 2001 > From: Stefan Bader <stefan.bader(a)canonical.com&gt; > Date: Thu, 12 Apr 2012 15:32:41 +0200 > Subject: [PATCH] libvirt: xen: do not use ioemu type for any emulated NIC > > When using the xm/xend stack to manage instances there is a bug > that causes the emulated interfaces to be unusable when the vif > config contains type=ioemu. > > The current code already has a special quirk to not use this > keyword if no specific model is given for the emulated NIC > (defaulting to rtl8139). > Essentially it works because regardless of the type argument,i > the Xen stack always creates emulated and paravirt interfaces and > lets the guest decide which one to use. So neither xl nor xm stack > actually require the type keyword for emulated NICs. > > [v2: move code around to have the exception in one case together] > Signed-off-by: Stefan Bader <stefan.bader(a)canonical.com&gt; > --- > src/xenxs/xen_sxpr.c | 28 +++++++++++++++------------- > src/xenxs/xen_xm.c | 27 ++++++++++++++------------- > 2 files changed, 29 insertions(+), 26 deletions(-) > > diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c > index e1bbd76..3a56345 100644 > --- a/src/xenxs/xen_sxpr.c > +++ b/src/xenxs/xen_sxpr.c > @@ -1999,20 +1999,22 @@ xenFormatSxprNet(virConnectPtr conn, > if (def->model != NULL) > virBufferEscapeSexpr(buf, "(model '%s')", def->model); > } > - else if (def->model == NULL) { > - /* > - * apparently (type ioemu) breaks paravirt drivers on HVM so skip > - * this from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU > - */ > - if (xendConfigVersion <= XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) > - virBufferAddLit(buf, "(type ioemu)"); > - } > - else if (STREQ(def->model, "netfront")) { > - virBufferAddLit(buf, "(type netfront)"); > - } > else { > - virBufferEscapeSexpr(buf, "(model '%s')", def->model); > - virBufferAddLit(buf, "(type ioemu)"); > + if (def->model != NULL && STREQ(def->model, "netfront")) { > + virBufferAddLit(buf, "(type netfront)"); > + } > + else { > + if (def->model != NULL) { > + virBufferEscapeSexpr(buf, "(model '%s')", def->model); > + } > + /* > + * apparently (type ioemu) breaks paravirt drivers on HVM so skip > + * this from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU > + */ > + if (xendConfigVersion <= XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) { > + virBufferAddLit(buf, "(type ioemu)"); > + } > + } > } > > if (!isAttach) > diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c > index d65e97a..93a26f9 100644 > --- a/src/xenxs/xen_xm.c > +++ b/src/xenxs/xen_xm.c > @@ -1381,20 +1381,21 @@ static int xenFormatXMNet(virConnectPtr conn, > if (net->model != NULL) > virBufferAsprintf(&buf, ",model=%s", net->model); > } > - else if (net->model == NULL) { > - /* > - * apparently type ioemu breaks paravirt drivers on HVM so skip this > - * from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU > - */ > - if (xendConfigVersion <= XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) > - virBufferAddLit(&buf, ",type=ioemu"); > - } > - else if (STREQ(net->model, "netfront")) { > - virBufferAddLit(&buf, ",type=netfront"); > - } > else { > - virBufferAsprintf(&buf, ",model=%s", net->model); > - virBufferAddLit(&buf, ",type=ioemu"); > + if (net->model != NULL && STREQ(net->model, "netfront")) { > + virBufferAddLit(&buf, ",type=netfront"); > + } > + else { > + if (net->model != NULL) > + virBufferAsprintf(&buf, ",model=%s", net->model); > + > + /* > + * apparently type ioemu breaks paravirt drivers on HVM so skip this > + * from XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU > + */ > + if (xendConfigVersion <= XEND_CONFIG_MAX_VERS_NET_TYPE_IOEMU) > + virBufferAddLit(&buf, ",type=ioemu"); > + } > } > > if (net->ifname) Looks good. Did you verify 'make check' passes as well? If so, ACK

- Cole