[libvirt] [PATCH v5 11/15] interface: Use virObjectLookupHash

John Ferlan jferlan at redhat.com
Wed Aug 23 21:22:07 UTC 2017


Use the virObjectLookupHash in _virInterfaceObjList. Convert the code
to use the LookupHash object and APIs rather than the local forward
linked list processing.

Usage of HashLookup{Find|Search} API's is via a callback mechanism
and returns a locked/reffed object.

The Clone API will make use of the ForEach functionality in
copying whatever is in one LookupHash list into the destination.

The only function requiring taking a lock is the AssignDef
function since it needs to be synchronized in such a way to
avoid multiple threads attempting to add the same named
object at the same time.

The NumOfInterfaces and GetNames can use the same callback
function with the only difference being the filling in of the
@names array from the passed @data structure if it exists.

Signed-off-by: John Ferlan <jferlan at redhat.com>
---
 src/conf/virinterfaceobj.c | 286 ++++++++++++++++++++++++---------------------
 1 file changed, 150 insertions(+), 136 deletions(-)

diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
index e993b92..a9b37d9 100644
--- a/src/conf/virinterfaceobj.c
+++ b/src/conf/virinterfaceobj.c
@@ -40,8 +40,7 @@ struct _virInterfaceObj {
 };
 
 struct _virInterfaceObjList {
-    size_t count;
-    virInterfaceObjPtr *objs;
+    virObjectLookupHash parent;
 };
 
 /* virInterfaceObj manipulation */
@@ -128,11 +127,41 @@ virInterfaceObjSetActive(virInterfaceObjPtr obj,
 virInterfaceObjListPtr
 virInterfaceObjListNew(void)
 {
-    virInterfaceObjListPtr interfaces;
+    return virObjectLookupHashNew(virClassForObjectLookupHash(), 10,
+                                  VIR_OBJECT_LOOKUP_HASH_NAME);
+}
 
-    if (VIR_ALLOC(interfaces) < 0)
-        return NULL;
-    return interfaces;
+
+static int
+virInterfaceObjListFindByMACStringCb(void *payload,
+                                     const void *name ATTRIBUTE_UNUSED,
+                                     void *opaque)
+{
+    virInterfaceObjPtr obj = payload;
+    virObjectLookupHashForEachDataPtr data = opaque;
+    char **const matches = (char **const)data->elems;
+    virInterfaceDefPtr def;
+    int ret = -1;
+
+    if (data->error)
+        return 0;
+
+    virObjectLock(obj);
+    def = obj->def;
+    if (STRCASEEQ(def->mac, data->matchStr)) {
+        if (data->nElems < data->maxElems) {
+            if (VIR_STRDUP(matches[data->nElems], def->name) < 0) {
+                data->error = true;
+                goto cleanup;
+            }
+            data->nElems++;
+        }
+    }
+    ret = 0;
+
+ cleanup:
+    virObjectUnlock(obj);
+    return ret;
 }
 
 
@@ -142,33 +171,21 @@ virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
                                    char **const matches,
                                    int maxmatches)
 {
-    size_t i;
-    int matchct = 0;
-
-    for (i = 0; i < interfaces->count; i++) {
-        virInterfaceObjPtr obj = interfaces->objs[i];
-        virInterfaceDefPtr def;
-
-        virObjectLock(obj);
-        def = obj->def;
-        if (STRCASEEQ(def->mac, mac)) {
-            if (matchct < maxmatches) {
-                if (VIR_STRDUP(matches[matchct], def->name) < 0) {
-                    virObjectUnlock(obj);
-                    goto error;
-                }
-                matchct++;
-            }
-        }
-        virObjectUnlock(obj);
-    }
-    return matchct;
+    virObjectLookupHashForEachData data = {
+        .error = false, .matchStr = mac, .nElems = 0,
+        .elems = (void **)matches, .maxElems = maxmatches };
+
+    return virObjectLookupHashForEachName(interfaces,
+                                          virInterfaceObjListFindByMACStringCb,
+                                          &data);
+}
 
- error:
-    while (--matchct >= 0)
-        VIR_FREE(matches[matchct]);
 
-    return -1;
+static virInterfaceObjPtr
+virInterfaceObjListFindByNameLocked(virInterfaceObjListPtr interfaces,
+                                    const char *name)
+{
+    return virObjectLookupHashFindLocked(interfaces, name);
 }
 
 
