[libvirt] [PATCH] qemu: fix ethernet network type ip/route assign

Laine Stump laine at laine.org
Thu Aug 25 06:00:11 UTC 2016


On 08/24/2016 02:32 PM, Laine Stump wrote:
> On 08/24/2016 12:09 PM, Vasiliy Tolstov wrote:
>> IP and routes assigenment incorrectly placed on device stop.
>> This is fixing it, also change device state according to xml.
>> Note that as i know in linux routes can't be created on device that does
>> not up.
>>
>> Signed-off-by: Vasiliy Tolstov <v.tolstov at selfip.ru>
>> ---
>>   src/qemu/qemu_interface.c | 23 ++++++++++++++++++-----
>>   1 file changed, 18 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
>> index e637d21fb77a..f80feff2d545 100644
>> --- a/src/qemu/qemu_interface.c
>> +++ b/src/qemu/qemu_interface.c
>> @@ -108,8 +108,25 @@ qemuInterfaceStartDevice(virDomainNetDefPtr net)
>>           break;
>>       }
>>   -    case VIR_DOMAIN_NET_TYPE_USER:
>>       case VIR_DOMAIN_NET_TYPE_ETHERNET:
>> +        switch (dev->linkstate) {
>> +            case VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP:
>> +            case VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DEFAULT:
>> +                if ((ret = virNetDevSetOnline(dev->ifname, true)) < 0)
>> +                    goto cleanup;
>> +                break;
>> +
>> +            case VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN:
>> +                if ((ret = virNetDevSetOnline(dev->ifname, false)) < 0)
>> +                    goto cleanup;
>> +                break;
>> +        }
>
> The Online/Offline handling of the tap device for ethernet devices 
> should be identical to that used for network/bridge network devices. 
> If something is necessary (and it may be), it should be in a separate 
> patch.

More on this - aside from code that you added in your patch 9717d6 
"autocreate tap device for ethernet network type", the linkstate of a 
device is only used to send qemu commands controlling the linkstate of 
the *guest* device, but never to set the online status of the host side 
of the tap device (several of us missed this during the review of 
multiple versions of patches leading to 9717d6; heck, I didn't even 
realize it when looking at this new patch until I went back and read 
through all the code again)

This (plus a bit of investigation about when tap devices for bridge 
networks are set online) leads to the following:

1) The above code is erroneous. It shouldn't be there at all.

2) The code in qemuDomainChangeNetLinkState() that modifies the online 
status of the tap device when the interface is type='ethernet' also 
should not be there.

3) The reason the tap interface isn't IFF_UP is that 
virNetDevTapCreate() (called from qemuInterfaceEthernetConnect() in the 
same patch 9717d6) *ignores the VIR_NETDEV_TAP_CREATE_IFUP flag* (and it 
is documented this way). That's never been noticed before because the 
only previous call to virNetDevTapCreate() is from 
virNetDevTapCreateInBridgePort(), which sets IFF_UP on the tap device 
itself after calling virNetDevTapCreate().

My assumption is that it was done this way possibly because the tap 
device needed to be offline during some part of the initialization (or 
possibly because that's just the way some function was split up during a 
refactoring or something).

Anyway, I think there are 3 patches needed:

1) The part of your patch that just moves the virNetDevIPInfoAddToDev() 
call.

2) A patch to remove the virNetDevSetOnline() from 
qemuDomainChangeNetLinkState()

3) A patch to call virNetDevSetOnline() after calling virNetDevTapCreate()

I just modified your patch, created the other two patches, and sent the 
resulting patch series to the list.

If you can try it out and verify that it works for you (it works for 
me), then I'll push it in the morning ( US EST) so it's in before the 
freeze.




More information about the libvir-list mailing list