On 03.03.2015 16:26, Peter Krempa wrote:
> 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.
D'oh! You're right. This has slipped my mind. So, I'm pushing first 12
patches (yep, some of them were not explicitly ACKed, but 01/31 is
trivial enough to be fixed without v2 :-P), and will respin the rest.
Thanks for the review!
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list