Re: [libvirt] [PATCH v4 3/4] nwfilter: Convert _virNWFilterObjList to use virObjectRWLockable

2 Feb 2018

On 02/02/2018 02:33 PM, John Ferlan wrote:
...
...
On 12/08/2017 09:01 AM, John Ferlan wrote:
...
Implement the self locking object list for nwfilter object lists
that uses two hash tables to store the nwfilter object by UUID or
by Name.
As part of this alter the uuid argument to virNWFilterObjLookupByUUID
to expect an already formatted uuidstr.
Alter the existing list traversal code to implement the hash table
find/lookup/search functionality.
Signed-off-by: John Ferlan <jferlan@redhat.com>
---
   src/conf/virnwfilterobj.c      | 402
++++++++++++++++++++++++++++-------------
   src/conf/virnwfilterobj.h      |   2 +-
   src/nwfilter/nwfilter_driver.c |   5 +-
   3 files changed, 282 insertions(+), 127 deletions(-)

diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
index 6b4758656..a4e6a03d2 100644
--- a/src/conf/virnwfilterobj.c
+++ b/src/conf/virnwfilterobj.c
@@ -43,12 +43,21 @@ struct _virNWFilterObj {
   };
struct _virNWFilterObjList {
-    size_t count;
-    virNWFilterObjPtr *objs;
+    virObjectRWLockable parent;
+
+    /* uuid string -> virNWFilterObj  mapping
+     * for O(1), lockless lookup-by-uuid */
+    virHashTable *objs;
+
+    /* name -> virNWFilterObj mapping for O(1),
+     * lockless lookup-by-name */
+    virHashTable *objsName;
   };
static virClassPtr virNWFilterObjClass;
+static virClassPtr virNWFilterObjListClass;
   static void virNWFilterObjDispose(void *opaque);
+static void virNWFilterObjListDispose(void *opaque);
static int
@@ -60,6 +69,12 @@ virNWFilterObjOnceInit(void)
                                               virNWFilterObjDispose)))
           return -1;
+    if (!(virNWFilterObjListClass =
virClassNew(virClassForObjectRWLockable(),
+                                                "virNWFilterObjList",
+
sizeof(virNWFilterObjList),
+
virNWFilterObjListDispose)))
+        return -1;
+
       return 0;
   }
@@ -144,14 +159,20 @@ virNWFilterObjDispose(void *opaque)
   }
+static void
+virNWFilterObjListDispose(void *opaque)
+{
+    virNWFilterObjListPtr nwfilters = opaque;
+
+    virHashFree(nwfilters->objs);
+    virHashFree(nwfilters->objsName);
+}
+
+
   void
   virNWFilterObjListFree(virNWFilterObjListPtr nwfilters)
   {
-    size_t i;
-    for (i = 0; i < nwfilters->count; i++)
-        virObjectUnref(nwfilters->objs[i]);
-    VIR_FREE(nwfilters->objs);
-    VIR_FREE(nwfilters);
+    virObjectUnref(nwfilters);
   }
@@ -160,8 +181,23 @@ virNWFilterObjListNew(void)
   {
       virNWFilterObjListPtr nwfilters;
-    if (VIR_ALLOC(nwfilters) < 0)
+    if (virNWFilterObjInitialize() < 0)
+        return NULL;
+
+    if (!(nwfilters = virObjectRWLockableNew(virNWFilterObjListClass)))
+        return NULL;
+
+    if (!(nwfilters->objs = virHashCreate(10, virObjectFreeHashData))) {
[1] As part of the magic of hash tables, when an element is removed from
On 01/31/2018 10:10 PM, Stefan Berger wrote:
the hash table the virObjectFreeHashData is called which does a
virObjectUnref...
Ah. I was looking for 'symmetry' ... so ideally we would have automatic 
increase  of the refcount as well.

    Stefan

    