[libvirt] [PATCH v4 17/17] network: Use virObjectLookup{Keys|Hash}

Friday, 18 August 2017

Use the virObjectLookupKeys in _virNetworkObj and use the
virObjectLookupHash in _virNetworkObjList.

Convert the code to use the LookupHash object and APIs rather
than the local code and usage of virHash* calls.

Since the _virNetworkObjList only contains the @parent object
the virClassNew must be removed from OnceInit because instantiation
would fail since the object size would be the same as the parent
object size.

Usage of HashLookup{Find|Search} API's returns a locked/reffed
object so need to remove virObjectLock after FindBy*Locked calls.

Use the def->name as a second key to the LookupHash to make for
faster lookup's by name (instead of using Search on uuid key
and matching the name.

Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt;
---
 src/conf/virnetworkobj.c    | 291 ++++++++++++--------------------------------
 src/conf/virnetworkobj.h    |   3 +-
 tests/networkxml2conftest.c |   4 +-
 3 files changed, 86 insertions(+), 212 deletions(-)

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index 20f846d..d175191 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -40,11 +40,10 @@ VIR_LOG_INIT("conf.virnetworkobj");
 #define INIT_CLASS_ID_BITMAP_SIZE (1<<4)
 
 struct _virNetworkObj {
-    virObjectLockable parent;
+    virObjectLookupKeys parent;
 
     pid_t dnsmasqPid;
     pid_t radvdPid;
-    bool active;
     bool autostart;
     bool persistent;
 
@@ -61,30 +60,21 @@ struct _virNetworkObj {
 };
 
 struct _virNetworkObjList {
-    virObjectLockable parent;
-
-    virHashTablePtr objs;
+    virObjectLookupHash parent;
 };
 
 static virClassPtr virNetworkObjClass;
-static virClassPtr virNetworkObjListClass;
 static void virNetworkObjDispose(void *obj);
-static void virNetworkObjListDispose(void *obj);
 
 static int
 virNetworkObjOnceInit(void)
 {
-    if (!(virNetworkObjClass = virClassNew(virClassForObjectLockable(),
+    if (!(virNetworkObjClass = virClassNew(virClassForObjectLookupKeys(),
                                            "virNetworkObj",
                                            sizeof(virNetworkObj),
                                            virNetworkObjDispose)))
         return -1;
 
-    if (!(virNetworkObjListClass = virClassNew(virClassForObjectLockable(),
-                                               "virNetworkObjList",
-                                               sizeof(virNetworkObjList),
-                                               virNetworkObjListDispose)))
-        return -1;
     return 0;
 }
 
@@ -92,14 +82,15 @@ virNetworkObjOnceInit(void)
 VIR_ONCE_GLOBAL_INIT(virNetworkObj)
 
 virNetworkObjPtr
-virNetworkObjNew(void)
+virNetworkObjNew(const char *uuidstr,
+                 const char *name)
 {
     virNetworkObjPtr obj;
 
     if (virNetworkObjInitialize() < 0)
         return NULL;
 
-    if (!(obj = virObjectLockableNew(virNetworkObjClass)))
+    if (!(obj = virObjectLookupKeysNew(virNetworkObjClass, uuidstr, name)))
         return NULL;
 
     if (!(obj->classIdMap = virBitmapNew(INIT_CLASS_ID_BITMAP_SIZE)))
@@ -158,7 +149,7 @@ virNetworkObjGetNewDef(virNetworkObjPtr obj)
 bool
 virNetworkObjIsActive(virNetworkObjPtr obj)
 {
-    return obj->active;
+    return virObjectLookupKeysIsActive(obj);
 }
 
 
@@ -166,7 +157,7 @@ void
 virNetworkObjSetActive(virNetworkObjPtr obj,
                        bool active)
 {
-    obj->active = active;
+    virObjectLookupKeysSetActive(obj, active);
 }
 
 
@@ -332,36 +323,16 @@ virNetworkObjMacMgrDel(virNetworkObjPtr obj,
 virNetworkObjListPtr
 virNetworkObjListNew(void)
 {
-    virNetworkObjListPtr nets;
-
-    if (virNetworkObjInitialize() < 0)
-        return NULL;
-
-    if (!(nets = virObjectLockableNew(virNetworkObjListClass)))
-        return NULL;
-
-    if (!(nets->objs = virHashCreate(50, virObjectFreeHashData))) {
-        virObjectUnref(nets);
-        return NULL;
-    }
-
-    return nets;
+    return virObjectLookupHashNew(virClassForObjectLookupHash(), 50, true);
 }
 
 
 static virNetworkObjPtr
 virNetworkObjFindByUUIDLocked(virNetworkObjListPtr nets,
-                              const unsigned char *uuid)
+                              const char *uuidstr)
 {
-    virNetworkObjPtr obj = NULL;
-    char uuidstr[VIR_UUID_STRING_BUFLEN];
-
-    virUUIDFormat(uuid, uuidstr);
-
-    obj = virHashLookup(nets->objs, uuidstr);
-    if (obj)
-        virObjectRef(obj);
-    return obj;
+    virObjectLookupKeysPtr obj = virObjectLookupHashFindLocked(nets, uuidstr);
+    return (virNetworkObjPtr)obj;
 }
 
 
@@ -379,30 +350,11 @@ virNetworkObjPtr
 virNetworkObjFindByUUID(virNetworkObjListPtr nets,
                         const unsigned char *uuid)
 {
-    virNetworkObjPtr obj;
-
-    virObjectLock(nets);
-    obj = virNetworkObjFindByUUIDLocked(nets, uuid);
-    virObjectUnlock(nets);
-    if (obj)
-        virObjectLock(obj);
-    return obj;
-}
-
-
-static int
-virNetworkObjSearchName(const void *payload,
-                        const void *name ATTRIBUTE_UNUSED,
-                        const void *data)
-{
-    virNetworkObjPtr obj = (virNetworkObjPtr) payload;
-    int want = 0;
+    char uuidstr[VIR_UUID_STRING_BUFLEN];
+    virUUIDFormat(uuid, uuidstr);
 
-    virObjectLock(obj);
-    if (STREQ(obj->def->name, (const char *)data))
-        want = 1;
-    virObjectUnlock(obj);
-    return want;
+    virObjectLookupKeysPtr obj = virObjectLookupHashFind(nets, uuidstr);
+    return (virNetworkObjPtr)obj;
 }
 
 
@@ -410,12 +362,8 @@ static virNetworkObjPtr
 virNetworkObjFindByNameLocked(virNetworkObjListPtr nets,
                               const char *name)
 {
-    virNetworkObjPtr obj = NULL;
-
-    obj = virHashSearch(nets->objs, virNetworkObjSearchName, name, NULL);
-    if (obj)
-        virObjectRef(obj);
-    return obj;
+    virObjectLookupKeysPtr obj = virObjectLookupHashFindLocked(nets, name);
+    return (virNetworkObjPtr)obj;
 }
 
 
@@ -433,14 +381,8 @@ virNetworkObjPtr
 virNetworkObjFindByName(virNetworkObjListPtr nets,
                         const char *name)
 {
-    virNetworkObjPtr obj;
-
-    virObjectLock(nets);
-    obj = virNetworkObjFindByNameLocked(nets, name);
-    virObjectUnlock(nets);
-    if (obj)
-        virObjectLock(obj);
-    return obj;
+    virObjectLookupKeysPtr obj = virObjectLookupHashFind(nets, name);
+    return (virNetworkObjPtr)obj;
 }
 
 
@@ -470,15 +412,6 @@ virNetworkObjDispose(void *opaque)
 }
 
 
-static void
-virNetworkObjListDispose(void *opaque)
-{
-    virNetworkObjListPtr nets = opaque;
-
-    virHashFree(nets->objs);
-}
-
-
 /*
  * virNetworkObjUpdateAssignDef:
  * @network: the network object to update
@@ -560,12 +493,12 @@ virNetworkObjAssignDefLocked(virNetworkObjListPtr nets,
     virNetworkObjPtr ret = NULL;
     char uuidstr[VIR_UUID_STRING_BUFLEN];
 
+    virUUIDFormat(def->uuid, uuidstr);
     /* See if a network with matching UUID already exists */
-    if ((obj = virNetworkObjFindByUUIDLocked(nets, def->uuid))) {
+    if ((obj = virNetworkObjFindByUUIDLocked(nets, uuidstr))) {
         virObjectLock(obj);
         /* UUID matches, but if names don't match, refuse it */
         if (STRNEQ(obj->def->name, def->name)) {
-            virUUIDFormat(obj->def->uuid, uuidstr);
             virReportError(VIR_ERR_OPERATION_FAILED,
                            _("network '%s' is already defined with uuid
%s"),
                            obj->def->name, uuidstr);
@@ -588,20 +521,17 @@ virNetworkObjAssignDefLocked(virNetworkObjListPtr nets,
         /* UUID does not match, but if a name matches, refuse it */
         if ((obj = virNetworkObjFindByNameLocked(nets, def->name))) {
             virObjectLock(obj);
-            virUUIDFormat(obj->def->uuid, uuidstr);
             virReportError(VIR_ERR_OPERATION_FAILED,
                            _("network '%s' already exists with uuid
%s"),
                            def->name, uuidstr);
             goto cleanup;
         }
 
-        if (!(obj = virNetworkObjNew()))
-              goto cleanup;
+        if (!(obj = virNetworkObjNew(uuidstr, def->name)))
+            goto cleanup;
 
-        virUUIDFormat(def->uuid, uuidstr);
-        if (virHashAddEntry(nets->objs, uuidstr, obj) < 0)
+        if (virObjectLookupHashAdd(nets, (virObjectLookupKeysPtr)obj) < 0)
             goto cleanup;
-        virObjectRef(obj);
 
         obj->def = def;
         obj->persistent = !(flags & VIR_NETWORK_OBJ_LIST_ADD_LIVE);
@@ -638,9 +568,9 @@ virNetworkObjAssignDef(virNetworkObjListPtr nets,
 {
     virNetworkObjPtr obj;
 
-    virObjectLock(nets);
+    virObjectRWLockWrite(nets);
     obj = virNetworkObjAssignDefLocked(nets, def, flags);
-    virObjectUnlock(nets);
+    virObjectRWUnlock(nets);
     return obj;
 }
 
@@ -784,16 +714,7 @@ void
 virNetworkObjRemoveInactive(virNetworkObjListPtr nets,
                             virNetworkObjPtr obj)
 {
-    char uuidstr[VIR_UUID_STRING_BUFLEN];
-
-    virUUIDFormat(obj->def->uuid, uuidstr);
-    virObjectRef(obj);
-    virObjectUnlock(obj);
-    virObjectLock(nets);
-    virObjectLock(obj);
-    virHashRemoveEntry(nets->objs, uuidstr);
-    virObjectUnlock(nets);
-    virObjectUnref(obj);
+    virObjectLookupHashRemove(nets, (virObjectLookupKeysPtr)obj);
 }
 
 
@@ -968,7 +889,8 @@ virNetworkLoadState(virNetworkObjListPtr nets,
         obj->floor_sum = floor_sum_val;
 
     obj->taint = taint;
-    obj->active = true; /* network with a state file is by definition active */
+    /* network with a state file is by definition active */
+    virNetworkObjSetActive(obj, true);
 
  cleanup:
     VIR_FREE(configFile);
@@ -1177,14 +1099,15 @@ virNetworkObjBridgeInUse(virNetworkObjListPtr nets,
                          const char *bridge,
                          const char *skipname)
 {
+    bool ret;
     virNetworkObjPtr obj;
     struct virNetworkObjBridgeInUseHelperData data = {bridge, skipname};
 
-    virObjectLock(nets);
-    obj = virHashSearch(nets->objs, virNetworkObjBridgeInUseHelper, &data, NULL);
-    virObjectUnlock(nets);
+    obj = (virNetworkObjPtr)virObjectLookupHashSearch(nets,
virNetworkObjBridgeInUseHelper, &data);
 
-    return obj != NULL;
+    ret = obj != NULL;
+    virNetworkObjEndAPI(&obj);
+    return ret;
 }
 
 
@@ -1309,21 +1232,14 @@ virNetworkMatch(virNetworkObjPtr obj,
 #undef MATCH
 
 
-struct virNetworkObjListData {
-    virConnectPtr conn;
-    virNetworkPtr *nets;
-    virNetworkObjListFilter filter;
-    unsigned int flags;
-    int nnets;
-    bool error;
-};
-
 static int
-virNetworkObjListPopulate(void *payload,
+virNetworkObjListExportCb(void *payload,
                           const void *name ATTRIBUTE_UNUSED,
                           void *opaque)
 {
-    struct virNetworkObjListData *data = opaque;
+    virObjectLookupHashForEachDataPtr data = opaque;
+    virNetworkPtr *nets = (virNetworkPtr *)data->elems;
+    virNetworkObjListFilter filter = data->filter;
     virNetworkObjPtr obj = payload;
     virNetworkPtr net = NULL;
 
@@ -1332,15 +1248,14 @@ virNetworkObjListPopulate(void *payload,
 
     virObjectLock(obj);
 
-    if (data->filter &&
-        !data->filter(data->conn, obj->def))
+    if (filter && !filter(data->conn, obj->def))
         goto cleanup;
 
     if (!virNetworkMatch(obj, data->flags))
         goto cleanup;
 
-    if (!data->nets) {
-        data->nnets++;
+    if (!nets) {
+        data->nElems++;
         goto cleanup;
     }
 
@@ -1349,7 +1264,7 @@ virNetworkObjListPopulate(void *payload,
         goto cleanup;
     }
 
-    data->nets[data->nnets++] = net;
+    nets[data->nElems++] = net;
 
  cleanup:
     virObjectUnlock(obj);
@@ -1364,34 +1279,19 @@ virNetworkObjListExport(virConnectPtr conn,
                         virNetworkObjListFilter filter,
                         unsigned int flags)
 {
-    int ret = -1;
-    struct virNetworkObjListData data = {
-        .conn = conn, .nets = NULL, .filter = filter, .flags = flags,
-        .nnets = 0, .error = false };
-
-    virObjectLock(netobjs);
-    if (nets && VIR_ALLOC_N(data.nets, virHashSize(netobjs->objs) + 1) <
0)
-        goto cleanup;
-
-    virHashForEach(netobjs->objs, virNetworkObjListPopulate, &data);
+    int ret;
+    virObjectLookupHashForEachData data = {
+        .conn = conn, .filter = filter, .error = false, .nElems = 0,
+        .elems = NULL, .maxElems = 0, .flags = flags };
 
-    if (data.error)
-        goto cleanup;
+    if (nets)
+        data.maxElems = -1;
 
-    if (data.nets) {
-        /* trim the array to the final size */
-        ignore_value(VIR_REALLOC_N(data.nets, data.nnets + 1));
-        *nets = data.nets;
-        data.nets = NULL;
-    }
+    ret = virObjectLookupHashForEach(netobjs, virNetworkObjListExportCb, &data);
 
-    ret = data.nnets;
- cleanup:
-    virObjectUnlock(netobjs);
-    while (data.nets && data.nnets)
-        virObjectUnref(data.nets[--data.nnets]);
+    if (nets)
+        *nets = (virNetworkPtr *)data.elems;
 
-    VIR_FREE(data.nets);
     return ret;
 }
 
@@ -1407,10 +1307,11 @@ virNetworkObjListForEachHelper(void *payload,
                                const void *name ATTRIBUTE_UNUSED,
                                void *opaque)
 {
-    struct virNetworkObjListForEachHelperData *data = opaque;
+    virObjectLookupHashForEachDataPtr data = opaque;
+    struct virNetworkObjListForEachHelperData *helperData = data->opaque;
 
-    if (data->callback(payload, data->opaque) < 0)
-        data->ret = -1;
+    if (helperData->callback(payload, helperData->opaque) < 0)
+        helperData->ret = -1;
     return 0;
 }
 
@@ -1433,54 +1334,45 @@ virNetworkObjListForEach(virNetworkObjListPtr nets,
                          virNetworkObjListIterator callback,
                          void *opaque)
 {
-    struct virNetworkObjListForEachHelperData data = {
+    struct virNetworkObjListForEachHelperData helperData = {
         .callback = callback, .opaque = opaque, .ret = 0};
-    virObjectLock(nets);
-    virHashForEach(nets->objs, virNetworkObjListForEachHelper, &data);
-    virObjectUnlock(nets);
-    return data.ret;
-}
+    virObjectLookupHashForEachData data = {
+        .opaque = &helperData, .error = false, .maxElems = 0 };
 
+    return virObjectLookupHashForEach(nets, virNetworkObjListForEachHelper,
+                                      &data);
+}
 
-struct virNetworkObjListGetHelperData {
-    virConnectPtr conn;
-    virNetworkObjListFilter filter;
-    char **names;
-    int nnames;
-    int maxnames;
-    bool active;
-    bool error;
-};
 
 static int
 virNetworkObjListGetHelper(void *payload,
                            const void *name ATTRIBUTE_UNUSED,
                            void *opaque)
 {
-    struct virNetworkObjListGetHelperData *data = opaque;
     virNetworkObjPtr obj = payload;
+    virObjectLookupHashForEachDataPtr data = opaque;
+    virNetworkObjListFilter filter = data->filter;
+    char **names = (char **)data->elems;
 
     if (data->error)
         return 0;
 
-    if (data->maxnames >= 0 &&
-        data->nnames == data->maxnames)
+    if (data->maxElems >= 0 &&
+        data->nElems == data->maxElems)
         return 0;
 
     virObjectLock(obj);
 
-    if (data->filter &&
-        !data->filter(data->conn, obj->def))
+    if (filter && !filter(data->conn, obj->def))
         goto cleanup;
 
-    if ((data->active && virNetworkObjIsActive(obj)) ||
-        (!data->active && !virNetworkObjIsActive(obj))) {
-        if (data->names &&
-            VIR_STRDUP(data->names[data->nnames], obj->def->name) < 0) {
+    if ((data->wantActive && virNetworkObjIsActive(obj)) ||
+        (!data->wantActive && !virNetworkObjIsActive(obj))) {
+        if (names && VIR_STRDUP(names[data->nElems], obj->def->name)
< 0) {
             data->error = true;
             goto cleanup;
         }
-        data->nnames++;
+        data->nElems++;
     }
 
  cleanup:
@@ -1497,26 +1389,11 @@ virNetworkObjListGetNames(virNetworkObjListPtr nets,
                           virNetworkObjListFilter filter,
                           virConnectPtr conn)
 {
-    int ret = -1;
-
-    struct virNetworkObjListGetHelperData data = {
-        .conn = conn, .filter = filter, .names = names, .nnames = 0,
-        .maxnames = maxnames, .active = active, .error = false};
-
-    virObjectLock(nets);
-    virHashForEach(nets->objs, virNetworkObjListGetHelper, &data);
-    virObjectUnlock(nets);
-
-    if (data.error)
-        goto cleanup;
+    virObjectLookupHashForEachData data = {
+        .conn = conn, .filter = filter, .wantActive = active, .error = false,
+        .nElems = 0, .elems = (void **)names, .maxElems = maxnames };
 
-    ret = data.nnames;
- cleanup:
-    if (ret < 0) {
-        while (data.nnames)
-            VIR_FREE(data.names[--data.nnames]);
-    }
-    return ret;
+    return virObjectLookupHashForEach(nets, virNetworkObjListGetHelper, &data);
 }
 
 
@@ -1526,15 +1403,11 @@ virNetworkObjListNumOfNetworks(virNetworkObjListPtr nets,
                                virNetworkObjListFilter filter,
                                virConnectPtr conn)
 {
-    struct virNetworkObjListGetHelperData data = {
-        .conn = conn, .filter = filter, .names = NULL, .nnames = 0,
-        .maxnames = -1, .active = active, .error = false};
-
-    virObjectLock(nets);
-    virHashForEach(nets->objs, virNetworkObjListGetHelper, &data);
-    virObjectUnlock(nets);
+    virObjectLookupHashForEachData data = {
+        .conn = conn, .filter = filter, .wantActive = active, .error = false,
+        .nElems = 0, .elems = NULL, .maxElems = -2 };
 
-    return data.nnames;
+    return virObjectLookupHashForEach(nets, virNetworkObjListGetHelper, &data);
 }
 
 
@@ -1572,7 +1445,5 @@ virNetworkObjListPrune(virNetworkObjListPtr nets,
 {
     struct virNetworkObjListPruneHelperData data = {flags};
 
-    virObjectLock(nets);
-    virHashRemoveSet(nets->objs, virNetworkObjListPruneHelper, &data);
-    virObjectUnlock(nets);
+    virObjectLookupHashPrune(nets, virNetworkObjListPruneHelper, &data);
 }
diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h
index 627277b..050a184 100644
--- a/src/conf/virnetworkobj.h
+++ b/src/conf/virnetworkobj.h
@@ -28,7 +28,8 @@ typedef struct _virNetworkObj virNetworkObj;
 typedef virNetworkObj *virNetworkObjPtr;
 
 virNetworkObjPtr
-virNetworkObjNew(void);
+virNetworkObjNew(const char *uuidstr,
+                 const char *name);
 
 virNetworkDefPtr
 virNetworkObjGetDef(virNetworkObjPtr obj);
diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c
index 4251a22..1a27aab 100644
--- a/tests/networkxml2conftest.c
+++ b/tests/networkxml2conftest.c
@@ -28,11 +28,13 @@ testCompareXMLToConfFiles(const char *inxml, const char *outconf,
dnsmasqCapsPtr
     virCommandPtr cmd = NULL;
     char *pidfile = NULL;
     dnsmasqContext *dctx = NULL;
+    char uuidstr[VIR_UUID_STRING_BUFLEN];
 
     if (!(def = virNetworkDefParseFile(inxml)))
         goto fail;
 
-    if (!(obj = virNetworkObjNew()))
+    virUUIDFormat(def->uuid, uuidstr);
+    if (!(obj = virNetworkObjNew(uuidstr, def->name)))
         goto fail;
 
     virNetworkObjSetDef(obj, def);
-- 
2.9.4


    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

[libvirt] [PATCH v4 17/17] network: Use virObjectLookup{Keys|Hash}