From: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com&gt; The test driver state (@testDriver) uses it's own reference counting and locking implementation. Instead of doing that, convert @testDriver into a virObjectLockable and use the provided functionalities. Signed-off-by: Marc Hartmayer <mhartmay(a)linux.ibm.com&gt; Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; --- This patch was originally posted with the patch series "[libvirt] [PATCH libvirt v2 0/9] Fix virConnectRegisterCloseCallback and get rid of global variables" (mid:20180412124104.10547-1-mhartmay@linux.vnet.ibm.com). I dropped the r-b from John because so much time has passed. --- src/test/test_driver.c | 198 ++++++++++++++++++----------------------- 1 file changed, 87 insertions(+), 111 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 31d0da744ce5..b76f0b718ecd 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -92,7 +92,7 @@ typedef struct _testAuth testAuth; typedef struct _testAuth *testAuthPtr; struct _testDriver { - virMutex lock; + virObjectLockable parent; virNodeInfo nodeInfo; virInterfaceObjListPtr ifaces; @@ -124,9 +124,19 @@ typedef struct _testDriver testDriver; typedef testDriver *testDriverPtr; static testDriverPtr defaultPrivconn; -static int defaultConnections; static virMutex defaultLock = VIR_MUTEX_INITIALIZER; +static virClassPtr testDriverClass; +static void testDriverDispose(void *obj); +static int testDriverOnceInit(void) +{ + if (!(VIR_CLASS_NEW(testDriver, virClassForObjectLockable()))) + return -1; + + return 0; +} +VIR_ONCE_GLOBAL_INIT(testDriver) + #define TEST_MODEL "i686" #define TEST_EMULATOR "/usr/bin/test-hv" @@ -142,10 +152,9 @@ static const virNodeInfo defaultNodeInfo = { }; static void -testDriverFree(testDriverPtr driver) +testDriverDispose(void *obj) { - if (!driver) - return; + testDriverPtr driver = obj; virObjectUnref(driver->caps); virObjectUnref(driver->xmlopt); @@ -155,21 +164,6 @@ testDriverFree(testDriverPtr driver) virObjectUnref(driver->ifaces); virObjectUnref(driver->pools); virObjectUnref(driver->eventState); - virMutexUnlock(&driver->lock); - virMutexDestroy(&driver->lock); - - VIR_FREE(driver); -} - - -static void testDriverLock(testDriverPtr driver) -{ - virMutexLock(&driver->lock); -} - -static void testDriverUnlock(testDriverPtr driver) -{ - virMutexUnlock(&driver->lock); } #define TEST_NAMESPACE_HREF "http://libvirt.org/schemas/domain/test/1.0" @@ -401,14 +395,11 @@ testDriverNew(void) }; testDriverPtr ret; - if (VIR_ALLOC(ret) < 0) + if (testDriverInitialize() < 0) return NULL; - if (virMutexInit(&ret->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot initialize mutex")); - goto error; - } + if (!(ret = virObjectLockableNew(testDriverClass))) + return NULL; if (!(ret->xmlopt = virDomainXMLOptionNew(NULL, NULL, &ns, NULL, NULL)) || !(ret->eventState = virObjectEventStateNew()) || @@ -424,7 +415,7 @@ testDriverNew(void) return ret; error: - testDriverFree(ret); + virObjectUnref(ret); return NULL; } @@ -1256,7 +1247,7 @@ testOpenFromFile(virConnectPtr conn, const char *file) if (!(privconn = testDriverNew())) return VIR_DRV_OPEN_ERROR; - testDriverLock(privconn); + virObjectLock(privconn); conn->privateData = privconn; if (!(privconn->caps = testBuildCapabilities(conn))) @@ -1273,14 +1264,14 @@ testOpenFromFile(virConnectPtr conn, const char *file) xmlXPathFreeContext(ctxt); xmlFreeDoc(doc); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return VIR_DRV_OPEN_SUCCESS; error: xmlXPathFreeContext(ctxt); xmlFreeDoc(doc); - testDriverFree(privconn); + virObjectUnref(privconn); conn->privateData = NULL; return VIR_DRV_OPEN_ERROR; } @@ -1298,8 +1289,8 @@ testOpenDefault(virConnectPtr conn) size_t i; virMutexLock(&defaultLock); - if (defaultConnections++) { - conn->privateData = defaultPrivconn; + if (defaultPrivconn) { + conn->privateData = virObjectRef(defaultPrivconn); virMutexUnlock(&defaultLock); return VIR_DRV_OPEN_SUCCESS; } @@ -1348,9 +1339,8 @@ testOpenDefault(virConnectPtr conn) return ret; error: - testDriverFree(privconn); + virObjectUnref(privconn); conn->privateData = NULL; - defaultConnections--; goto cleanup; } @@ -1363,9 +1353,9 @@ testConnectAuthenticate(virConnectPtr conn, ssize_t i; char *username = NULL, *password = NULL; - testDriverLock(privconn); + virObjectLock(privconn); if (privconn->numAuths == 0) { - testDriverUnlock(privconn); + virObjectUnlock(privconn); return 0; } @@ -1400,7 +1390,7 @@ testConnectAuthenticate(virConnectPtr conn, ret = 0; cleanup: - testDriverUnlock(privconn); + virObjectUnlock(privconn); VIR_FREE(username); VIR_FREE(password); return ret; @@ -1410,24 +1400,11 @@ testConnectAuthenticate(virConnectPtr conn, static void testDriverCloseInternal(testDriverPtr driver) { - bool dflt = false; - - if (driver == defaultPrivconn) { - dflt = true; - virMutexLock(&defaultLock); - if (--defaultConnections) { - virMutexUnlock(&defaultLock); - return; - } - } - - testDriverLock(driver); - testDriverFree(driver); - - if (dflt) { + virMutexLock(&defaultLock); + bool disposed = !virObjectUnref(driver); + if (disposed && driver == defaultPrivconn) defaultPrivconn = NULL; - virMutexUnlock(&defaultLock); - } + virMutexUnlock(&defaultLock); } @@ -1546,9 +1523,9 @@ static int testNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) { testDriverPtr privconn = conn->privateData; - testDriverLock(privconn); + virObjectLock(privconn); memcpy(info, &privconn->nodeInfo, sizeof(virNodeInfo)); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return 0; } @@ -1556,9 +1533,9 @@ static char *testConnectGetCapabilities(virConnectPtr conn) { testDriverPtr privconn = conn->privateData; char *xml; - testDriverLock(privconn); + virObjectLock(privconn); xml = virCapabilitiesFormatXML(privconn->caps); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return xml; } @@ -1593,9 +1570,9 @@ static int testConnectNumOfDomains(virConnectPtr conn) testDriverPtr privconn = conn->privateData; int count; - testDriverLock(privconn); + virObjectLock(privconn); count = virDomainObjListNumOfDomains(privconn->domains, true, NULL, NULL); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return count; } @@ -1648,7 +1625,7 @@ testDomainCreateXML(virConnectPtr conn, const char *xml, if (flags & VIR_DOMAIN_START_VALIDATE) parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; - testDriverLock(privconn); + virObjectLock(privconn); if ((def = virDomainDefParseString(xml, privconn->caps, privconn->xmlopt, NULL, parse_flags)) == NULL) goto cleanup; @@ -1680,7 +1657,7 @@ testDomainCreateXML(virConnectPtr conn, const char *xml, virDomainObjEndAPI(&dom); virObjectEventStateQueue(privconn->eventState, event); virDomainDefFree(def); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return ret; } @@ -2842,7 +2819,7 @@ static int testNodeGetCellsFreeMemory(virConnectPtr conn, size_t i; int ret = -1; - testDriverLock(privconn); + virObjectLock(privconn); if (startCell >= privconn->numCells) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("Range exceeds available cells")); @@ -2857,7 +2834,7 @@ static int testNodeGetCellsFreeMemory(virConnectPtr conn, ret = i; cleanup: - testDriverUnlock(privconn); + virObjectUnlock(privconn); return ret; } @@ -2915,12 +2892,12 @@ testNodeGetFreeMemory(virConnectPtr conn) unsigned int freeMem = 0; size_t i; - testDriverLock(privconn); + virObjectLock(privconn); for (i = 0; i < privconn->numCells; i++) freeMem += privconn->cells[i].freeMem; - testDriverUnlock(privconn); + virObjectUnlock(privconn); return freeMem; } @@ -2957,7 +2934,7 @@ static int testDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) virCheckFlags(0, -1); - testDriverLock(privconn); + virObjectLock(privconn); if (!(privdom = testDomObjFromDomain(domain))) goto cleanup; @@ -2981,7 +2958,7 @@ static int testDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) cleanup: virDomainObjEndAPI(&privdom); virObjectEventStateQueue(privconn->eventState, event); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return ret; } @@ -3746,9 +3723,9 @@ testInterfaceObjFindByName(testDriverPtr privconn, { virInterfaceObjPtr obj; - testDriverLock(privconn); + virObjectLock(privconn); obj = virInterfaceObjListFindByName(privconn->ifaces, name); - testDriverUnlock(privconn); + virObjectUnlock(privconn); if (!obj) virReportError(VIR_ERR_NO_INTERFACE, @@ -3765,9 +3742,9 @@ testConnectNumOfInterfaces(virConnectPtr conn) testDriverPtr privconn = conn->privateData; int ninterfaces; - testDriverLock(privconn); + virObjectLock(privconn); ninterfaces = virInterfaceObjListNumOfInterfaces(privconn->ifaces, true); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return ninterfaces; } @@ -3780,10 +3757,10 @@ testConnectListInterfaces(virConnectPtr conn, testDriverPtr privconn = conn->privateData; int nnames; - testDriverLock(privconn); + virObjectLock(privconn); nnames = virInterfaceObjListGetNames(privconn->ifaces, true, names, maxnames); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return nnames; } @@ -3795,9 +3772,9 @@ testConnectNumOfDefinedInterfaces(virConnectPtr conn) testDriverPtr privconn = conn->privateData; int ninterfaces; - testDriverLock(privconn); + virObjectLock(privconn); ninterfaces = virInterfaceObjListNumOfInterfaces(privconn->ifaces, false); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return ninterfaces; } @@ -3810,10 +3787,10 @@ testConnectListDefinedInterfaces(virConnectPtr conn, testDriverPtr privconn = conn->privateData; int nnames; - testDriverLock(privconn); + virObjectLock(privconn); nnames = virInterfaceObjListGetNames(privconn->ifaces, false, names, maxnames); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return nnames; } @@ -3862,10 +3839,10 @@ testInterfaceLookupByMACString(virConnectPtr conn, char *ifacenames[] = { NULL, NULL }; virInterfacePtr ret = NULL; - testDriverLock(privconn); + virObjectLock(privconn); ifacect = virInterfaceObjListFindByMACString(privconn->ifaces, mac, ifacenames, 2); - testDriverUnlock(privconn); + virObjectUnlock(privconn); if (ifacect == 0) { virReportError(VIR_ERR_NO_INTERFACE, @@ -3913,7 +3890,7 @@ testInterfaceChangeBegin(virConnectPtr conn, virCheckFlags(0, -1); - testDriverLock(privconn); + virObjectLock(privconn); if (privconn->transaction_running) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("there is another transaction running.")); @@ -3927,7 +3904,7 @@ testInterfaceChangeBegin(virConnectPtr conn, ret = 0; cleanup: - testDriverUnlock(privconn); + virObjectUnlock(privconn); return ret; } @@ -3941,7 +3918,7 @@ testInterfaceChangeCommit(virConnectPtr conn, virCheckFlags(0, -1); - testDriverLock(privconn); + virObjectLock(privconn); if (!privconn->transaction_running) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -3956,7 +3933,7 @@ testInterfaceChangeCommit(virConnectPtr conn, ret = 0; cleanup: - testDriverUnlock(privconn); + virObjectUnlock(privconn); return ret; } @@ -3971,7 +3948,7 @@ testInterfaceChangeRollback(virConnectPtr conn, virCheckFlags(0, -1); - testDriverLock(privconn); + virObjectLock(privconn); if (!privconn->transaction_running) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -3989,7 +3966,7 @@ testInterfaceChangeRollback(virConnectPtr conn, ret = 0; cleanup: - testDriverUnlock(privconn); + virObjectUnlock(privconn); return ret; } @@ -4029,7 +4006,7 @@ testInterfaceDefineXML(virConnectPtr conn, virCheckFlags(0, NULL); - testDriverLock(privconn); + virObjectLock(privconn); if ((def = virInterfaceDefParseString(xmlStr)) == NULL) goto cleanup; @@ -4043,7 +4020,7 @@ testInterfaceDefineXML(virConnectPtr conn, cleanup: virInterfaceDefFree(def); virInterfaceObjEndAPI(&obj); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return ret; } @@ -4147,9 +4124,9 @@ testStoragePoolObjFindByName(testDriverPtr privconn, { virStoragePoolObjPtr obj; - testDriverLock(privconn); + virObjectLock(privconn); obj = virStoragePoolObjFindByName(privconn->pools, name); - testDriverUnlock(privconn); + virObjectUnlock(privconn); if (!obj) virReportError(VIR_ERR_NO_STORAGE_POOL, @@ -4207,9 +4184,9 @@ testStoragePoolObjFindByUUID(testDriverPtr privconn, virStoragePoolObjPtr obj; char uuidstr[VIR_UUID_STRING_BUFLEN]; - testDriverLock(privconn); + virObjectLock(privconn); obj = virStoragePoolObjFindByUUID(privconn->pools, uuid); - testDriverUnlock(privconn); + virObjectUnlock(privconn); if (!obj) { virUUIDFormat(uuid, uuidstr); @@ -4275,10 +4252,10 @@ testConnectNumOfStoragePools(virConnectPtr conn) testDriverPtr privconn = conn->privateData; int numActive = 0; - testDriverLock(privconn); + virObjectLock(privconn); numActive = virStoragePoolObjNumOfStoragePools(privconn->pools, conn, true, NULL); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return numActive; } @@ -4292,10 +4269,10 @@ testConnectListStoragePools(virConnectPtr conn, testDriverPtr privconn = conn->privateData; int n = 0; - testDriverLock(privconn); + virObjectLock(privconn); n = virStoragePoolObjGetNames(privconn->pools, conn, true, NULL, names, maxnames); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return n; } @@ -4307,10 +4284,10 @@ testConnectNumOfDefinedStoragePools(virConnectPtr conn) testDriverPtr privconn = conn->privateData; int numInactive = 0; - testDriverLock(privconn); + virObjectLock(privconn); numInactive = virStoragePoolObjNumOfStoragePools(privconn->pools, conn, false, NULL); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return numInactive; } @@ -4324,10 +4301,10 @@ testConnectListDefinedStoragePools(virConnectPtr conn, testDriverPtr privconn = conn->privateData; int n = 0; - testDriverLock(privconn); + virObjectLock(privconn); n = virStoragePoolObjGetNames(privconn->pools, conn, false, NULL, names, maxnames); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return n; } @@ -4343,10 +4320,10 @@ testConnectListAllStoragePools(virConnectPtr conn, virCheckFlags(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ALL, -1); - testDriverLock(privconn); + virObjectLock(privconn); ret = virStoragePoolObjListExport(conn, privconn->pools, pools, NULL, flags); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return ret; } @@ -4508,7 +4485,7 @@ testStoragePoolCreateXML(virConnectPtr conn, virCheckFlags(0, NULL); - testDriverLock(privconn); + virObjectLock(privconn); if (!(newDef = virStoragePoolDefParseString(xml))) goto cleanup; @@ -4556,7 +4533,7 @@ testStoragePoolCreateXML(virConnectPtr conn, virStoragePoolDefFree(newDef); virObjectEventStateQueue(privconn->eventState, event); virStoragePoolObjEndAPI(&obj); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return pool; } @@ -4575,7 +4552,7 @@ testStoragePoolDefineXML(virConnectPtr conn, virCheckFlags(0, NULL); - testDriverLock(privconn); + virObjectLock(privconn); if (!(newDef = virStoragePoolDefParseString(xml))) goto cleanup; @@ -4605,7 +4582,7 @@ testStoragePoolDefineXML(virConnectPtr conn, virStoragePoolDefFree(newDef); virObjectEventStateQueue(privconn->eventState, event); virStoragePoolObjEndAPI(&obj); - testDriverUnlock(privconn); + virObjectUnlock(privconn); return pool; } @@ -5006,7 +4983,7 @@ testStorageVolLookupByKey(virConnectPtr conn, .key = key, .voldef = NULL }; virStorageVolPtr vol = NULL; - testDriverLock(privconn); + virObjectLock(privconn); if ((obj = virStoragePoolObjListSearch(privconn->pools, testStorageVolLookupByKeyCallback, &data)) && data.voldef) { @@ -5016,7 +4993,7 @@ testStorageVolLookupByKey(virConnectPtr conn, NULL, NULL); virStoragePoolObjEndAPI(&obj); } - testDriverUnlock(privconn); + virObjectUnlock(privconn); if (!vol) virReportError(VIR_ERR_NO_STORAGE_VOL, @@ -5050,7 +5027,7 @@ testStorageVolLookupByPath(virConnectPtr conn, .path = path, .voldef = NULL }; virStorageVolPtr vol = NULL; - testDriverLock(privconn); + virObjectLock(privconn); if ((obj = virStoragePoolObjListSearch(privconn->pools, testStorageVolLookupByPathCallback, &data)) && data.voldef) { @@ -5060,7 +5037,7 @@ testStorageVolLookupByPath(virConnectPtr conn, NULL, NULL); virStoragePoolObjEndAPI(&obj); } - testDriverUnlock(privconn); + virObjectUnlock(privconn); if (!vol) virReportError(VIR_ERR_NO_STORAGE_VOL, @@ -6819,7 +6796,6 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } - static virHypervisorDriver testHypervisorDriver = { .name = "Test", .connectOpen = testConnectOpen, /* 0.1.1 */ -- 2.17.0