virobject: Introduce virObjectRWLockable

Up until now we only had virObjectLockable which uses mutexes for
mutually excluding each other in critical section. Well, this is
not enough. Future work will require RW locks so we might as well
have virObjectRWLockable which is introduced here.

Moreover, polymorphism is introduced to our code for the first
time. Yay! More specifically, virObjectLock will grab a write
lock, virObjectLockRead will grab a read lock then (what a
surprise right?). This has great advantage that an object can be
made derived from virObjectRWLockable in a single line and still
continue functioning properly (mutexes can be viewed as grabbing
write locks only). Then just those critical sections that can
grab a read lock need fixing. Therefore the resulting change is
going to be way smaller.

In order to avoid writer starvation, the object initializes RW
lock that prefers writers.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e58b5ebe32e91e775dc4b4f1ff7f51e58607ffa4..87753758f9ccf7cc1cafb560e927d107968ca6fc 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2283,6 +2283,7 @@ virNumaSetupMemoryPolicy;
 # util/virobject.h
 virClassForObject;
 virClassForObjectLockable;
+virClassForObjectRWLockable;
 virClassIsDerivedFrom;
 virClassName;
 virClassNew;
@@ -2293,8 +2294,10 @@ virObjectListFree;
 virObjectListFreeCount;
 virObjectLock;
 virObjectLockableNew;
+virObjectLockRead;
 virObjectNew;
 virObjectRef;
+virObjectRWLockableNew;
 virObjectUnlock;
 virObjectUnref;
 

diff --git a/src/util/virobject.c b/src/util/virobject.c
index 34805d34afd6bbadc98317fd666c42e5bed3f6da..4236abfef7ff99ef6d29223a112388f2ca1cc3bc 100644 (file)
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -49,8 +49,10 @@ struct _virClass {
 
 static virClassPtr virObjectClass;
 static virClassPtr virObjectLockableClass;
+static virClassPtr virObjectRWLockableClass;
 
 static void virObjectLockableDispose(void *anyobj);
+static void virObjectRWLockableDispose(void *anyobj);
 
 static int
 virObjectOnceInit(void)
@@ -67,6 +69,12 @@ virObjectOnceInit(void)
                                                virObjectLockableDispose)))
         return -1;
 
+    if (!(virObjectRWLockableClass = virClassNew(virObjectClass,
+                                                 "virObjectRWLockable",
+                                                 sizeof(virObjectRWLockable),
+                                                 virObjectRWLockableDispose)))
+        return -1;
+
     return 0;
 }
 
@@ -103,6 +111,21 @@ virClassForObjectLockable(void)
 }
 
 
+/**
+ * virClassForObjectRWLockable:
+ *
+ * Returns the class instance for the virObjectRWLockable type
+ */
+virClassPtr
+virClassForObjectRWLockable(void)
+{
+    if (virObjectInitialize() < 0)
+        return NULL;
+
+    return virObjectRWLockableClass;
+}
+
+
 /**
  * virClassNew:
  * @parent: the parent class
@@ -237,6 +260,32 @@ virObjectLockableNew(virClassPtr klass)
 }
 
 
+void *
+virObjectRWLockableNew(virClassPtr klass)
+{
+    virObjectRWLockablePtr obj;
+
+    if (!virClassIsDerivedFrom(klass, virClassForObjectRWLockable())) {
+        virReportInvalidArg(klass,
+                            _("Class %s must derive from virObjectRWLockable"),
+                            virClassName(klass));
+        return NULL;
+    }
+
+    if (!(obj = virObjectNew(klass)))
+        return NULL;
+
+    if (virRWLockInitPreferWriter(&obj->lock) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Unable to initialize RW lock"));
+        virObjectUnref(obj);
+        return NULL;
+    }
+
+    return obj;
+}
+
+
 static void
 virObjectLockableDispose(void *anyobj)
 {
@@ -246,6 +295,15 @@ virObjectLockableDispose(void *anyobj)
 }
 
 
+static void
+virObjectRWLockableDispose(void *anyobj)
+{
+    virObjectRWLockablePtr obj = anyobj;
+
+    virRWLockDestroy(&obj->lock);
+}
+
+
 /**
  * virObjectUnref:
  * @anyobj: any instance of virObjectPtr
@@ -309,27 +367,42 @@ virObjectRef(void *anyobj)
 }
 
 
-static virObjectLockablePtr
-virObjectGetLockableObj(void *anyobj)
+/**
+ * virObjectLock:
+ * @anyobj: any instance of virObjectLockable or virObjectRWLockable
+ *
+ * Acquire a lock on @anyobj. The lock must be released by
+ * virObjectUnlock. In case the passed object is instance of
+ * virObjectRWLockable a write lock is acquired.
+ *
+ * The caller is expected to have acquired a reference
+ * on the object before locking it (eg virObjectRef).
+ * The object must be unlocked before releasing this
+ * reference.
+ */
+void
+virObjectLock(void *anyobj)
 {
-    virObjectPtr obj;
-
-    if (virObjectIsClass(anyobj, virObjectLockableClass))
-        return anyobj;
-
-    obj = anyobj;
-    VIR_WARN("Object %p (%s) is not a virObjectLockable instance",
-              anyobj, obj ? obj->klass->name : "(unknown)");
-
-    return NULL;
+    if (virObjectIsClass(anyobj, virObjectLockableClass)) {
+        virObjectLockablePtr obj = anyobj;
+        virMutexLock(&obj->lock);
+    } else if (virObjectIsClass(anyobj, virObjectRWLockableClass)) {
+        virObjectRWLockablePtr obj = anyobj;
+        virRWLockWrite(&obj->lock);
+    } else {
+        virObjectPtr obj = anyobj;
+        VIR_WARN("Object %p (%s) is not a virObjectLockable "
+                 "nor virObjectRWLockable instance",
+                 anyobj, obj ? obj->klass->name : "(unknown)");
+    }
 }
 
 
 /**
- * virObjectLock:
- * @anyobj: any instance of virObjectLockablePtr
+ * virObjectLockRead:
+ * @anyobj: any instance of virObjectRWLockable
  *
- * Acquire a lock on @anyobj. The lock must be
+ * Acquire a read lock on @anyobj. The lock must be
  * released by virObjectUnlock.
  *
  * The caller is expected to have acquired a reference
@@ -338,33 +411,41 @@ virObjectGetLockableObj(void *anyobj)
  * reference.
  */
 void
