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

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

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]