[libvirt] [PATCH 1/13] Adding recursive locks

This patch adds recursive locks necessary due to the processing of network filter XML that can reference other network filters, including references that cause looks. Loops in the XML are prevented but their detection requires recursive locks.

On Thu, Mar 11, 2010 at 08:06:34AM -0500, Stefan Berger wrote:
This patch adds recursive locks necessary due to the processing of network filter XML that can reference other network filters, including references that cause looks. Loops in the XML are prevented but their detection requires recursive locks.
--- src/util/threads-pthread.c | 12 ++++++++++++ src/util/threads-pthread.h | 1 + src/util/threads-win32.c | 5 +++++ src/util/threads.h | 1 + 4 files changed, 19 insertions(+)
Index: libvirt-acl/src/util/threads-pthread.c =================================================================== --- libvirt-acl.orig/src/util/threads-pthread.c +++ libvirt-acl/src/util/threads-pthread.c @@ -43,6 +43,18 @@ int virMutexInit(virMutexPtr m) return 0; }
+int virRecursiveMutexInit(virMutexPtr m) +{ + int ret; + pthread_mutexattr_init(&m->attr); + pthread_mutexattr_settype(&m->attr, PTHREAD_MUTEX_RECURSIVE); + if ((ret = pthread_mutex_init(&m->lock, &m->attr)) != 0) { + errno = ret; + return -1; + } + return 0; +} + void virMutexDestroy(virMutexPtr m) { pthread_mutex_destroy(&m->lock);
Minor point, I think it would be better to call it virMutexInitRecursive() so we keep the standard virMutex prefix for naming convention
Index: libvirt-acl/src/util/threads-pthread.h =================================================================== --- libvirt-acl.orig/src/util/threads-pthread.h +++ libvirt-acl/src/util/threads-pthread.h @@ -24,6 +24,7 @@ #include <pthread.h>
struct virMutex { + pthread_mutexattr_t attr; pthread_mutex_t lock; };
I don't think this is neccessary - the attributes are only used at time of pthread_mutex_init() call, so don't need to be kept around after that. Thus the 'pthread_mutexattr_t attr' could just be a local variable in virRecursiveMutexInit().
Index: libvirt-acl/src/util/threads.h =================================================================== --- libvirt-acl.orig/src/util/threads.h +++ libvirt-acl/src/util/threads.h @@ -38,6 +38,7 @@ int virThreadInitialize(void) ATTRIBUTE_ void virThreadOnExit(void);
int virMutexInit(virMutexPtr m) ATTRIBUTE_RETURN_CHECK; +int virRecursiveMutexInit(virMutexPtr m) ATTRIBUTE_RETURN_CHECK; void virMutexDestroy(virMutexPtr m);
void virMutexLock(virMutexPtr m); Index: libvirt-acl/src/util/threads-win32.c =================================================================== --- libvirt-acl.orig/src/util/threads-win32.c +++ libvirt-acl/src/util/threads-win32.c @@ -76,6 +76,11 @@ int virMutexInit(virMutexPtr m) return 0; }
+int virRecursiveMutexInit(virMutexPtr m) +{ + return virMutexInit(m); +} + void virMutexDestroy(virMutexPtr m) { CloseHandle(m->lock);
Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

"Daniel P. Berrange" <berrange@redhat.com> wrote on 03/17/2010 07:48:46 AM: Berrange"
void virMutexDestroy(virMutexPtr m) { pthread_mutex_destroy(&m->lock);
Minor point, I think it would be better to call it virMutexInitRecursive() so we keep the standard virMutex prefix for naming convention
Ok. Done.
Index: libvirt-acl/src/util/threads-pthread.h =================================================================== --- libvirt-acl.orig/src/util/threads-pthread.h +++ libvirt-acl/src/util/threads-pthread.h @@ -24,6 +24,7 @@ #include <pthread.h>
struct virMutex { + pthread_mutexattr_t attr; pthread_mutex_t lock; };
I don't think this is neccessary - the attributes are only used at time of pthread_mutex_init() call, so don't need to be kept around after that. Thus the 'pthread_mutexattr_t attr' could just be a local variable in virRecursiveMutexInit().
Ok. Thanks for reviewing. Regards, Stefan
participants (2)
-
Daniel P. Berrange
-
Stefan Berger