[libvirt] PATCH: 14/28: Thread safety for network driver

[Jim Meyering]

Daniel Veillard <veillard at redhat.com> wrote:

> On Wed, Dec 03, 2008 at 09:44:50AM +0000, Daniel P. Berrange wrote:
>> On Wed, Dec 03, 2008 at 10:32:58AM +0100, Daniel Veillard wrote:
>> > On Sun, Nov 30, 2008 at 11:55:51PM +0000, Daniel P. Berrange wrote:
>> > > This patch makes the network driver thread safe, following the pattern of
>> > > the earlier patches
>> >
>> > > @@ -95,6 +107,7 @@ networkAutostartConfigs(struct network_d
>> > >      unsigned int i;
>> >
>> >   Hum, you're not locking the driver itself during the loop ?
>> > If a concurrent call were to remove one of the network object in
>> > the meantime, that would lead to potentially random memory accesses
>> > no ?
>>
>> There are only two callers of this method, and they both already hold the
>> driver lock - networkStartup() and networkReload().
>
>   I realized that when digging more in the storage case... see next mail
>   :-)
>
>> > >  cleanup:
>> > > +    if (network)
>> > > +        virNetworkObjUnlock(network);
>> >
>> >   like for domain, I would suggest the unlocking accessor to accept NULL
>> > and drop all the if constructs
>>
>> I've thought about doing that, but wasn't really sure whether it was good
>> idea or not. I thought it might hide bugs, but on balance it would certainly
>> simplify alot of code. If people generally think its worth allowing NULL
>> in the Unlock routine, i'll make it so.
>
>   well we did a similar change for freeing ...

And if you do that, you can automatically detect any unnecessary
if-before-unlock tests by adding virNetworkObjUnlock to the
useless_free_options list in Makefile.cfg ;-)