[libvirt] [PATCH v3 15/16] interface: Use virObjectLookupHash

Convert the code to use the LookupHash object and APIs rather than the local forward linked list processing. While this could have been done function by function (for easier review) - converting to the LookupHash would have required changes to the Add, Remove, Find, and List functions anyway in order to properly search hash tables - so rather than piecemeal those changes only to undo them, just use the object. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/conf/virinterfaceobj.c | 321 ++++++++++++++++++++++++++------------------- 1 file changed, 189 insertions(+), 132 deletions(-) diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c index 69a716b..053a5e5 100644 --- a/src/conf/virinterfaceobj.c +++ b/src/conf/virinterfaceobj.c @@ -39,8 +39,7 @@ struct _virInterfaceObj { }; struct _virInterfaceObjList { - size_t count; - virInterfaceObjPtr *objs; + virObjectLookupHash parent; }; /* virInterfaceObj manipulation */ @@ -128,11 +127,40 @@ virInterfaceObjSetActive(virInterfaceObjPtr obj, virInterfaceObjListPtr virInterfaceObjListNew(void) { - virInterfaceObjListPtr interfaces; + return virObjectLookupHashNew(virClassForObjectLookupHash(), 10); +} - if (VIR_ALLOC(interfaces) < 0) - return NULL; - return interfaces; + +static int +virInterfaceObjListFindByMACStringCallback(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 +170,23 @@ 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; + virObjectLookupHashForEachData data = { + .error = false, .matchstr = mac, .nelems = 0, + .elems = (void **)matches, .maxelems = maxmatches }; - 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; + return virObjectLookupHashForEach(interfaces, false, + virInterfaceObjListFindByMACStringCallback, + &data); +} - error: - while (--matchct >= 0) - VIR_FREE(matches[matchct]); - return -1; +static virInterfaceObjPtr +virInterfaceObjListFindByNameLocked(virInterfaceObjListPtr interfaces, + const char *name) +{ + virObjectLookupKeysPtr obj = virObjectLookupHashFind(interfaces, false, + name); + return virObjectRef((virInterfaceObjPtr)obj); } @@ -176,74 +194,96 @@ virInterfaceObjPtr virInterfaceObjListFindByName(virInterfaceObjListPtr interfaces, const char *name) { - size_t i; - - for (i = 0; i < interfaces->count; i++) { - virInterfaceObjPtr obj = interfaces->objs[i]; - virInterfaceDefPtr def; + virInterfaceObjPtr obj; + virObjectLock(interfaces); + obj = virInterfaceObjListFindByNameLocked(interfaces, name); + virObjectUnlock(interfaces); + if (obj) virObjectLock(obj); - def = obj->def; - if (STREQ(def->name, name)) - return virObjectRef(obj); - virObjectUnlock(obj); - } - return NULL; + return obj; } void virInterfaceObjListFree(virInterfaceObjListPtr interfaces) { - size_t i; + virObjectUnref(interfaces); +} + + +struct interfaceCloneData { + const char *primaryKey; + virInterfaceObjListPtr dest; + bool error; +}; + +static int +virInterfaceObjListCloneCallback(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *opaque) +{ + virInterfaceObjPtr srcobj = payload; + struct interfaceCloneData *data = opaque; + int ret = -1; + char *xml = NULL; + virInterfaceObjPtr obj; + virInterfaceDefPtr backup = NULL; + + if (data->error) + return 0; + + virObjectLock(srcobj); + if (!(xml = virInterfaceDefFormat(srcobj->def))) + goto error; + + if (!(backup = virInterfaceDefParseString(xml))) + goto error; + + if (!(obj = virInterfaceObjListAssignDef(data->dest, backup))) + goto error; - for (i = 0; i < interfaces->count; i++) - virObjectUnref(interfaces->objs[i]); - VIR_FREE(interfaces->objs); - VIR_FREE(interfaces); + virInterfaceObjEndAPI(&obj); + ret = 0; + + cleanup: + VIR_FREE(xml); + virInterfaceDefFree(backup); + + return ret; + + error: + data->error = true; + goto cleanup; } virInterfaceObjListPtr virInterfaceObjListClone(virInterfaceObjListPtr interfaces) { - size_t i; - unsigned int cnt; - virInterfaceObjListPtr dest; + virHashTablePtr objsName; + struct interfaceCloneData data = { .error = false }; if (!interfaces) return NULL; - if (!(dest = virInterfaceObjListNew())) + if (!(data.dest = 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); + virObjectLock(interfaces); + objsName = virObjectLookupHashGetName(interfaces); + virHashForEach(objsName, virInterfaceObjListCloneCallback, &data); + virObjectUnlock(interfaces); - if (!xml) - goto error; + if (data.error) + goto cleanup; - if (!(backup = virInterfaceDefParseString(xml))) { - VIR_FREE(xml); - goto error; - } - - VIR_FREE(xml); - if (!(obj = virInterfaceObjListAssignDef(dest, backup))) - goto error; - virInterfaceObjEndAPI(&obj); - } + return data.dest; - return dest; - - error: - virInterfaceObjListFree(dest); - return NULL; + cleanup: + virObjectUnref(data.dest); + return NULL; } @@ -252,24 +292,31 @@ virInterfaceObjListAssignDef(virInterfaceObjListPtr interfaces, virInterfaceDefPtr def) { virInterfaceObjPtr obj; + virInterfaceObjPtr ret = NULL; + + virObjectLock(interfaces); - if ((obj = virInterfaceObjListFindByName(interfaces, def->name))) { + if ((obj = virInterfaceObjListFindByNameLocked(interfaces, def->name))) { + virObjectLock(obj); virInterfaceDefFree(obj->def); obj->def = def; - - return obj; + } else { + if (!(obj = virInterfaceObjNew(def))) + goto cleanup; + + if (virObjectLookupHashAdd(interfaces, + (virObjectLookupKeysPtr)obj) < 0) { + obj->def = NULL; + virInterfaceObjEndAPI(&obj); + goto cleanup; + } } - if (!(obj = virInterfaceObjNew(def))) - return NULL; + ret = obj; - if (VIR_APPEND_ELEMENT_COPY(interfaces->objs, - interfaces->count, obj) < 0) { - obj->def = NULL; - virInterfaceObjEndAPI(&obj); - return NULL; - } - return virObjectRef(obj); + cleanup: + virObjectUnlock(interfaces); + return ret; } @@ -277,20 +324,24 @@ void virInterfaceObjListRemove(virInterfaceObjListPtr interfaces, virInterfaceObjPtr obj) { - size_t i; + virObjectLookupHashRemove(interfaces, (virObjectLookupKeysPtr)obj); +} + + +static int +virInterfaceObjListNumOfInterfacesCb(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *opaque) +{ + virInterfaceObjPtr obj = payload; + virObjectLookupHashForEachDataPtr data = opaque; + virObjectLock(obj); + if (data->wantActive == virInterfaceObjIsActive(obj)) + data->nelems++; 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; - } - virObjectUnlock(interfaces->objs[i]); - } + + return 0; } @@ -298,18 +349,44 @@ int virInterfaceObjListNumOfInterfaces(virInterfaceObjListPtr interfaces, bool wantActive) { - size_t i; - int ninterfaces = 0; + virObjectLookupHashForEachData data = { + .wantActive = wantActive, .nelems = 0, .error = false }; - for (i = 0; (i < interfaces->count); i++) { - virInterfaceObjPtr obj = interfaces->objs[i]; - virObjectLock(obj); - if (wantActive == virInterfaceObjIsActive(obj)) - ninterfaces++; - virObjectUnlock(obj); - } + return virObjectLookupHashForEach(interfaces, false, + virInterfaceObjListNumOfInterfacesCb, + &data); +} + + +static int +virInterfaceObjListGetNamesCb(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 (VIR_STRDUP(names[data->nelems], def->name) < 0) { + data->error = true; + goto cleanup; + } + data->nelems++; + } - return ninterfaces; + cleanup: + virObjectUnlock(obj); + return 0; } @@ -319,30 +396,10 @@ 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 virObjectLookupHashForEach(interfaces, false, + virInterfaceObjListGetNamesCb, &data); } -- 2.9.4