On Thu, Feb 26, 2015 at 15:17:38 +0100, Michal Privoznik wrote:
> Now that we have fine grained locks, there's no need to lock the
> whole driver. We can rely on self-locking APIs.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/network/bridge_driver.c | 45 +++------------------------------------------
> 1 file changed, 3 insertions(+), 42 deletions(-)
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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list