[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 05/13] Make virDomainObjList self-locking via virObjectLockable



From: "Daniel P. Berrange" <berrange 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 38f5a68..a60a857 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 */
@@ -717,7 +717,7 @@ static int virDomainObjOnceInit(void)
                                           virDomainObjDispose)))
         return -1;
 
-    if (!(virDomainObjListClass = virClassNew(virClassForObject(),
+    if (!(virDomainObjListClass = virClassNew(virClassForObjectLockable(),
                                               "virDomainObjList",
                                               sizeof(virDomainObjList),
                                               virDomainObjListDispose)))
@@ -842,9 +842,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;
 }
 
@@ -855,11 +857,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;
 }
 
@@ -881,9 +885,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;
 }
 
@@ -1882,25 +1888,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;
         }
 
@@ -1910,6 +1923,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;
             }
         }
@@ -1936,12 +1951,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;
         }
@@ -1960,6 +1974,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.
@@ -2078,11 +2107,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);
 }
 
 
@@ -14864,6 +14896,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;
@@ -14880,7 +14913,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) &&
@@ -14895,7 +14931,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;
@@ -14983,6 +15019,8 @@ int virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
         return -1;
     }
 
+    virObjectLock(doms);
+
     while ((entry = readdir(dir))) {
         virDomainObjPtr dom;
 
@@ -15020,7 +15058,7 @@ int virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
     }
 
     closedir(dir);
-
+    virObjectUnlock(doms);
     return 0;
 }
 
@@ -15145,10 +15183,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;
 }
 
@@ -15173,7 +15213,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;
 }
 
@@ -15209,7 +15251,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;
@@ -15245,8 +15289,9 @@ int virDomainObjListForEach(virDomainObjListPtr doms,
     struct virDomainListIterData data = {
         callback, opaque, 0,
     };
+    virObjectLock(doms);
     virHashForEach(doms->objs, virDomainObjListHelper, &data);
-
+    virObjectUnlock(doms);
     return data.ret;
 }
 
@@ -16009,6 +16054,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();
@@ -16038,6 +16084,7 @@ cleanup:
     }
 
     VIR_FREE(data.domains);
+    virObjectUnlock(doms);
     return ret;
 }
 
-- 
1.8.0.2


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]