From: "Daniel P. Berrange" <berra...@redhat.com>

Switch virDomainObjList to inherit from virObjectLockable and
make all the APIs acquire/release the mutex when running. This
makes virDomainObjList completely self-locking and no longer
reliant on the hypervisor driver locks
---
 src/conf/domain_conf.c | 75 ++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 61 insertions(+), 14 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 79da5eb..a1e899f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -60,7 +60,7 @@ verify(VIR_DOMAIN_VIRT_LAST <= 32);
 
 
 struct _virDomainObjList {
-    virObject parent;
+    virObjectLockable parent;
 
     /* uuid string -> virDomainObj  mapping
      * for O(1), lockless lookup-by-uuid */
@@ -715,7 +715,7 @@ static int virDomainObjOnceInit(void)
                                           virDomainObjDispose)))
         return -1;
 
-    if (!(virDomainObjListClass = virClassNew(virClassForObject(),
+    if (!(virDomainObjListClass = virClassNew(virClassForObjectLockable(),
                                               "virDomainObjList",
                                               sizeof(virDomainObjList),
                                               virDomainObjListDispose)))
@@ -840,9 +840,11 @@ virDomainObjPtr virDomainObjListFindByID(const 
virDomainObjListPtr doms,
                                          int id)
 {
     virDomainObjPtr obj;
+    virObjectLock(doms);
     obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id);
     if (obj)
         virObjectLock(obj);
+    virObjectUnlock(doms);
     return obj;
 }
 
@@ -853,11 +855,13 @@ virDomainObjPtr virDomainObjListFindByUUID(const 
virDomainObjListPtr doms,
     char uuidstr[VIR_UUID_STRING_BUFLEN];
     virDomainObjPtr obj;
 
+    virObjectLock(doms);
     virUUIDFormat(uuid, uuidstr);
 
     obj = virHashLookup(doms->objs, uuidstr);
     if (obj)
         virObjectLock(obj);
+    virObjectUnlock(doms);
     return obj;
 }
 
@@ -879,9 +883,11 @@ virDomainObjPtr virDomainObjListFindByName(const 
virDomainObjListPtr doms,
                                            const char *name)
 {
     virDomainObjPtr obj;
+    virObjectLock(doms);
     obj = virHashSearch(doms->objs, virDomainObjListSearchName, name);
     if (obj)
         virObjectLock(obj);
+    virObjectUnlock(doms);
     return obj;
 }
 
@@ -1880,25 +1886,32 @@ void virDomainObjAssignDef(virDomainObjPtr domain,
  * current def is Live
  *
  */
-virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms,
-                                    virCapsPtr caps,
-                                    const virDomainDefPtr def,
-                                    unsigned int flags,
-                                    virDomainDefPtr *oldDef)
+static virDomainObjPtr
+virDomainObjListAddLocked(virDomainObjListPtr doms,
+                          virCapsPtr caps,
+                          const virDomainDefPtr def,
+                          unsigned int flags,
+                          virDomainDefPtr *oldDef)
 {
     virDomainObjPtr vm;
     char uuidstr[VIR_UUID_STRING_BUFLEN];
+
     if (oldDef)
         *oldDef = false;
 
+    virUUIDFormat(def->uuid, uuidstr);
+
     /* See if a VM with matching UUID already exists */
-    if ((vm = virDomainObjListFindByUUID(doms, def->uuid))) {
+    if ((vm = virHashLookup(doms->objs, uuidstr))) {
+        virObjectLock(vm);
         /* UUID matches, but if names don't match, refuse it */
         if (STRNEQ(vm->def->name, def->name)) {
             virUUIDFormat(vm->def->uuid, uuidstr);
             virReportError(VIR_ERR_OPERATION_FAILED,
                            _("domain '%s' is already defined with uuid %s"),
                            vm->def->name, uuidstr);
+            virObjectUnlock(vm);
+            vm = NULL;
             goto cleanup;
         }
 
@@ -1908,6 +1921,8 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr 
doms,
                 virReportError(VIR_ERR_OPERATION_INVALID,
                                _("domain is already active as '%s'"),
                                vm->def->name);
+                virObjectUnlock(vm);
+                vm = NULL;
                 goto cleanup;
             }
         }
@@ -1934,12 +1949,11 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr 
doms,
         }
     } else {
         /* UUID does not match, but if a name matches, refuse it */
-        if ((vm = virDomainObjListFindByName(doms, def->name))) {
+        if ((vm = virHashSearch(doms->objs, virDomainObjListSearchName, 
def->name))) {
             virUUIDFormat(vm->def->uuid, uuidstr);
             virReportError(VIR_ERR_OPERATION_FAILED,
                            _("domain '%s' already exists with uuid %s"),
                            def->name, uuidstr);
-            virObjectUnlock(vm);
             vm = NULL;
             goto cleanup;
         }
@@ -1958,6 +1972,21 @@ cleanup:
     return vm;
 }
 
+
+virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms,
+                                    virCapsPtr caps,
+                                    const virDomainDefPtr def,
+                                    unsigned int flags,
+                                    virDomainDefPtr *oldDef)
+{
+    virDomainObjPtr ret;
+
+    virObjectLock(doms);
+    ret = virDomainObjListAddLocked(doms, caps, def, flags, oldDef);
+    virObjectUnlock(doms);
+    return ret;
+}
+
 /*
  * Mark the running VM config as transient. Ensures transient hotplug
  * operations do not persist past shutdown.
@@ -2076,11 +2105,14 @@ void virDomainObjListRemove(virDomainObjListPtr doms,
                             virDomainObjPtr dom)
 {
     char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+    virObjectLock(doms);
     virUUIDFormat(dom->def->uuid, uuidstr);
 
     virObjectUnlock(dom);
 
     virHashRemoveEntry(doms->objs, uuidstr);
+    virObjectUnlock(doms);
 }
 
 
@@ -14880,6 +14912,7 @@ virDomainObjListLoadConfig(virDomainObjListPtr doms,
     virDomainObjPtr dom;
     int autostart;
     int newVM = 1;
+    char uuidstr[VIR_UUID_STRING_BUFLEN];
 
     if ((configFile = virDomainConfigFile(configDir, name)) == NULL)
         goto error;
@@ -14896,7 +14929,10 @@ virDomainObjListLoadConfig(virDomainObjListPtr doms,
     /* if the domain is already in our hashtable, we only need to
      * update the autostart flag
      */
-    if ((dom = virDomainObjListFindByUUID(doms, def->uuid))) {
+    virUUIDFormat(def->uuid, uuidstr);
+
+    if ((dom = virHashLookup(doms->objs, uuidstr))) {
+        virObjectLock(dom);
         dom->autostart = autostart;
 
         if (virDomainObjIsActive(dom) &&
@@ -14911,7 +14947,7 @@ virDomainObjListLoadConfig(virDomainObjListPtr doms,
         return dom;
     }
 
-    if (!(dom = virDomainObjListAdd(doms, caps, def, 0, NULL)))
+    if (!(dom = virDomainObjListAddLocked(doms, caps, def, 0, NULL)))
         goto error;
 
     dom->autostart = autostart;
@@ -14999,6 +15035,8 @@ int virDomainObjListLoadAllConfigs(virDomainObjListPtr 
doms,
         return -1;
     }
 
+    virObjectLock(doms);
+
     while ((entry = readdir(dir))) {
         virDomainObjPtr dom;
 
@@ -15036,7 +15074,7 @@ int virDomainObjListLoadAllConfigs(virDomainObjListPtr 
doms,
     }
 
     closedir(dir);
-
+    virObjectUnlock(doms);
     return 0;
 }
 
@@ -15161,10 +15199,12 @@ static void virDomainObjListCountInactive(void 
*payload, const void *name ATTRIB
 int virDomainObjListNumOfDomains(virDomainObjListPtr doms, int active)
 {
     int count = 0;
+    virObjectLock(doms);
     if (active)
         virHashForEach(doms->objs, virDomainObjListCountActive, &count);
     else
         virHashForEach(doms->objs, virDomainObjListCountInactive, &count);
+    virObjectUnlock(doms);
     return count;
 }
 
@@ -15189,7 +15229,9 @@ int virDomainObjListGetActiveIDs(virDomainObjListPtr 
doms,
                                  int maxids)
 {
     struct virDomainIDData data = { 0, maxids, ids };
+    virObjectLock(doms);
     virHashForEach(doms->objs, virDomainObjListCopyActiveIDs, &data);
+    virObjectUnlock(doms);
     return data.numids;
 }
 
@@ -15225,7 +15267,9 @@ int 
virDomainObjListGetInactiveNames(virDomainObjListPtr doms,
 {
     struct virDomainNameData data = { 0, 0, maxnames, names };
     int i;
+    virObjectLock(doms);
     virHashForEach(doms->objs, virDomainObjListCopyInactiveNames, &data);
+    virObjectUnlock(doms);
     if (data.oom) {
         virReportOOMError();
         goto cleanup;
@@ -15261,8 +15305,9 @@ int virDomainObjListForEach(virDomainObjListPtr doms,
     struct virDomainListIterData data = {
         callback, opaque, 0,
     };
+    virObjectLock(doms);
     virHashForEach(doms->objs, virDomainObjListHelper, &data);
-
+    virObjectUnlock(doms);
     return data.ret;
 }
 
@@ -16033,6 +16078,7 @@ virDomainObjListExport(virDomainObjListPtr doms,
 
     struct virDomainListData data = { conn, NULL, flags, 0, false };
 
+    virObjectLock(doms);
     if (domains) {
         if (VIR_ALLOC_N(data.domains, virHashSize(doms->objs) + 1) < 0) {
             virReportOOMError();
@@ -16062,6 +16108,7 @@ cleanup:
     }
 
     VIR_FREE(data.domains);
+    virObjectUnlock(doms);
     return ret;
 }
 
-- 
1.8.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to