[libvirt] [PATCH] network: get vlan info for Open vSwitch interfaces from proper source

[Kyle Mestery (kmestery)]

On Aug 29, 2012, at 9:49 AM, Kyle Mestery (kmestery) wrote:
> On Aug 29, 2012, at 6:19 AM, Alex Jia wrote:
>> On 08/29/2012 06:43 PM, Laine Stump wrote:
>>> This bug was revealed by the crash described in
>>> 
>>>  https://bugzilla.redhat.com/show_bug.cgi?id=852383
>>> 
>>> The vlan info pointer sent to virNetDevOpenvswitchAddPort should never
>>> be non-NULL unless there is at least one tag. The factthat such a vlan
>>> info pointer was receveid pointed out that a caller was passing the
>>> wrong pointer. Instead of sending&net->vlan, the result of
>>> virDomainNetGetActualVlan(net) should be sent - that function will
>>> look for vlan info in net->data.network.actual->vlan, and in cany case
>>> return NULL instead of a pointer if the vlan info it finds has no
>>> tags.
>>> 
>>> Aside from causing the crash, sending a hardcoded&net->vlan has the
>>> effect of ignoring vlan info from a<network>  or<portgroup>  config.
>>> ---
>>> 
>>> Since I'm not online in a regular fashion for the next few days (too
>>> bad I wasn't online in the 12 hours or so *before* the 0.10.0 release
>>> instead of after :-/), I would appreciate if whoever ACKs this could
>>> push it. Thanks!
>> 
>> Laine, unfortunately, the libvirtd still is crash without my patch after applying your patch :(
>> 
> Hi Alex and Laine:
> 
> What I notice is, that with Laine's patch, for virtualport of type openvswitch, if I do NOT define
> a VLAN, it crashes. If I do, then I can start my virtual machines. Without Laine's patch, it's the
> same, which leads me to believe Laine's patch only fixes a symptom of this issue.
> 
> I'm looking into this now, will see how far I get with it today. But it appears to me to be a bug
> in the logic which is handling VLANs at a higher layer.
> 
> Thanks,
> Kyle
> 
I just sent a patch, which combined with Laine's patches, stop the crashes in the latest libvirt code
when using Open vSwitch virtual ports. I think this should go into the 0.10.1 release for this
Friday, because without it libvirt crashes consistently with Open vSwitch virtualports.

Thanks,
Kyle

>>> src/qemu/qemu_command.c | 2 +-
>>> src/uml/uml_conf.c      | 2 +-
>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>>> index 8c32a4d..25f2451 100644
>>> --- a/src/qemu/qemu_command.c
>>> +++ b/src/qemu/qemu_command.c
>>> @@ -258,7 +258,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
>>>     err = virNetDevTapCreateInBridgePort(brname,&net->ifname,&net->mac,
>>>                                          def->uuid,&tapfd,
>>>                                          virDomainNetGetActualVirtPortProfile(net),
>>> -&net->vlan,
>>> +                                         virDomainNetGetActualVlan(net),
>>>                                          tap_create_flags);
>>>     virDomainAuditNetDevice(def, net, "/dev/net/tun", tapfd>= 0);
>>>     if (err<  0) {
>>> diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
>>> index 5461b42..410f3e2 100644
>>> --- a/src/uml/uml_conf.c
>>> +++ b/src/uml/uml_conf.c
>>> @@ -141,7 +141,7 @@ umlConnectTapDevice(virConnectPtr conn,
>>>     if (virNetDevTapCreateInBridgePort(bridge,&net->ifname,&net->mac,
>>>                                        vm->uuid, NULL,
>>>                                        virDomainNetGetActualVirtPortProfile(net),
>>> -&net->vlan,
>>> +                                       virDomainNetGetActualVlan(net),
>>>                                        VIR_NETDEV_TAP_CREATE_IFUP)<  0) {
>>>         if (template_ifname)
>>>             VIR_FREE(net->ifname);