[libvirt PATCH 6/7] virMutex*: Warn on error

[Daniel P. Berrangé]

On Thu, Aug 05, 2021 at 05:53:15PM +0200, Peter Krempa wrote:
> On Thu, Aug 05, 2021 at 16:48:59 +0100, Daniel P. Berrangé wrote:
> > On Thu, Aug 05, 2021 at 05:44:15PM +0200, Peter Krempa wrote:
> > > On Thu, Aug 05, 2021 at 16:33:57 +0100, Daniel P. Berrangé wrote:
> > > > On Thu, Aug 05, 2021 at 05:29:24PM +0200, Peter Krempa wrote:
> 
> [...]
> 
> > Lock state tracking doesn't sound very appealing to me.
> > 
> > QEMU makes use of g_auto for unlocking mutexes within scopes
> > 
> >     WITH_QEMU_LOCK_GUARD(&mutex) {
> >         ...
> >         if (error) {
> >             return; <-- mutex is automatically unlocked
> >         }
> >  
> >         if (early_exit) {
> >             break;  <-- leave this scope early, unlocking
> >         }
> >         ...
> >     }
> > 
> > 
> > and another pattern
> > 
> >     ... <-- mutex not locked
> >     QEMU_LOCK_GUARD(&mutex); <-- mutex locked from here onwards
> >     ...
> >     if (error) {
> >         return; <-- mutex is automatically unlocked
> >     }
> > 
> > This would work for libvirt too, at least for cases where the
> > lock+unlock are in the same scope (which should be the majority).
> 
> Yes I thought about this some time ago, but unfortunately in this very
> specific case it doesn't work.
> 
> For some specific/historic reason 'virDomainObjNew' returns a locked
> mutex. Caller is expected then to attach the object into the list or
> dispose of it, so this doesn't happen in the same context.

There's not that many callers of virDomainObjNew, nor users of
g_autoptr(virDomainObj), so we could remove that lock call and
push it up into the caller.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|