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>

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;
 

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,28 +367,13 @@ virObjectRef(void *anyobj)
 }
 
 
-static virObjectLockablePtr
-virObjectGetLockableObj(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;
-}
-
-
 /**
  * virObjectLock:
- * @anyobj: any instance of virObjectLockablePtr
+ * @anyobj: any instance of virObjectLockable or virObjectRWLockable
  *
- * Acquire a lock on @anyobj. The lock must be
- * released by virObjectUnlock.
+ * 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).
@@ -340,31 +383,69 @@ virObjectGetLockableObj(void *anyobj)
 void
 virObjectLock(void *anyobj)
 {
-    virObjectLockablePtr obj = virObjectGetLockableObj(anyobj);
+    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)");
+    }
+}
 
-    if (!obj)
-        return;
 
-    virMutexLock(&obj->lock);
+/**
+ * virObjectLockRead:
+ * @anyobj: any instance of virObjectRWLockable
+ *
+ * Acquire a read lock on @anyobj. The lock must be
+ * released by virObjectUnlock.
+ *
+ * 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
+virObjectLockRead(void *anyobj)
+{
+    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)");
+    }
 }
 
 

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);