[libvirt] [PATCH 2/6] virObject: Introduce virObjectRecursiveLockableNew

[John Ferlan]

On 02/12/2018 05:52 AM, Michal Privoznik wrote:
> Sometimes we need the lock in virObjectLockable to be recursive.
> Because of the nature of pthreads we don't need a special class
> for that - the pthread_* APIs don't distinguish between normal
> and recursive locks.
> 
> Based-on-work-of: John Ferlan <jferlan at redhat.com>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virobject.c     | 22 +++++++++++++++++++---
>  src/util/virobject.h     |  4 ++++
>  3 files changed, 24 insertions(+), 3 deletions(-)
> 

Adding recursive locks to virObject* was already rejected once before:

Patch:
https://www.redhat.com/archives/libvir-list/2017-May/msg01183.html

Responses:
https://www.redhat.com/archives/libvir-list/2017-May/msg01212.html
https://www.redhat.com/archives/libvir-list/2017-May/msg01215.html

When you added RWLock's in commit '77f4593b' (afterwards) - I began
thinking - hey the NWFilter code could use those. So I essentially
ditched the effort that used trylock:

https://www.redhat.com/archives/libvir-list/2017-July/msg00673.html

in favor of one that used RWLocks:

https://www.redhat.com/archives/libvir-list/2017-October/msg00264.html

Unlike previous alterations to driver/vir*objlist code - I found that I
needed to change the vir*Obj before changing the vir*ObjList, although I
have forgotten why at this point. Eventually I also discovered why
avocado test was causing libvirtd to go defunct (as noted in the cover
of the v3). That was due to the way the test restarts libvirtd prior to
each run and an unrelated issue in nodedev w/r/t initialization, see:

https://www.redhat.com/archives/libvir-list/2017-December/msg00312.html

John

> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 3b14d7d15..fcf378105 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2417,6 +2417,7 @@ virObjectListFreeCount;
>  virObjectLock;
>  virObjectLockableNew;
>  virObjectNew;
> +virObjectRecursiveLockableNew;
>  virObjectRef;
>  virObjectRWLockableNew;
>  virObjectRWLockRead;
> diff --git a/src/util/virobject.c b/src/util/virobject.c
> index b2fc63aec..1d82e826b 100644
> --- a/src/util/virobject.c
> +++ b/src/util/virobject.c
> @@ -257,8 +257,9 @@ virObjectNew(virClassPtr klass)
>  }
>  
>  
> -void *
> -virObjectLockableNew(virClassPtr klass)
> +static void *
> +virObjectLockableNewInternal(virClassPtr klass,
> +                             bool recursive)
>  {
>      virObjectLockablePtr obj;
>  
> @@ -272,7 +273,8 @@ virObjectLockableNew(virClassPtr klass)
>      if (!(obj = virObjectNew(klass)))
>          return NULL;
>  
> -    if (virMutexInit(&obj->lock) < 0) {
> +    if ((!recursive && virMutexInit(&obj->lock) < 0) ||
> +        (recursive && virMutexInitRecursive(&obj->lock) < 0)) {
>          virReportSystemError(errno, "%s",
>                               _("Unable to initialize mutex"));
>          virObjectUnref(obj);
> @@ -283,6 +285,20 @@ virObjectLockableNew(virClassPtr klass)
>  }
>  
>  
> +void *
> +virObjectLockableNew(virClassPtr klass)
> +{
> +    return virObjectLockableNewInternal(klass, false);
> +}
> +
> +
> +void *
> +virObjectRecursiveLockableNew(virClassPtr klass)
> +{
> +    return virObjectLockableNewInternal(klass, true);
> +}
> +
> +
>  void *
>  virObjectRWLockableNew(virClassPtr klass)
>  {
> diff --git a/src/util/virobject.h b/src/util/virobject.h
> index ac6cf22f9..367d505ae 100644
> --- a/src/util/virobject.h
> +++ b/src/util/virobject.h
> @@ -116,6 +116,10 @@ void *
>  virObjectLockableNew(virClassPtr klass)
>      ATTRIBUTE_NONNULL(1);
>  
> +void *
> +virObjectRecursiveLockableNew(virClassPtr klass)
> +    ATTRIBUTE_NONNULL(1);
> +
>  void *
>  virObjectRWLockableNew(virClassPtr klass)
>      ATTRIBUTE_NONNULL(1);
>