-virObjectLock(void *anyobj)
+virObjectLockRead(void *anyobj)
 {
-    virObjectLockablePtr obj = virObjectGetLockableObj(anyobj);
-
-    if (!obj)
-        return;
-
-    virMutexLock(&obj->lock);
+    if (virObjectIsClass(anyobj, virObjectRWLockableClass)) {
+        virObjectRWLockablePtr obj = anyobj;
+        virRWLockRead(&obj->lock);
+    } else {
+        virObjectPtr obj = anyobj;
+        VIR_WARN("Object %p (%s) is not a virObjectRWLockable instance",
+                 anyobj, obj ? obj->klass->name : "(unknown)");
+    }
 }
 
 
 /**
  * virObjectUnlock:
- * @anyobj: any instance of virObjectLockablePtr
+ * @anyobj: any instance of virObjectLockable or virObjectRWLockable
  *
- * Release a lock on @anyobj. The lock must have been
- * acquired by virObjectLock.
+ * Release a lock on @anyobj. The lock must have been acquired by
+ * virObjectLock or virObjectLockRead.
  */
 void
 virObjectUnlock(void *anyobj)
 {
-    virObjectLockablePtr obj = virObjectGetLockableObj(anyobj);
-
-    if (!obj)
-        return;
-
-    virMutexUnlock(&obj->lock);
+    if (virObjectIsClass(anyobj, virObjectLockableClass)) {
+        virObjectLockablePtr obj = anyobj;
+        virMutexUnlock(&obj->lock);
+    } else if (virObjectIsClass(anyobj, virObjectRWLockableClass)) {
+        virObjectRWLockablePtr obj = anyobj;
+        virRWLockUnlock(&obj->lock);
+    } else {
+        virObjectPtr obj = anyobj;
+        VIR_WARN("Object %p (%s) is not a virObjectLockable "
+                 "nor virObjectRWLockable instance",
+                 anyobj, obj ? obj->klass->name : "(unknown)");
+    }
 }
 
 

diff --git a/src/util/virobject.h b/src/util/virobject.h
index f4c292b16c1104c26bb2fa073eccb59097a3af2b..5985fadb2de62c46f287e76fe38d2edce1d95df5 100644 (file)
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -34,6 +34,9 @@ typedef virObject *virObjectPtr;
 typedef struct _virObjectLockable virObjectLockable;
 typedef virObjectLockable *virObjectLockablePtr;
 
+typedef struct _virObjectRWLockable virObjectRWLockable;
+typedef virObjectRWLockable *virObjectRWLockablePtr;
+
 typedef void (*virObjectDisposeCallback)(void *obj);
 
 /* Most code should not play with the contents of this struct; however,
@@ -59,9 +62,14 @@ struct _virObjectLockable {
     virMutex lock;
 };
 
+struct _virObjectRWLockable {
+    virObject parent;
+    virRWLock lock;
+};
 
 virClassPtr virClassForObject(void);
 virClassPtr virClassForObjectLockable(void);
+virClassPtr virClassForObjectRWLockable(void);
 
 # ifndef VIR_PARENT_REQUIRED
 #  define VIR_PARENT_REQUIRED ATTRIBUTE_NONNULL(1)
@@ -108,10 +116,18 @@ void *
 virObjectLockableNew(virClassPtr klass)
     ATTRIBUTE_NONNULL(1);
 
+void *
+virObjectRWLockableNew(virClassPtr klass)
+    ATTRIBUTE_NONNULL(1);
+
 void
 virObjectLock(void *lockableobj)
     ATTRIBUTE_NONNULL(1);
 
+void
+virObjectLockRead(void *lockableobj)
+    ATTRIBUTE_NONNULL(1);
+
 void
 virObjectUnlock(void *lockableobj)
     ATTRIBUTE_NONNULL(1);