Now that we're moved the object into virstorageobj, let's make the code use the lockable object. Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> --- src/conf/virstorageobj.c | 136 +++++++++++++++++++++-------------------- src/conf/virstorageobj.h | 3 - src/libvirt_private.syms | 1 - src/storage/storage_driver.c | 18 +++--- tests/storagevolxml2argvtest.c | 1 - 5 files changed, 77 insertions(+), 82 deletions(-) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index eb8a57324b..357e6a967e 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -36,8 +36,10 @@ VIR_LOG_INIT("conf.virstorageobj"); +static virClassPtr virStoragePoolObjClass; + static void -virStoragePoolObjUnlock(virStoragePoolObjPtr obj); +virStoragePoolObjDispose(void *opaque); struct _virStorageVolDefList { @@ -46,7 +48,7 @@ struct _virStorageVolDefList { }; struct _virStoragePoolObj { - virMutex lock; + virObjectLockable parent; char *configFile; char *autostartLink; @@ -60,21 +62,34 @@ struct _virStoragePoolObj { virStorageVolDefList volumes; }; + +static int +virStoragePoolObjOnceInit(void) +{ + if (!(virStoragePoolObjClass = virClassNew(virClassForObjectLockable(), + "virStoragePoolObj", + sizeof(virStoragePoolObj), + virStoragePoolObjDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virStoragePoolObj) + + virStoragePoolObjPtr virStoragePoolObjNew(void) { virStoragePoolObjPtr obj; - if (VIR_ALLOC(obj) < 0) + if (virStoragePoolObjInitialize() < 0) return NULL; - if (virMutexInit(&obj->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot initialize mutex")); - VIR_FREE(obj); + if (!(obj = virObjectLockableNew(virStoragePoolObjClass))) return NULL; - } - virStoragePoolObjLock(obj); + + virObjectLock(obj); obj->active = false; return obj; } @@ -86,7 +101,9 @@ virStoragePoolObjEndAPI(virStoragePoolObjPtr *obj) if (!*obj) return; - virStoragePoolObjUnlock(*obj); + virObjectUnlock(*obj); + virObjectUnref(*obj); + *obj = NULL; } @@ -200,8 +217,10 @@ virStoragePoolObjDecrAsyncjobs(virStoragePoolObjPtr obj) void -virStoragePoolObjFree(virStoragePoolObjPtr obj) +virStoragePoolObjDispose(void *opaque) { + virStoragePoolObjPtr obj = opaque; + if (!obj) return; @@ -212,10 +231,6 @@ virStoragePoolObjFree(virStoragePoolObjPtr obj) VIR_FREE(obj->configFile); VIR_FREE(obj->autostartLink); - - virMutexDestroy(&obj->lock); - - VIR_FREE(obj); } @@ -224,7 +239,7 @@ virStoragePoolObjListFree(virStoragePoolObjListPtr pools) { size_t i; for (i = 0; i < pools->count; i++) - virStoragePoolObjFree(pools->objs[i]); + virObjectUnref(pools->objs[i]); VIR_FREE(pools->objs); pools->count = 0; } @@ -252,9 +267,9 @@ virStoragePoolObjListForEach(virStoragePoolObjListPtr pools, for (i = 0; i < pools->count; i++) { obj = pools->objs[i]; - virStoragePoolObjLock(obj); + virObjectLock(obj); iter(obj, opaque); - virStoragePoolObjUnlock(obj); + virObjectUnlock(obj); } } @@ -269,7 +284,7 @@ virStoragePoolObjListForEach(virStoragePoolObjListPtr pools, * the @opaque data in order to find an object that matches some criteria * and return that object locked. * - * Returns a locked object when found and NULL when not found + * Returns a locked and reffed object when found and NULL when not found */ virStoragePoolObjPtr virStoragePoolObjListSearch(virStoragePoolObjListPtr pools, @@ -281,10 +296,10 @@ virStoragePoolObjListSearch(virStoragePoolObjListPtr pools, for (i = 0; i < pools->count; i++) { obj = pools->objs[i]; - virStoragePoolObjLock(obj); + virObjectLock(obj); if (searcher(obj, opaque)) - return obj; - virStoragePoolObjUnlock(obj); + return virObjectRef(obj); + virObjectUnlock(obj); } return NULL; @@ -297,18 +312,18 @@ virStoragePoolObjRemove(virStoragePoolObjListPtr pools, { size_t i; - virStoragePoolObjUnlock(obj); + virObjectUnlock(obj); for (i = 0; i < pools->count; i++) { - virStoragePoolObjLock(pools->objs[i]); + virObjectLock(pools->objs[i]); if (pools->objs[i] == obj) { - virStoragePoolObjUnlock(pools->objs[i]); - virStoragePoolObjFree(pools->objs[i]); + virObjectUnlock(pools->objs[i]); + virObjectUnref(pools->objs[i]); VIR_DELETE_ELEMENT(pools->objs, i, pools->count); break; } - virStoragePoolObjUnlock(pools->objs[i]); + virObjectUnlock(pools->objs[i]); } } @@ -320,10 +335,12 @@ virStoragePoolObjFindByUUID(virStoragePoolObjListPtr pools, size_t i; for (i = 0; i < pools->count; i++) { - virStoragePoolObjLock(pools->objs[i]); - if (!memcmp(pools->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN)) - return pools->objs[i]; - virStoragePoolObjUnlock(pools->objs[i]); + virStoragePoolObjPtr obj = pools->objs[i]; + + virObjectLock(obj); + if (!memcmp(obj->def->uuid, uuid, VIR_UUID_BUFLEN)) + return virObjectRef(obj); + virObjectUnlock(obj); } return NULL; @@ -337,10 +354,12 @@ virStoragePoolObjFindByName(virStoragePoolObjListPtr pools, size_t i; for (i = 0; i < pools->count; i++) { - virStoragePoolObjLock(pools->objs[i]); - if (STREQ(pools->objs[i]->def->name, name)) - return pools->objs[i]; - virStoragePoolObjUnlock(pools->objs[i]); + virStoragePoolObjPtr obj = pools->objs[i]; + + virObjectLock(obj); + if (STREQ(obj->def->name, name)) + return virObjectRef(obj); + virObjectUnlock(obj); } return NULL; @@ -608,13 +627,12 @@ virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools, return NULL; if (VIR_APPEND_ELEMENT_COPY(pools->objs, pools->count, obj) < 0) { - virStoragePoolObjUnlock(obj); - virStoragePoolObjFree(obj); + virStoragePoolObjEndAPI(&obj); return NULL; } obj->def = def; - return obj; + return virObjectRef(obj); } @@ -741,7 +759,7 @@ virStoragePoolObjLoadAllState(virStoragePoolObjListPtr pools, if (!(obj = virStoragePoolObjLoadState(pools, stateDir, entry->d_name))) continue; - virStoragePoolObjUnlock(obj); + virStoragePoolObjEndAPI(&obj); } VIR_DIR_CLOSE(dir); @@ -780,8 +798,7 @@ virStoragePoolObjLoadAllConfigs(virStoragePoolObjListPtr pools, } obj = virStoragePoolObjLoad(pools, entry->d_name, path, autostartLink); - if (obj) - virStoragePoolObjUnlock(obj); + virStoragePoolObjEndAPI(&obj); VIR_FREE(path); VIR_FREE(autostartLink); @@ -852,12 +869,12 @@ virStoragePoolObjNumOfStoragePools(virStoragePoolObjListPtr pools, for (i = 0; i < pools->count; i++) { virStoragePoolObjPtr obj = pools->objs[i]; - virStoragePoolObjLock(obj); + virObjectLock(obj); if (!filter || filter(conn, obj->def)) { if (wantActive == virStoragePoolObjIsActive(obj)) npools++; } - virStoragePoolObjUnlock(obj); + virObjectUnlock(obj); } return npools; @@ -877,17 +894,17 @@ virStoragePoolObjGetNames(virStoragePoolObjListPtr pools, for (i = 0; i < pools->count && nnames < maxnames; i++) { virStoragePoolObjPtr obj = pools->objs[i]; - virStoragePoolObjLock(obj); + virObjectLock(obj); if (!filter || filter(conn, obj->def)) { if (wantActive == virStoragePoolObjIsActive(obj)) { if (VIR_STRDUP(names[nnames], obj->def->name) < 0) { - virStoragePoolObjUnlock(obj); + virObjectUnlock(obj); goto failure; } nnames++; } } - virStoragePoolObjUnlock(obj); + virObjectUnlock(obj); } return nnames; @@ -957,8 +974,7 @@ virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools, } cleanup: - if (obj) - virStoragePoolObjUnlock(obj); + virStoragePoolObjEndAPI(&obj); return ret; } @@ -1273,7 +1289,7 @@ virStoragePoolObjSourceFindDuplicate(virConnectPtr conn, if (STREQ(obj->def->name, def->name)) continue; - virStoragePoolObjLock(obj); + virObjectLock(obj); switch ((virStoragePoolType)obj->def->type) { case VIR_STORAGE_POOL_DIR: @@ -1325,7 +1341,7 @@ virStoragePoolObjSourceFindDuplicate(virConnectPtr conn, case VIR_STORAGE_POOL_LAST: break; } - virStoragePoolObjUnlock(obj); + virObjectUnlock(obj); if (matchobj) break; @@ -1341,20 +1357,6 @@ virStoragePoolObjSourceFindDuplicate(virConnectPtr conn, } -void -virStoragePoolObjLock(virStoragePoolObjPtr obj) -{ - virMutexLock(&obj->lock); -} - - -static void -virStoragePoolObjUnlock(virStoragePoolObjPtr obj) -{ - virMutexUnlock(&obj->lock); -} - - #define MATCH(FLAG) (flags & (FLAG)) static bool virStoragePoolMatch(virStoragePoolObjPtr obj, @@ -1438,7 +1440,7 @@ virStoragePoolObjListExport(virConnectPtr conn, for (i = 0; i < poolobjs->count; i++) { virStoragePoolObjPtr obj = poolobjs->objs[i]; - virStoragePoolObjLock(obj); + virObjectLock(obj); if ((!filter || filter(conn, obj->def)) && virStoragePoolMatch(obj, flags)) { if (pools) { @@ -1446,14 +1448,14 @@ virStoragePoolObjListExport(virConnectPtr conn, obj->def->name, obj->def->uuid, NULL, NULL))) { - virStoragePoolObjUnlock(obj); + virObjectUnlock(obj); goto cleanup; } tmp_pools[npools] = pool; } npools++; } - virStoragePoolObjUnlock(obj); + virObjectUnlock(obj); } if (tmp_pools) { diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h index 7fe4a9f68a..34c4c9e10b 100644 --- a/src/conf/virstorageobj.h +++ b/src/conf/virstorageobj.h @@ -258,9 +258,6 @@ virStoragePoolObjSourceFindDuplicate(virConnectPtr conn, virStoragePoolObjListPtr pools, virStoragePoolDefPtr def); -void -virStoragePoolObjLock(virStoragePoolObjPtr obj); - int virStoragePoolObjListExport(virConnectPtr conn, virStoragePoolObjListPtr poolobjs, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e4aebb3ca5..e408df8671 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1094,7 +1094,6 @@ virStoragePoolObjListFree; virStoragePoolObjListSearch; virStoragePoolObjLoadAllConfigs; virStoragePoolObjLoadAllState; -virStoragePoolObjLock; virStoragePoolObjNew; virStoragePoolObjNumOfStoragePools; virStoragePoolObjNumOfVolumes; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 033196dc95..e19b5a2071 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1768,7 +1768,6 @@ virStorageVolDefFromVol(virStorageVolPtr vol, error: virStoragePoolObjEndAPI(obj); - *obj = NULL; return NULL; } @@ -1901,14 +1900,14 @@ storageVolCreateXML(virStoragePoolPtr pool, /* Drop the pool lock during volume allocation */ virStoragePoolObjIncrAsyncjobs(obj); voldef->building = true; - virStoragePoolObjEndAPI(&obj); + virObjectUnlock(obj); buildret = backend->buildVol(pool->conn, obj, buildvoldef, flags); VIR_FREE(buildvoldef); storageDriverLock(); - virStoragePoolObjLock(obj); + virObjectLock(obj); storageDriverUnlock(); voldef->building = false; @@ -1976,9 +1975,9 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, storageDriverLock(); obj = virStoragePoolObjFindByUUID(&driver->pools, pool->uuid); if (obj && STRNEQ(pool->name, volsrc->pool)) { - virStoragePoolObjEndAPI(&obj); + virObjectUnlock(obj); objsrc = virStoragePoolObjFindByName(&driver->pools, volsrc->pool); - virStoragePoolObjLock(obj); + virObjectLock(obj); } storageDriverUnlock(); if (!obj) { @@ -2097,19 +2096,19 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, virStoragePoolObjIncrAsyncjobs(obj); voldef->building = true; voldefsrc->in_use++; - virStoragePoolObjEndAPI(&obj); + virObjectUnlock(obj); if (objsrc) { virStoragePoolObjIncrAsyncjobs(objsrc); - virStoragePoolObjEndAPI(&objsrc); + virObjectUnlock(objsrc); } buildret = backend->buildVolFrom(pool->conn, obj, shadowvol, voldefsrc, flags); storageDriverLock(); - virStoragePoolObjLock(obj); + virObjectLock(obj); if (objsrc) - virStoragePoolObjLock(objsrc); + virObjectLock(objsrc); storageDriverUnlock(); voldefsrc->in_use--; @@ -2119,7 +2118,6 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, if (objsrc) { virStoragePoolObjDecrAsyncjobs(objsrc); virStoragePoolObjEndAPI(&objsrc); - objsrc = NULL; } if (buildret < 0 || diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index 0b2f2bb3d3..1cd083766a 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -111,7 +111,6 @@ testCompareXMLToArgvFiles(bool shouldFail, virCommandFree(cmd); VIR_FREE(actualCmdline); virStoragePoolObjEndAPI(&obj); - virStoragePoolObjFree(obj); virObjectUnref(conn); return ret; } -- 2.13.6 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list