@@ -176,73 +193,66 @@ virInterfaceObjPtr
 virInterfaceObjListFindByName(virInterfaceObjListPtr interfaces,
                               const char *name)
 {
-    size_t i;
-
-    for (i = 0; i < interfaces->count; i++) {
-        virInterfaceObjPtr obj = interfaces->objs[i];
-        virInterfaceDefPtr def;
-
-        virObjectLock(obj);
-        def = obj->def;
-        if (STREQ(def->name, name))
-            return virObjectRef(obj);
-        virObjectUnlock(obj);
-    }
-
-    return NULL;
+    return virObjectLookupHashFind(interfaces, name);
 }
 
 
 void
 virInterfaceObjListFree(virInterfaceObjListPtr interfaces)
 {
-    size_t i;
+    virObjectUnref(interfaces);
+}
+
+
+static int
+virInterfaceObjListCloneCb(void *dstHashTable,
+                           void *sourceObject)
+{
+    virInterfaceObjListPtr dest = dstHashTable;
+    virInterfaceObjPtr srcObj = sourceObject;
+    int ret = -1;
+    char *xml = NULL;
+    virInterfaceObjPtr obj;
+    virInterfaceDefPtr backup = NULL;
+
+    if (!(xml = virInterfaceDefFormat(srcObj->def)))
+        goto cleanup;
+
+    if (!(backup = virInterfaceDefParseString(xml)))
+        goto cleanup;
+
+    if (!(obj = virInterfaceObjListAssignDef(dest, backup)))
+        goto cleanup;
 
-    for (i = 0; i < interfaces->count; i++)
-        virObjectUnref(interfaces->objs[i]);
-    VIR_FREE(interfaces->objs);
-    VIR_FREE(interfaces);
+    ret = 0;
+
+ cleanup:
+    VIR_FREE(xml);
+    virInterfaceDefFree(backup);
+
+    return ret;
 }
 
 
 virInterfaceObjListPtr
 virInterfaceObjListClone(virInterfaceObjListPtr interfaces)
 {
-    size_t i;
-    unsigned int cnt;
-    virInterfaceObjListPtr dest;
+    virInterfaceObjListPtr destInterfaces = NULL;
 
     if (!interfaces)
         return NULL;
 
-    if (!(dest = virInterfaceObjListNew()))
+    if (!(destInterfaces = virInterfaceObjListNew()))
         return NULL;
 
-    cnt = interfaces->count;
-    for (i = 0; i < cnt; i++) {
-        virInterfaceObjPtr srcobj = interfaces->objs[i];
-        virInterfaceDefPtr backup;
-        virInterfaceObjPtr obj;
-        char *xml = virInterfaceDefFormat(srcobj->def);
+    if (virObjectLookupHashClone(interfaces, destInterfaces,
+                                 virInterfaceObjListCloneCb) < 0)
+        goto cleanup;
 
-        if (!xml)
-            goto error;
+    return destInterfaces;
 
-        if (!(backup = virInterfaceDefParseString(xml))) {
-            VIR_FREE(xml);
-            goto error;
-        }
-
-        VIR_FREE(xml);
-        if (!(obj = virInterfaceObjListAssignDef(dest, backup)))
-            goto error;
-        virInterfaceObjEndAPI(&obj);
-    }
-
-    return dest;
-
- error:
-    virInterfaceObjListFree(dest);
+ cleanup:
+    virObjectUnref(destInterfaces);
     return NULL;
 }
 
