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

[Stefan Bader]

On 16.04.2012 13:58, Cole Robinson wrote:
> 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 at canonical.com>
>> 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 at canonical.com>
>> ---
>>  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

No :( And it fails.

TEST: libvirtdconftest
      .....!!!!!!...!!!!!!!!!!!!!!!!!!!!!!!    37  FAIL
FAIL: libvirtdconftest

I guess this is expected as the change really does modify the generated configs.
So I need to find out how to make it expect that...


> 
> - Cole


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 900 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120416/54d3478d/attachment-0001.sig>