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

[Cole Robinson]

On 04/16/2012 08:36 AM, Stefan Bader wrote:
> 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...
> 

Make sure it's actually your patch causing those issues. I wouldn't expect
your change to affect that particular test.

The first section of HACKING has some info about things to run before
submitting a patch, and ways to debug test failures.

Thanks,
Cole