Re: [libvirt] [PATCH v1 29/31] bridge_driver: Drop networkDriverLock() from almost everywhere

On Thu, Feb 26, 2015 at 15:17:38 +0100, Michal Privoznik wrote: This patch allows the following race: Threads 1 and 2 want to define a network with same name. Both threads call into: virNetworkObjPtr virNetworkAssignDef(virNetworkObjListPtr nets, virNetworkDefPtr def, bool live) { virNetworkObjPtr network; // both threads are here now if ((network = virNetworkObjFindByName(nets, def->name))) { // they both serialize on the virNetworkObjFindByName, but neither of // those finds the object before the other one manages to add it virNetworkObjAssignDef(network, def, live); return network; } if (!(network = virNetworkObjNew())) // now both threads allocate the object ... return NULL; virObjectLock(network); virObjectLock(nets); // and serialize here again, but both threads think now that they // shall add the network to the list ... if (VIR_APPEND_ELEMENT_COPY(nets->objs, nets->count, network) < 0) .. and do so. goto error; virNetworkAssignDef() will need more love. I think that virNetworkObjFindByName and friends should not lock or ref the network object and the callers such as virNetworkAssignDef() or networkObjFromNetwork() should do it. In that way, you'll be able to lock the list before and TEST_AND_SET the object in an atomic fashion. Rest of this patch looks okay. Peter