Re: [libvirt] [PATCH 3/4] storage: Convert virStoragePoolObjList to use virObjectRWLockable

Now that we have a private storage pool list, we can take the next step and convert to using objects. In this case, we're going to use RWLockable objects (just like every other driver) with two hash tables for lookup by UUID or Name. Along the way the ForEach and Search API's will be adjusted to use the related Hash API's and the various FindBy functions altered and augmented to allow for HashLookup w/ and w/o the pool lock already taken. After virStoragePoolObjRemove we will need to virObjectUnref(obj) after to indicate the caller is "done" with it's reference. The Unlock occurs during the Remove. The NumOf, GetNames, and Export functions all have their own callback functions to return the required data and the FindDuplicate code can use the HashSearch function callbacks. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/conf/virstorageobj.c | 638 +++++++++++++++++++++++++++++-------------- src/conf/virstorageobj.h | 3 - src/libvirt_private.syms | 1 - src/storage/storage_driver.c | 8 +- src/test/test_driver.c | 7 +- 5 files changed, 449 insertions(+), 208 deletions(-) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index a48346b24..0e5c98bf7 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -27,6 +27,7 @@ #include "viralloc.h" #include "virerror.h" #include "virfile.h" +#include "virhash.h" #include "virlog.h" #include "virscsihost.h" #include "virstring.h" @@ -37,9 +38,12 @@ VIR_LOG_INIT("conf.virstorageobj"); static virClassPtr virStoragePoolObjClass; +static virClassPtr virStoragePoolObjListClass; static void virStoragePoolObjDispose(void *opaque); +static void +virStoragePoolObjListDispose(void *opaque); struct _virStorageVolDefList { @@ -63,8 +67,15 @@ struct _virStoragePoolObj { }; struct _virStoragePoolObjList { - size_t count; - virStoragePoolObjPtr *objs; + virObjectRWLockable parent; + + /* uuid string -> virStoragePoolObj mapping + * for (1), lockless lookup-by-uuid */ + virHashTable *objs; + + /* name string -> virStoragePoolObj mapping + * for (1), lockless lookup-by-name */ + virHashTable *objsName; }; @@ -77,6 +88,12 @@ virStoragePoolObjOnceInit(void) virStoragePoolObjDispose))) return -1; + if (!(virStoragePoolObjListClass = virClassNew(virClassForObjectRWLockable(), + "virStoragePoolObjList", + sizeof(virStoragePoolObjList), + virStoragePoolObjListDispose))) + return -1; + return 0; } @@ -240,13 +257,15 @@ virStoragePoolObjDispose(void *opaque) void -virStoragePoolObjListFree(virStoragePoolObjListPtr pools) +virStoragePoolObjListDispose(void *opaque) { - size_t i; - for (i = 0; i < pools->count; i++) - virObjectUnref(pools->objs[i]); - VIR_FREE(pools->objs); - VIR_FREE(pools); + virStoragePoolObjListPtr pools = opaque; + + if (!pools) + return;

+ + virHashFree(pools->objs); + virHashFree(pools->objsName); } @@ -255,13 +274,44 @@ virStoragePoolObjListNew(void) { virStoragePoolObjListPtr pools; - if (VIR_ALLOC(pools) < 0) + if (virStoragePoolObjInitialize() < 0) + return NULL; + + if (!(pools = virObjectRWLockableNew(virStoragePoolObjListClass))) + return NULL; + + if (!(pools->objs = virHashCreate(20, virObjectFreeHashData)) || + !(pools->objsName = virHashCreate(20, virObjectFreeHashData))) { + virObjectUnref(pools); return NULL; + } return pools; } +struct _virStoragePoolObjListForEachData { + virStoragePoolObjListIterator iter; + const void *opaque; +}; + +static int +virStoragePoolObjListForEachCb(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *opaque) +{ + virStoragePoolObjPtr obj = payload; + struct _virStoragePoolObjListForEachData *data = + (struct _virStoragePoolObjListForEachData *)opaque;

+ + virObjectLock(obj); + data->iter(obj, data->opaque); + virObjectUnlock(obj); + + return 0; +} + + /** * virStoragePoolObjListForEach * @pools: Pointer to pools object @@ -279,15 +329,35 @@ virStoragePoolObjListForEach(virStoragePoolObjListPtr pools, virStoragePoolObjListIterator iter, const void *opaque) { - size_t i; - virStoragePoolObjPtr obj; + struct _virStoragePoolObjListForEachData data = { .iter = iter, + .opaque = opaque }; - for (i = 0; i < pools->count; i++) { - obj = pools->objs[i]; - virObjectLock(obj); - iter(obj, opaque); - virObjectUnlock(obj); - } + virObjectRWLockRead(pools); + virHashForEach(pools->objs, virStoragePoolObjListForEachCb, &data); + virObjectRWUnlock(pools); +} + + +struct _virStoragePoolObjListSearchData { + virStoragePoolObjListSearcher searcher; + const void *opaque; +}; + + +static int +virStoragePoolObjListSearchCb(const void *payload, + const void *name ATTRIBUTE_UNUSED, + const void *opaque) +{ + virStoragePoolObjPtr obj = (virStoragePoolObjPtr) payload; + struct _virStoragePoolObjListSearchData *data = + (struct _virStoragePoolObjListSearchData *)opaque;

+ + virObjectLock(obj); + if (data->searcher(obj, data->opaque)) + return 1; + virObjectUnlock(obj); + return 0; }