[libvirt] [PATCH] virNetDevVethCreate: assign names based on mac address by default

[Gao feng]

On 08/29/2013 04:52 PM, Oskari Saarenmaa wrote:
> 29.08.2013 11:36, Gao feng kirjoitti:
>> On 08/29/2013 04:20 PM, Oskari Saarenmaa wrote:
>>> On Thu, Aug 29, 2013 at 10:10:36AM +0200, Michal Privoznik wrote:
>>>> On 28.08.2013 23:05, Oskari Saarenmaa wrote:
>>>>> Interface names do not have to be numerical (or veth + number) and trying to
>>>>> assign them to that format is susceptible to race conditions.  Instead,
>>>>> assign the parent interface name according to the mac address (the last
>>>>> three bytes) if no name was given by the caller and use the parent interface
>>>>> name plus 'p' for the container name.
>>>> I don't think it is a good idea. What it user defines MAC addresses in a
>>>> way that the last three bytes are the same? E.g.
>>>>
>>>> 00:11:22:33:44:55
>>>> 00:22:11:33:44:55
>>>>
>>>> (there are plenty of possibilities). With your code we would create
>>>> veth334455 for the first domain and there's nothing left for the other
>>>> one but eyes to cry.
>>>
>>> Sure, it's possible for the user to assign addresses like this, but I
>>> believe most mac addresses are assigned randomly, that's what libvirt does
>>> by default, no?  Also, it's possible for the user to override the interface
>>> name if they want to.  Currently it's only possible to set the first
>>> interface name, but there's no way to set the name of the second interface
>>> which causes failures because of races.  In any case it makes sense to set
>>> the second interface's name based on the first interface's name so we don't
>>> have to try to come up with two different unique names.
>>>
>>>> Moreover, there's no race now, as we use global lock to mutually exclude
>>>> call to virNetDevVethCreate. Although, I must admit it's only within a
>>>> single driver, I think. So if there are two domains starting (one in
>>>> qemu driver the other one in lxc, for instance) there can be race. But
>>>> this should be solved in a different way then. The virNetDevVethCreate
>>>> should be turned into virObjectLockable and a veth should be allocated
>>>> by calling a method.
>>>
>>> I ran into this issue repeatedly when I tried to start multiple LXC domains
>>> simultaneously from my application (https://github.com/melor/poni) which
>>> runs libvirt operations in their own threads.
>>>
>>
>> It's easy to be resolved. give the parentVeth and containerVeth a name by default.
> 
> At the moment virLXCProcessSetupInterfaceBridged always passes a NULL
> for the container interface name and while it would be possible to make
> it configurable like the parent interface name, I believe it makes more
> sense to just do the right thing by default.
> 

Agree.

> The suggested patch also greatly simplifies name selection by removing
> the loops trying to find a supposedly unused interface name.  If you
> don't like using mac address in the interface name we could just replace
> it with a random string with a loop checking that the selected name
> wasn't in use when it was selected.  The current approach of trying to
> use a name with a low number just doesn't work in an environment where
> another process does the same thing.
> 

Use mac address to create an unique name is not a good idea,
I suggest you pass an index to virLXCProcessSetupInterfaceBridged.
use this index and domain_name to create an unique name.

Such as, for the first net interface for domain "fedora19"
parentVeth's name is "fedora19-host-1", containerVeth is "fedora19-container-1"
for the second interface,
parentVeth's name is "fedora19-host-2", containerVeth is "fedora19-container-2"
...

find out a proper name for these veth device :)