Discussion:
[libvirt] OBFrom 2ca6bb41cd7d86a89ac565ca71ba1f2baffd0c68 Mon Sep 17 00:00:00 2001
Marc Hartmayer
2018-11-20 13:49:14 UTC
Permalink
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 <***@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <***@linux.ibm.com>
---

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-***@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
Marc Hartmayer
2018-11-20 13:54:16 UTC
Permalink
From: Marc Hartmayer <***@linux.vnet.ibm.com>

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 <***@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <***@linux.ibm.com>
---

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-***@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
Michal Privoznik
2018-11-26 14:29:45 UTC
Permalink
Post by Marc Hartmayer
into a virObjectLockable and use the provided functionalities.
---
This patch was originally posted with the patch series "[libvirt]
[PATCH libvirt v2 0/9] Fix virConnectRegisterCloseCallback and get rid
of global variables"
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(-)
ACKed and pushed.

Michal

Loading...