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.