[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

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.

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.

/ Oskari

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]