@@ -252,24 +262,29 @@ virInterfaceObjListAssignDef(virInterfaceObjListPtr interfaces,
                              virInterfaceDefPtr def)
 {
     virInterfaceObjPtr obj;
+    virInterfaceObjPtr ret = NULL;
 
-    if ((obj = virInterfaceObjListFindByName(interfaces, def->name))) {
+    virObjectRWLockWrite(interfaces);
+
+    if ((obj = virInterfaceObjListFindByNameLocked(interfaces, def->name))) {
         virInterfaceDefFree(obj->def);
         obj->def = def;
+    } else {
+        if (!(obj = virInterfaceObjNew()))
+            goto cleanup;
 
-        return obj;
+        if (virObjectLookupHashAdd(interfaces, obj, NULL, def->name) < 0)
+            goto cleanup;
+        obj->def = def;
     }
 
-    if (!(obj = virInterfaceObjNew()))
-        return NULL;
+    ret = obj;
+    obj = NULL;
 
-    if (VIR_APPEND_ELEMENT_COPY(interfaces->objs,
-                                interfaces->count, obj) < 0) {
-        virInterfaceObjEndAPI(&obj);
-        return NULL;
-    }
-    obj->def = def;
-    return virObjectRef(obj);
+ cleanup:
+    virInterfaceObjEndAPI(&obj);
+    virObjectRWUnlock(interfaces);
+    return ret;
 }
 
 
@@ -277,20 +292,43 @@ void
 virInterfaceObjListRemove(virInterfaceObjListPtr interfaces,
                           virInterfaceObjPtr obj)
 {
-    size_t i;
+    if (!obj)
+        return;
 
-    virObjectUnlock(obj);
-    for (i = 0; i < interfaces->count; i++) {
-        virObjectLock(interfaces->objs[i]);
-        if (interfaces->objs[i] == obj) {
-            virObjectUnlock(interfaces->objs[i]);
-            virObjectUnref(interfaces->objs[i]);
-
-            VIR_DELETE_ELEMENT(interfaces->objs, i, interfaces->count);
-            break;
+    /* @obj is locked upon entry */
+    virObjectLookupHashRemove(interfaces, obj, NULL, obj->def->name);
+}
+
+
+static int
+virInterfaceObjListGetHelper(void *payload,
+                             const void *name ATTRIBUTE_UNUSED,
+                             void *opaque)
+{
+    virInterfaceObjPtr obj = payload;
+    virObjectLookupHashForEachDataPtr data = opaque;
+    char **const names = (char **const)data->elems;
+    virInterfaceDefPtr def;
+
+    if (data->error)
+        return 0;
+
+    if (data->maxElems >= 0 && data->nElems == data->maxElems)
+        return 0;
+
+    virObjectLock(obj);
+    def = obj->def;
+    if (data->wantActive == virInterfaceObjIsActive(obj)) {
+        if (names && VIR_STRDUP(names[data->nElems], def->name) < 0) {
+            data->error = true;
+            goto cleanup;
         }
-        virObjectUnlock(interfaces->objs[i]);
-    }
+        data->nElems++;
+     }
+
+ cleanup:
+    virObjectUnlock(obj);
+    return 0;
 }
 
 
@@ -298,18 +336,13 @@ int
 virInterfaceObjListNumOfInterfaces(virInterfaceObjListPtr interfaces,
                                    bool wantActive)
 {
-    size_t i;
-    int ninterfaces = 0;
-
-    for (i = 0; (i < interfaces->count); i++) {
-        virInterfaceObjPtr obj = interfaces->objs[i];
-        virObjectLock(obj);
-        if (wantActive == virInterfaceObjIsActive(obj))
-            ninterfaces++;
-        virObjectUnlock(obj);
-    }
+    virObjectLookupHashForEachData data = {
+        .wantActive = wantActive, .error = false, .nElems = 0,
+        .elems = NULL, .maxElems = -2 };
 
-    return ninterfaces;
+    return virObjectLookupHashForEachName(interfaces,
+                                          virInterfaceObjListGetHelper,
+                                          &data);
 }
 
 
@@ -319,30 +352,11 @@ virInterfaceObjListGetNames(virInterfaceObjListPtr interfaces,
                             char **const names,
                             int maxnames)
 {
-    int nnames = 0;
-    size_t i;
-
-    for (i = 0; i < interfaces->count && nnames < maxnames; i++) {
-        virInterfaceObjPtr obj = interfaces->objs[i];
-        virInterfaceDefPtr def;
-
-        virObjectLock(obj);
-        def = obj->def;
-        if (wantActive == virInterfaceObjIsActive(obj)) {
-            if (VIR_STRDUP(names[nnames], def->name) < 0) {
-                virObjectUnlock(obj);
-                goto failure;
-            }
-            nnames++;
-        }
-        virObjectUnlock(obj);
-    }
-
-    return nnames;
-
- failure:
-    while (--nnames >= 0)
-        VIR_FREE(names[nnames]);
+    virObjectLookupHashForEachData data = {
+        .wantActive = wantActive, .error = false, .nElems = 0,
+        .elems = (void **)names, .maxElems = maxnames };
 
-    return -1;
+    return virObjectLookupHashForEachName(interfaces,
+                                          virInterfaceObjListGetHelper,
+                                          &data);
 }
-- 
2.9.5




More information about the libvir-list mailing list