[libvirt] [PATCH 4/4] util: use netlink to create bridge devices

[Laine Stump]

On 03/24/2015 06:39 PM, John Ferlan wrote:
>
> On 03/23/2015 03:43 PM, Laine Stump wrote:
>> Just as it is possible to delete a bridge device with the netlink
>> RTM_DELLINK message, one can be created with the RTM_NEWLINK
>> message. Because of differences in the format of the message, it's not
>> as straightforward as with virNetlinkDelLink() to create a single
>> utility function that can be used to create any type of interface, so
>> the new netlink version of virNetDevBridgeCreate() does its own
>> construction of the netlink message and calls virNetlinkCommand()
>> itself.
>>
>> This doesn't provide any extra functionality, just provides symmetry
>> with the previous commit.
>>
>> NB: We *could* alter the API of virNetDevBridgeCreate() to take a MAC
>> address, and directly program that mac address into the bridge (by
>> adding an IFLA_ADDRESS attribute, as is done in
>> virNetDevMacVLanCreate()) rather than separately creating the "dummy
>> tap" (e.g. virbr0-nic) to maintain a fixed mac address on the bridge,
>> but the commit history of virnetdevbridge.c shows that the presence of
>> this dummy tap is essential in some older versions of the kernel
>> (between 2.6.39 and 3.1 or 3.2, possibly?) to proper operation of IPv6
>> DAD, and I don't want to take the chance of breaking something that I
>> don't have the time/setup to test (my RHEL6 box is at kernel
>> 2.6.32-544, and the next lowest kernel I have is 3.17)
>> ---
>>  src/util/virnetdevbridge.c | 78 +++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 77 insertions(+), 1 deletion(-)
>>
> What's here seems reasonable - I don't have "history" on my side to
> answer the older versions of the kernel query...
>
> One other difference between this and virNetDevMacVLanCreate is the
> error path that handles *retry if the name is already in use...  

Hmm. Interesting idea, but it wouldn't be useful with the current way
that things work - unlike macvtap devices, which are created one per
domain-interface and whose name we don't care about, by the time we get
to the point that we want to create a bridge device (which is one for
each network), we've already committed to the name we're going to use.

Some restructuring of network startup could make that useful though.
Still, I think I'll push this as it is for now (since it meets the
current usage demands, and maintains the existing API for
virNetDevBridgeCreate()) and think about building in EEXIST handling
later (this could be especially problematic, since the same capability
would need to be put into the other two implementations of
virNetDevBridgeCreate(), and one of those is for a platform that I don't
have setup to test on).

Thanks for the reviews!