On 06/02/2017 12:25 PM, John Ferlan wrote:
Perhaps a bit out of order from the normal convert driver object to virObjectLockable, then convert the driver object list. However, as it turns out nwfilter objects have been using a recursive mutex for which the common virObject code does not want to use.
So, if we convert the nwfilter object list to use virObjectLockable, then it will be more possible to make the necessary adjustments to the virNWFilterObjListFindInstantiateFilter algorithm in order to handle a recursive lock operation required as part of the <rule> and <filterref> (or "inc" list) processing.
This patch takes the plunge, modifying the nwfilter object list handling code to utilize hash tables for both the UUID and Name for which an nwfilter def would utilize. This makes lookup by either "key" possible without needing to first lock the object in order to find a match as long as of course the object list itself is locked.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/virnwfilterobj.c | 395 +++++++++++++++++++++++++++++------------ src/nwfilter/nwfilter_driver.c | 4 + 2 files changed, 286 insertions(+), 113 deletions(-)
diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index 99be59c..84ef7d1 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -53,10 +53,34 @@ struct _virNWFilterObj { };
struct _virNWFilterObjList { - size_t count; - virNWFilterObjPtr *objs; + virObjectLockable 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 virNWFilterObjListClass; +static void virNWFilterObjListDispose(void *opaque); + +static int +virNWFilterObjOnceInit(void) +{ + if (!(virNWFilterObjListClass = virClassNew(virClassForObjectLockable(), + "virNWFilterObjList", + sizeof(virNWFilterObjList), + virNWFilterObjListDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virNWFilterObj) +
static virNWFilterObjPtr virNWFilterObjNew(virNWFilterDefPtr def) @@ -151,14 +175,30 @@ virNWFilterObjUnref(virNWFilterObjPtr obj) }
+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++) - virNWFilterObjUnref(nwfilters->objs[i]); - VIR_FREE(nwfilters->objs); - VIR_FREE(nwfilters); + virObjectUnref(nwfilters); +} + + +static void +virNWFilterObjListObjsFreeData(void *payload, + const void *name ATTRIBUTE_UNUSED) +{ + virNWFilterObjPtr obj = payload; + + virNWFilterObjUnref(obj); }
@@ -167,8 +207,24 @@ virNWFilterObjListNew(void) { virNWFilterObjListPtr nwfilters;
- if (VIR_ALLOC(nwfilters) < 0) + if (virNWFilterObjInitialize() < 0) + return NULL; + + if (!(nwfilters = virObjectLockableNew(virNWFilterObjListClass))) + return NULL; + + if (!(nwfilters->objs = virHashCreate(10, virNWFilterObjListObjsFreeData))) { + virObjectUnref(nwfilters); + return NULL; + } + + if (!(nwfilters->objsName = + virHashCreate(10, virNWFilterObjListObjsFreeData))) {
Again, 86 characters is not that much ;-)
+ virHashFree(nwfilters->objs); + virObjectUnref(nwfilters); return NULL; + } + return nwfilters; }
Otherwise looking good. ACK Michal