7:43 p.m.
Note that patch 1/8 of this series depends on patch 2/13 of the
"vCPU pinning and related refactors - Part 1.5" series:
http://www.redhat.com/archives/libvir-list/2015-June/msg00678.html
Peter Krempa (8):
test: Reuse virDomainObjGetOneDef in testDomainGetVcpusFlags
test: Switch to reference counting with testDomObjFromDomain
test: Refactor test driver domain object retrieval
test: Refactor test driver event sending
test: group domain APIs together
lib: setvcpus: Remove bogous flag check
test: Refactor testDomainSetVcpusFlags
test: Fix lock ordering in testDomainRevertToSnapshot
src/libvirt-domain.c | 6 -
src/test/test_driver.c | 941 +++++++++++++++++--------------------------------
2 files changed, 324 insertions(+), 623 deletions(-)
--
2.4.1
7:43 p.m.
New subject: [libvirt] [PATCH 1/8] test: Reuse virDomainObjGetOneDef in testDomainGetVcpusFlags
The test driver copies the domain definition correctly so we can reuse
the helper.
---
src/test/test_driver.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index d1f0af3..c0ef459 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2576,13 +2576,9 @@ testDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags)
goto cleanup;
}
- if (virDomainLiveConfigHelperMethod(privconn->caps, privconn->xmlopt,
- vm, &flags, &def) < 0)
+ if (!(def = virDomainObjGetOneDef(vm, flags)))
goto cleanup;
- if (flags & VIR_DOMAIN_AFFECT_LIVE)
- def = vm->def;
-
ret = (flags & VIR_DOMAIN_VCPU_MAXIMUM) ? def->maxvcpus : def->vcpus;
cleanup:
--
2.4.1
7:43 p.m.
New subject: [libvirt] [PATCH 2/8] test: Switch to reference counting with testDomObjFromDomain
Retrieve domain objects with reference and release them with
virDomainObjEndAPI.
---
src/test/test_driver.c | 89 ++++++++++++++++++++------------------------------
1 file changed, 35 insertions(+), 54 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index c0ef459..6613ed7 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -484,7 +484,7 @@ testDomObjFromDomain(virDomainPtr domain)
char uuidstr[VIR_UUID_STRING_BUFLEN];
testDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, domain->uuid);
+ vm = virDomainObjListFindByUUIDRef(driver->domains, domain->uuid);
if (!vm) {
virUUIDFormat(domain->uuid, uuidstr);
virReportError(VIR_ERR_NO_DOMAIN,
@@ -6262,19 +6262,17 @@ static int
testDomainSnapshotNum(virDomainPtr domain, unsigned int flags)
{
virDomainObjPtr vm = NULL;
- int n = -1;
+ int n;
virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_ROOTS |
VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1);
if (!(vm = testDomObjFromDomain(domain)))
- goto cleanup;
+ return -1;
n = virDomainSnapshotObjListNum(vm->snapshots, NULL, flags);
- cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return n;
}
@@ -6285,20 +6283,18 @@ testDomainSnapshotListNames(virDomainPtr domain,
unsigned int flags)
{
virDomainObjPtr vm = NULL;
- int n = -1;
+ int n;
virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_ROOTS |
VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1);
if (!(vm = testDomObjFromDomain(domain)))
- goto cleanup;
+ return -1;
n = virDomainSnapshotObjListGetNames(vm->snapshots, NULL, names, nameslen,
flags);
- cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return n;
}
@@ -6308,19 +6304,17 @@ testDomainListAllSnapshots(virDomainPtr domain,
unsigned int flags)
{
virDomainObjPtr vm = NULL;
- int n = -1;
+ int n;
virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_ROOTS |
VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1);
if (!(vm = testDomObjFromDomain(domain)))
- goto cleanup;
+ return -1;
n = virDomainListSnapshots(vm->snapshots, NULL, domain, snaps, flags);
- cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return n;
}
@@ -6338,7 +6332,7 @@ testDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot,
VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1);
if (!(vm = testDomObjFromSnapshot(snapshot)))
- goto cleanup;
+ return -1;
if (!(snap = testSnapObjFromSnapshot(vm, snapshot)))
goto cleanup;
@@ -6347,8 +6341,7 @@ testDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot,
flags);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return n;
}
@@ -6364,7 +6357,7 @@ testDomainSnapshotNumChildren(virDomainSnapshotPtr snapshot,
VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1);
if (!(vm = testDomObjFromSnapshot(snapshot)))
- goto cleanup;
+ return -1;
if (!(snap = testSnapObjFromSnapshot(vm, snapshot)))
goto cleanup;
@@ -6372,8 +6365,7 @@ testDomainSnapshotNumChildren(virDomainSnapshotPtr snapshot,
n = virDomainSnapshotObjListNum(vm->snapshots, snap, flags);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return n;
}
@@ -6390,7 +6382,7 @@ testDomainSnapshotListAllChildren(virDomainSnapshotPtr snapshot,
VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1);
if (!(vm = testDomObjFromSnapshot(snapshot)))
- goto cleanup;
+ return -1;
if (!(snap = testSnapObjFromSnapshot(vm, snapshot)))
goto cleanup;
@@ -6399,8 +6391,7 @@ testDomainSnapshotListAllChildren(virDomainSnapshotPtr snapshot,
flags);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return n;
}
@@ -6416,7 +6407,7 @@ testDomainSnapshotLookupByName(virDomainPtr domain,
virCheckFlags(0, NULL);
if (!(vm = testDomObjFromDomain(domain)))
- goto cleanup;
+ return NULL;
if (!(snap = testSnapObjFromName(vm, name)))
goto cleanup;
@@ -6424,8 +6415,7 @@ testDomainSnapshotLookupByName(virDomainPtr domain,
snapshot = virGetDomainSnapshot(domain, snap->def->name);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return snapshot;
}
@@ -6434,18 +6424,16 @@ testDomainHasCurrentSnapshot(virDomainPtr domain,
unsigned int flags)
{
virDomainObjPtr vm;
- int ret = -1;
+ int ret;
virCheckFlags(0, -1);
if (!(vm = testDomObjFromDomain(domain)))
- goto cleanup;
+ return -1;
ret = (vm->current_snapshot != NULL);
- cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -6460,7 +6448,7 @@ testDomainSnapshotGetParent(virDomainSnapshotPtr snapshot,
virCheckFlags(0, NULL);
if (!(vm = testDomObjFromSnapshot(snapshot)))
- goto cleanup;
+ return NULL;
if (!(snap = testSnapObjFromSnapshot(vm, snapshot)))
goto cleanup;
@@ -6475,8 +6463,7 @@ testDomainSnapshotGetParent(virDomainSnapshotPtr snapshot,
parent = virGetDomainSnapshot(snapshot->domain, snap->def->parent);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return parent;
}
@@ -6490,7 +6477,7 @@ testDomainSnapshotCurrent(virDomainPtr domain,
virCheckFlags(0, NULL);
if (!(vm = testDomObjFromDomain(domain)))
- goto cleanup;
+ return NULL;
if (!vm->current_snapshot) {
virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, "%s",
@@ -6501,8 +6488,7 @@ testDomainSnapshotCurrent(virDomainPtr domain,
snapshot = virGetDomainSnapshot(domain, vm->current_snapshot->def->name);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return snapshot;
}
@@ -6518,7 +6504,7 @@ testDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot,
virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL);
if (!(vm = testDomObjFromSnapshot(snapshot)))
- goto cleanup;
+ return NULL;
if (!(snap = testSnapObjFromSnapshot(vm, snapshot)))
goto cleanup;
@@ -6530,8 +6516,7 @@ testDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot,
0);
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return xml;
}
@@ -6540,19 +6525,17 @@ testDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot,
unsigned int flags)
{
virDomainObjPtr vm = NULL;
- int ret = -1;
+ int ret;
virCheckFlags(0, -1);
if (!(vm = testDomObjFromSnapshot(snapshot)))
- goto cleanup;
+ return -1;
ret = (vm->current_snapshot &&
STREQ(snapshot->name, vm->current_snapshot->def->name));
- cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -6567,7 +6550,7 @@ testDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot,
virCheckFlags(0, -1);
if (!(vm = testDomObjFromSnapshot(snapshot)))
- goto cleanup;
+ return -1;
if (!testSnapObjFromSnapshot(vm, snapshot))
goto cleanup;
@@ -6575,8 +6558,7 @@ testDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot,
ret = 1;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -6717,7 +6699,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain,
snap->sibling = other->first_child;
other->first_child = snap;
}
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
}
if (event) {
testDriverLock(privconn);
@@ -6854,8 +6836,7 @@ testDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -7067,7 +7048,7 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
} else {
virObjectUnref(event2);
}
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
testDriverUnlock(privconn);
return ret;
--
2.4.1
4:45 p.m.
New subject: [libvirt] [PATCH 2/8] test: Switch to reference counting with testDomObjFromDomain
On 16.06.2015 19:43, Peter Krempa wrote:
Retrieve domain objects with reference and release them with
virDomainObjEndAPI.
---
src/test/test_driver.c | 89 ++++++++++++++++++++------------------------------
1 file changed, 35 insertions(+), 54 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index c0ef459..6613ed7 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -484,7 +484,7 @@ testDomObjFromDomain(virDomainPtr domain)
char uuidstr[VIR_UUID_STRING_BUFLEN];
testDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, domain->uuid);
+ vm = virDomainObjListFindByUUIDRef(driver->domains, domain->uuid);
if (!vm) {
virUUIDFormat(domain->uuid, uuidstr);
virReportError(VIR_ERR_NO_DOMAIN,
Oh, this reminds me that I wanted to drop testDriverLock. It's
unnecessary here - domainObjList has self-locking APIs.
ACK
Michal
7:43 p.m.
New subject: [libvirt] [PATCH 3/8] test: Refactor test driver domain object retrieval
Add testDomObjFromDomainLocked and reuse it together with
testDomObjFromDomain to retrieve domain objects in the qemu driver
instead of open-coding it in every API.
---
src/test/test_driver.c | 409 +++++++++++--------------------------------------
1 file changed, 92 insertions(+), 317 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 6613ed7..dc6e49a 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -477,13 +477,12 @@ static int testStoragePoolObjSetDefaults(virStoragePoolObjPtr
pool);
static int testNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info);
static virDomainObjPtr
-testDomObjFromDomain(virDomainPtr domain)
+testDomObjFromDomainLocked(testConnPtr driver,
+ virDomainPtr domain)
{
virDomainObjPtr vm;
- testConnPtr driver = domain->conn->privateData;
char uuidstr[VIR_UUID_STRING_BUFLEN];
- testDriverLock(driver);
vm = virDomainObjListFindByUUIDRef(driver->domains, domain->uuid);
if (!vm) {
virUUIDFormat(domain->uuid, uuidstr);
@@ -492,10 +491,22 @@ testDomObjFromDomain(virDomainPtr domain)
uuidstr, domain->name);
}
- testDriverUnlock(driver);
return vm;
}
+static virDomainObjPtr
+testDomObjFromDomain(virDomainPtr domain)
+{
+ testConnPtr driver = domain->conn->privateData;
+ virDomainObjPtr ret;
+
+ testDriverLock(driver);
+ ret = testDomObjFromDomainLocked(driver, domain);
+ testDriverUnlock(driver);
+
+ return ret;
+}
+
static char *
testDomainGenerateIfname(virDomainDefPtr domdef)
{
@@ -1688,43 +1699,28 @@ static int testConnectNumOfDomains(virConnectPtr conn)
static int testDomainIsActive(virDomainPtr dom)
{
- testConnPtr privconn = dom->conn->privateData;
virDomainObjPtr obj;
- int ret = -1;
+ int ret;
- testDriverLock(privconn);
- obj = virDomainObjListFindByUUID(privconn->domains, dom->uuid);
- testDriverUnlock(privconn);
- if (!obj) {
- virReportError(VIR_ERR_NO_DOMAIN, NULL);
- goto cleanup;
- }
- ret = virDomainObjIsActive(obj);
+ if (!(obj = testDomObjFromDomain(dom)))
+ return -1;
- cleanup:
- if (obj)
- virObjectUnlock(obj);
+ ret = virDomainObjIsActive(obj);
+ virDomainObjEndAPI(&obj);
return ret;
}
static int testDomainIsPersistent(virDomainPtr dom)
{
- testConnPtr privconn = dom->conn->privateData;
virDomainObjPtr obj;
- int ret = -1;
+ int ret;
+
+ if (!(obj = testDomObjFromDomain(dom)))
+ return -1;
- testDriverLock(privconn);
- obj = virDomainObjListFindByUUID(privconn->domains, dom->uuid);
- testDriverUnlock(privconn);
- if (!obj) {
- virReportError(VIR_ERR_NO_DOMAIN, NULL);
- goto cleanup;
- }
ret = obj->persistent;
- cleanup:
- if (obj)
- virObjectUnlock(obj);
+ virDomainObjEndAPI(&obj);
return ret;
}
@@ -1885,13 +1881,9 @@ static int testDomainDestroy(virDomainPtr domain)
int ret = -1;
testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- domain->name);
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+ if (!(privdom = testDomObjFromDomainLocked(privconn, domain)))
goto cleanup;
- }
testDomainShutdownState(domain, privdom, VIR_DOMAIN_SHUTOFF_DESTROYED);
event = virDomainEventLifecycleNewFromObj(privdom,
@@ -1917,15 +1909,8 @@ static int testDomainResume(virDomainPtr domain)
virObjectEventPtr event = NULL;
int ret = -1;
- testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- domain->name);
- testDriverUnlock(privconn);
-
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto cleanup;
- }
+ if (!(privdom = testDomObjFromDomain(domain)))
+ return -1;
if (virDomainObjGetState(privdom, NULL) != VIR_DOMAIN_PAUSED) {
virReportError(VIR_ERR_INTERNAL_ERROR, _("domain '%s' not
paused"),
@@ -1958,15 +1943,8 @@ static int testDomainSuspend(virDomainPtr domain)
int ret = -1;
int state;
- testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- domain->name);
- testDriverUnlock(privconn);
-
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto cleanup;
- }
+ if (!(privdom = testDomObjFromDomain(domain)))
+ return -1;
state = virDomainObjGetState(privdom, NULL);
if (state == VIR_DOMAIN_SHUTOFF || state == VIR_DOMAIN_PAUSED) {
@@ -2003,13 +1981,9 @@ static int testDomainShutdownFlags(virDomainPtr domain,
virCheckFlags(0, -1);
testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- domain->name);
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+ if (!(privdom = testDomObjFromDomainLocked(privconn, domain)))
goto cleanup;
- }
if (virDomainObjGetState(privdom, NULL) == VIR_DOMAIN_SHUTOFF) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -2049,13 +2023,9 @@ static int testDomainReboot(virDomainPtr domain,
int ret = -1;
testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- domain->name);
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+ if (!(privdom = testDomObjFromDomainLocked(privconn, domain)))
goto cleanup;
- }
virDomainObjSetState(privdom, VIR_DOMAIN_SHUTDOWN,
VIR_DOMAIN_SHUTDOWN_USER);
@@ -2109,20 +2079,12 @@ static int testDomainReboot(virDomainPtr domain,
static int testDomainGetInfo(virDomainPtr domain,
virDomainInfoPtr info)
{
- testConnPtr privconn = domain->conn->privateData;
struct timeval tv;
virDomainObjPtr privdom;
int ret = -1;
- testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- domain->name);
- testDriverUnlock(privconn);
-
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto cleanup;
- }
+ if (!(privdom = testDomObjFromDomain(domain)))
+ return -1;
if (gettimeofday(&tv, NULL) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -2148,28 +2110,18 @@ testDomainGetState(virDomainPtr domain,
int *reason,
unsigned int flags)
{
- testConnPtr privconn = domain->conn->privateData;
virDomainObjPtr privdom;
- int ret = -1;
virCheckFlags(0, -1);
- testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- domain->name);
- testDriverUnlock(privconn);
-
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto cleanup;
- }
+ if (!(privdom = testDomObjFromDomain(domain)))
+ return -1;
*state = virDomainObjGetState(privdom, reason);
- ret = 0;
- cleanup:
virDomainObjEndAPI(&privdom);
- return ret;
+
+ return 0;
}
#define TEST_SAVE_MAGIC "TestGuestMagic"
@@ -2194,13 +2146,9 @@ testDomainSaveFlags(virDomainPtr domain, const char *path,
}
testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- domain->name);
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+ if (!(privdom = testDomObjFromDomainLocked(privconn, domain)))
goto cleanup;
- }
xml = virDomainDefFormat(privdom->def,
VIR_DOMAIN_DEF_FORMAT_SECURE);
@@ -2398,13 +2346,9 @@ static int testDomainCoreDumpWithFormat(virDomainPtr domain,
virCheckFlags(VIR_DUMP_CRASH, -1);
testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- domain->name);
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+ if (!(privdom = testDomObjFromDomainLocked(privconn, domain)))
goto cleanup;
- }
if ((fd = open(to, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) {
virReportSystemError(errno,
@@ -2475,23 +2419,14 @@ testDomainGetOSType(virDomainPtr dom ATTRIBUTE_UNUSED)
static unsigned long long
testDomainGetMaxMemory(virDomainPtr domain)
{
- testConnPtr privconn = domain->conn->privateData;
virDomainObjPtr privdom;
unsigned long long ret = 0;
- testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- domain->name);
- testDriverUnlock(privconn);
-
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto cleanup;
- }
+ if (!(privdom = testDomObjFromDomain(domain)))
+ return 0;
ret = virDomainDefGetMemoryActual(privdom->def);
- cleanup:
virDomainObjEndAPI(&privdom);
return ret;
}
@@ -2499,45 +2434,26 @@ testDomainGetMaxMemory(virDomainPtr domain)
static int testDomainSetMaxMemory(virDomainPtr domain,
unsigned long memory)
{
- testConnPtr privconn = domain->conn->privateData;
virDomainObjPtr privdom;
- int ret = -1;
- testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- domain->name);
- testDriverUnlock(privconn);
-
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto cleanup;
- }
+ if (!(privdom = testDomObjFromDomain(domain)))
+ return -1;
/* XXX validate not over host memory wrt to other domains */
virDomainDefSetMemoryInitial(privdom->def, memory);
- ret = 0;
- cleanup:
virDomainObjEndAPI(&privdom);
- return ret;
+ return 0;
}
static int testDomainSetMemory(virDomainPtr domain,
unsigned long memory)
{
- testConnPtr privconn = domain->conn->privateData;
virDomainObjPtr privdom;
int ret = -1;
- testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- domain->name);
- testDriverUnlock(privconn);
-
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto cleanup;
- }
+ if (!(privdom = testDomObjFromDomain(domain)))
+ return -1;
if (memory > virDomainDefGetMemoryActual(privdom->def)) {
virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
@@ -2555,7 +2471,6 @@ static int testDomainSetMemory(virDomainPtr domain,
static int
testDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags)
{
- testConnPtr privconn = domain->conn->privateData;
virDomainObjPtr vm;
virDomainDefPtr def;
int ret = -1;
@@ -2564,17 +2479,8 @@ testDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags)
VIR_DOMAIN_AFFECT_CONFIG |
VIR_DOMAIN_VCPU_MAXIMUM, -1);
- testDriverLock(privconn);
- vm = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
- testDriverUnlock(privconn);
-
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(domain->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("no domain with matching uuid '%s'"),
uuidstr);
- goto cleanup;
- }
+ if (!(vm = testDomObjFromDomain(domain)))
+ return -1;
if (!(def = virDomainObjGetOneDef(vm, flags)))
goto cleanup;
@@ -2582,8 +2488,7 @@ testDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags)
ret = (flags & VIR_DOMAIN_VCPU_MAXIMUM) ? def->maxvcpus : def->vcpus;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virDomainObjEndAPI(&vm);
return ret;
}
@@ -2622,14 +2527,8 @@ testDomainSetVcpusFlags(virDomainPtr domain, unsigned int nrCpus,
return -1;
}
- testDriverLock(privconn);
- privdom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
- testDriverUnlock(privconn);
-
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto cleanup;
- }
+ if (!(privdom = testDomObjFromDomain(domain)))
+ return -1;
if (!virDomainObjIsActive(privdom) && (flags & VIR_DOMAIN_AFFECT_LIVE))
{
virReportError(VIR_ERR_OPERATION_INVALID,
@@ -2680,8 +2579,7 @@ testDomainSetVcpusFlags(virDomainPtr domain, unsigned int nrCpus,
}
cleanup:
- if (privdom)
- virObjectUnlock(privdom);
+ virDomainObjEndAPI(&privdom);
return ret;
}
@@ -2706,14 +2604,8 @@ static int testDomainGetVcpus(virDomainPtr domain,
struct timeval tv;
unsigned long long statbase;
- testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains, domain->name);
- testDriverUnlock(privconn);
-
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto cleanup;
- }
+ if (!(privdom = testDomObjFromDomain(domain)))
+ return -1;
if (!virDomainObjIsActive(privdom)) {
virReportError(VIR_ERR_OPERATION_INVALID,
@@ -2791,14 +2683,8 @@ static int testDomainPinVcpu(virDomainPtr domain,
int maxcpu, hostcpus, privmaplen;
int ret = -1;
- testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains, domain->name);
- testDriverUnlock(privconn);
-
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto cleanup;
- }
+ if (!(privdom = testDomObjFromDomain(domain)))
+ return -1;
if (!virDomainObjIsActive(privdom)) {
virReportError(VIR_ERR_OPERATION_INVALID,
@@ -2836,30 +2722,20 @@ static int testDomainPinVcpu(virDomainPtr domain,
static char *testDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
{
- testConnPtr privconn = domain->conn->privateData;
virDomainDefPtr def;
virDomainObjPtr privdom;
char *ret = NULL;
/* Flags checked by virDomainDefFormat */
- testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- domain->name);
- testDriverUnlock(privconn);
-
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto cleanup;
- }
+ if (!(privdom = testDomObjFromDomain(domain)))
+ return NULL;
def = (flags & VIR_DOMAIN_XML_INACTIVE) &&
privdom->newDef ? privdom->newDef : privdom->def;
- ret = virDomainDefFormat(def,
- virDomainDefFormatConvertXMLFlags(flags));
+ ret = virDomainDefFormat(def, virDomainDefFormatConvertXMLFlags(flags));
- cleanup:
virDomainObjEndAPI(&privdom);
return ret;
}
@@ -2960,25 +2836,17 @@ static char *testDomainGetMetadata(virDomainPtr dom,
{
testConnPtr privconn = dom->conn->privateData;
virDomainObjPtr privdom;
- char *ret = NULL;
+ char *ret;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG, NULL);
- testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- dom->name);
- testDriverUnlock(privconn);
-
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto cleanup;
- }
+ if (!(privdom = testDomObjFromDomain(dom)))
+ return NULL;
ret = virDomainObjGetMetadata(privdom, type, uri, privconn->caps,
privconn->xmlopt, flags);
- cleanup:
virDomainObjEndAPI(&privdom);
return ret;
}
@@ -2992,26 +2860,18 @@ static int testDomainSetMetadata(virDomainPtr dom,
{
testConnPtr privconn = dom->conn->privateData;
virDomainObjPtr privdom;
- int ret = -1;
+ int ret;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG, -1);
- testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- dom->name);
- testDriverUnlock(privconn);
-
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto cleanup;
- }
+ if (!(privdom = testDomObjFromDomain(dom)))
+ return -1;
ret = virDomainObjSetMetadata(privdom, type, metadata, key, uri,
privconn->caps, privconn->xmlopt,
NULL, NULL, flags);
- cleanup:
virDomainObjEndAPI(&privdom);
return ret;
}
@@ -3056,13 +2916,9 @@ static int testDomainCreateWithFlags(virDomainPtr domain, unsigned
int flags)
virCheckFlags(0, -1);
testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- domain->name);
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+ if (!(privdom = testDomObjFromDomainLocked(privconn, domain)))
goto cleanup;
- }
if (virDomainObjGetState(privdom, NULL) != VIR_DOMAIN_SHUTOFF) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -3106,13 +2962,9 @@ static int testDomainUndefineFlags(virDomainPtr domain,
VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1);
testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- domain->name);
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+ if (!(privdom = testDomObjFromDomainLocked(privconn, domain)))
goto cleanup;
- }
if (privdom->hasManagedSave &&
!(flags & VIR_DOMAIN_UNDEFINE_MANAGED_SAVE)) {
@@ -3168,52 +3020,30 @@ static int testDomainUndefine(virDomainPtr domain)
static int testDomainGetAutostart(virDomainPtr domain,
int *autostart)
{
- testConnPtr privconn = domain->conn->privateData;
virDomainObjPtr privdom;
- int ret = -1;
- testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- domain->name);
- testDriverUnlock(privconn);
-
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto cleanup;
- }
+ if (!(privdom = testDomObjFromDomain(domain)))
+ return -1;
*autostart = privdom->autostart;
- ret = 0;
- cleanup:
virDomainObjEndAPI(&privdom);
- return ret;
+ return 0;
}
static int testDomainSetAutostart(virDomainPtr domain,
int autostart)
{
- testConnPtr privconn = domain->conn->privateData;
virDomainObjPtr privdom;
- int ret = -1;
- testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- domain->name);
- testDriverUnlock(privconn);
-
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto cleanup;
- }
+ if (!(privdom = testDomObjFromDomain(domain)))
+ return -1;
privdom->autostart = autostart ? 1 : 0;
- ret = 0;
- cleanup:
virDomainObjEndAPI(&privdom);
- return ret;
+ return 0;
}
static char *testDomainGetSchedulerType(virDomainPtr domain ATTRIBUTE_UNUSED,
@@ -3235,21 +3065,13 @@ testDomainGetSchedulerParametersFlags(virDomainPtr domain,
int *nparams,
unsigned int flags)
{
- testConnPtr privconn = domain->conn->privateData;
virDomainObjPtr privdom;
int ret = -1;
virCheckFlags(0, -1);
- testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- domain->name);
- testDriverUnlock(privconn);
-
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto cleanup;
- }
+ if (!(privdom = testDomObjFromDomain(domain)))
+ return -1;
if (virTypedParameterAssign(params, VIR_DOMAIN_SCHEDULER_WEIGHT,
VIR_TYPED_PARAM_UINT, 50) < 0)
@@ -3279,7 +3101,6 @@ testDomainSetSchedulerParametersFlags(virDomainPtr domain,
int nparams,
unsigned int flags)
{
- testConnPtr privconn = domain->conn->privateData;
virDomainObjPtr privdom;
int ret = -1;
size_t i;
@@ -3291,15 +3112,8 @@ testDomainSetSchedulerParametersFlags(virDomainPtr domain,
NULL) < 0)
return -1;
- testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- domain->name);
- testDriverUnlock(privconn);
-
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto cleanup;
- }
+ if (!(privdom = testDomObjFromDomain(domain)))
+ return -1;
for (i = 0; i < nparams; i++) {
if (STREQ(params[i].field, VIR_DOMAIN_SCHEDULER_WEIGHT)) {
@@ -3310,7 +3124,6 @@ testDomainSetSchedulerParametersFlags(virDomainPtr domain,
ret = 0;
- cleanup:
virDomainObjEndAPI(&privdom);
return ret;
}
@@ -3327,7 +3140,6 @@ static int testDomainBlockStats(virDomainPtr domain,
const char *path,
virDomainBlockStatsPtr stats)
{
- testConnPtr privconn = domain->conn->privateData;
virDomainObjPtr privdom;
struct timeval tv;
unsigned long long statbase;
@@ -3339,15 +3151,8 @@ static int testDomainBlockStats(virDomainPtr domain,
return ret;
}
- testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- domain->name);
- testDriverUnlock(privconn);
-
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto error;
- }
+ if (!(privdom = testDomObjFromDomain(domain)))
+ return ret;
if (virDomainDiskIndexByName(privdom->def, path, false) < 0) {
virReportError(VIR_ERR_INVALID_ARG,
@@ -3379,22 +3184,14 @@ static int testDomainInterfaceStats(virDomainPtr domain,
const char *path,
virDomainInterfaceStatsPtr stats)
{
- testConnPtr privconn = domain->conn->privateData;
virDomainObjPtr privdom;
struct timeval tv;
unsigned long long statbase;
size_t i;
int found = 0, ret = -1;
- testDriverLock(privconn);
- privdom = virDomainObjListFindByName(privconn->domains,
- domain->name);
- testDriverUnlock(privconn);
-
- if (privdom == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto error;
- }
+ if (!(privdom = testDomObjFromDomain(domain)))
+ return -1;
for (i = 0; i < privdom->def->nnets; i++) {
if (privdom->def->nets[i]->ifname &&
@@ -6138,14 +5935,8 @@ testDomainManagedSave(virDomainPtr dom, unsigned int flags)
VIR_DOMAIN_SAVE_RUNNING |
VIR_DOMAIN_SAVE_PAUSED, -1);
- testDriverLock(privconn);
- vm = virDomainObjListFindByName(privconn->domains, dom->name);
- testDriverUnlock(privconn);
-
- if (vm == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto cleanup;
- }
+ if (!(vm = testDomObjFromDomain(dom)))
+ return -1;
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID,
@@ -6181,50 +5972,34 @@ testDomainManagedSave(virDomainPtr dom, unsigned int flags)
static int
testDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags)
{
- testConnPtr privconn = dom->conn->privateData;
virDomainObjPtr vm;
- int ret = -1;
+ int ret;
virCheckFlags(0, -1);
- testDriverLock(privconn);
-
- vm = virDomainObjListFindByName(privconn->domains, dom->name);
- if (vm == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto cleanup;
- }
+ if (!(vm = testDomObjFromDomain(dom)))
+ return -1;
ret = vm->hasManagedSave;
- cleanup:
+
virDomainObjEndAPI(&vm);
- testDriverUnlock(privconn);
return ret;
}
static int
testDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
{
- testConnPtr privconn = dom->conn->privateData;
virDomainObjPtr vm;
- int ret = -1;
virCheckFlags(0, -1);
- testDriverLock(privconn);
-
- vm = virDomainObjListFindByName(privconn->domains, dom->name);
- if (vm == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
- goto cleanup;
- }
+ if (!(vm = testDomObjFromDomain(dom)))
+ return -1;
vm->hasManagedSave = false;
- ret = 0;
- cleanup:
+
virDomainObjEndAPI(&vm);
- testDriverUnlock(privconn);
- return ret;
+ return 0;
}
--
2.4.1
4:45 p.m.
New subject: [libvirt] [PATCH 3/8] test: Refactor test driver domain object retrieval
On 16.06.2015 19:43, Peter Krempa wrote:
Add testDomObjFromDomainLocked and reuse it together with
testDomObjFromDomain to retrieve domain objects in the qemu driver
s/qemu/test/
instead of open-coding it in every API.
---
src/test/test_driver.c | 409 +++++++++++--------------------------------------
1 file changed, 92 insertions(+), 317 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 6613ed7..dc6e49a 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -477,13 +477,12 @@ static int testStoragePoolObjSetDefaults(virStoragePoolObjPtr
pool);
static int testNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info);
static virDomainObjPtr
-testDomObjFromDomain(virDomainPtr domain)
+testDomObjFromDomainLocked(testConnPtr driver,
+ virDomainPtr domain)
{
virDomainObjPtr vm;
- testConnPtr driver = domain->conn->privateData;
char uuidstr[VIR_UUID_STRING_BUFLEN];
- testDriverLock(driver);
vm = virDomainObjListFindByUUIDRef(driver->domains, domain->uuid);
if (!vm) {
virUUIDFormat(domain->uuid, uuidstr);
@@ -492,10 +491,22 @@ testDomObjFromDomain(virDomainPtr domain)
uuidstr, domain->name);
}
- testDriverUnlock(driver);
return vm;
}
+static virDomainObjPtr
+testDomObjFromDomain(virDomainPtr domain)
+{
+ testConnPtr driver = domain->conn->privateData;
+ virDomainObjPtr ret;
+
+ testDriverLock(driver);
+ ret = testDomObjFromDomainLocked(driver, domain);
+ testDriverUnlock(driver);
+
+ return ret;
+}
+
I don't think this is necessary. As I've said in the previous e-mail -
domainObjList has self-locking APIs. And the driver->domains is
immutable pointer. Or am I missing something?
ACK to the rest though.
Michal
7:43 p.m.
New subject: [libvirt] [PATCH 4/8] test: Refactor test driver event sending
Make testObjectEventQueue tolerant to NULL @event and move it so that it
does not require a prototype. Additionally add
testObjectEventQueueUnlocked that will lock @driver before sending the
event.
Refactor the rest of the codebase to make use of the above features
---
src/test/test_driver.c | 112 +++++++++++++++++++------------------------------
1 file changed, 44 insertions(+), 68 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index dc6e49a..20d3c71 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -137,8 +137,6 @@ static const virNodeInfo defaultNodeInfo = {
static int testConnectClose(virConnectPtr conn);
-static void testObjectEventQueue(testConnPtr driver,
- virObjectEventPtr event);
static void testDriverLock(testConnPtr driver)
{
@@ -150,6 +148,28 @@ static void testDriverUnlock(testConnPtr driver)
virMutexUnlock(&driver->lock);
}
+static void testObjectEventQueue(testConnPtr driver,
+ virObjectEventPtr event)
+{
+ if (!event)
+ return;
+
+ virObjectEventStateQueue(driver->eventState, event);
+}
+
+
+static void
+testObjectEventQueueUnlocked(testConnPtr driver,
+ virObjectEventPtr event)
+{
+ if (!event)
+ return;
+
+ testDriverLock(driver);
+ testObjectEventQueue(driver, event);
+ testDriverUnlock(driver);
+}
+
static void *testDomainObjPrivateAlloc(void)
{
testDomainObjPrivatePtr priv;
@@ -1774,8 +1794,7 @@ testDomainCreateXML(virConnectPtr conn, const char *xml,
cleanup:
if (dom)
virObjectUnlock(dom);
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
virDomainDefFree(def);
testDriverUnlock(privconn);
return ret;
@@ -1896,8 +1915,7 @@ static int testDomainDestroy(virDomainPtr domain)
ret = 0;
cleanup:
virDomainObjEndAPI(&privdom);
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
testDriverUnlock(privconn);
return ret;
}
@@ -1927,11 +1945,7 @@ static int testDomainResume(virDomainPtr domain)
cleanup:
virDomainObjEndAPI(&privdom);
- if (event) {
- testDriverLock(privconn);
- testObjectEventQueue(privconn, event);
- testDriverUnlock(privconn);
- }
+ testObjectEventQueueUnlocked(privconn, event);
return ret;
}
@@ -1961,12 +1975,7 @@ static int testDomainSuspend(virDomainPtr domain)
cleanup:
virDomainObjEndAPI(&privdom);
-
- if (event) {
- testDriverLock(privconn);
- testObjectEventQueue(privconn, event);
- testDriverUnlock(privconn);
- }
+ testObjectEventQueueUnlocked(privconn, event);
return ret;
}
@@ -2002,8 +2011,7 @@ static int testDomainShutdownFlags(virDomainPtr domain,
ret = 0;
cleanup:
virDomainObjEndAPI(&privdom);
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
testDriverUnlock(privconn);
return ret;
}
@@ -2070,8 +2078,7 @@ static int testDomainReboot(virDomainPtr domain,
ret = 0;
cleanup:
virDomainObjEndAPI(&privdom);
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
testDriverUnlock(privconn);
return ret;
}
@@ -2214,8 +2221,7 @@ testDomainSaveFlags(virDomainPtr domain, const char *path,
unlink(path);
}
virDomainObjEndAPI(&privdom);
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
testDriverUnlock(privconn);
return ret;
}
@@ -2319,8 +2325,7 @@ testDomainRestoreFlags(virConnectPtr conn,
VIR_FORCE_CLOSE(fd);
if (dom)
virObjectUnlock(dom);
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
testDriverUnlock(privconn);
return ret;
}
@@ -2389,8 +2394,7 @@ static int testDomainCoreDumpWithFormat(virDomainPtr domain,
cleanup:
VIR_FORCE_CLOSE(fd);
virDomainObjEndAPI(&privdom);
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
testDriverUnlock(privconn);
return ret;
}
@@ -2817,8 +2821,7 @@ static virDomainPtr testDomainDefineXMLFlags(virConnectPtr conn,
virDomainDefFree(oldDef);
if (dom)
virObjectUnlock(dom);
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
testDriverUnlock(privconn);
return ret;
}
@@ -2938,8 +2941,7 @@ static int testDomainCreateWithFlags(virDomainPtr domain, unsigned
int flags)
cleanup:
virDomainObjEndAPI(&privdom);
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
testDriverUnlock(privconn);
return ret;
}
@@ -3006,8 +3008,7 @@ static int testDomainUndefineFlags(virDomainPtr domain,
cleanup:
virDomainObjEndAPI(&privdom);
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
testDriverUnlock(privconn);
return ret;
}
@@ -3385,8 +3386,7 @@ static virNetworkPtr testNetworkCreateXML(virConnectPtr conn, const
char *xml)
cleanup:
virNetworkDefFree(def);
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
virNetworkObjEndAPI(&net);
return ret;
}
@@ -3415,8 +3415,7 @@ virNetworkPtr testNetworkDefineXML(virConnectPtr conn, const char
*xml)
cleanup:
virNetworkDefFree(def);
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
virNetworkObjEndAPI(&net);
return ret;
}
@@ -3449,8 +3448,7 @@ static int testNetworkUndefine(virNetworkPtr network)
ret = 0;
cleanup:
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
virNetworkObjEndAPI(&privnet);
return ret;
}
@@ -3527,8 +3525,7 @@ static int testNetworkCreate(virNetworkPtr network)
ret = 0;
cleanup:
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
virNetworkObjEndAPI(&privnet);
return ret;
}
@@ -3556,8 +3553,7 @@ static int testNetworkDestroy(virNetworkPtr network)
ret = 0;
cleanup:
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
virNetworkObjEndAPI(&privnet);
return ret;
}
@@ -5840,15 +5836,6 @@ testConnectNetworkEventDeregisterAny(virConnectPtr conn,
return ret;
}
-
-/* driver must be locked before calling */
-static void testObjectEventQueue(testConnPtr driver,
- virObjectEventPtr event)
-{
- virObjectEventStateQueue(driver->eventState, event);
-}
-
-
static int testConnectListAllDomains(virConnectPtr conn,
virDomainPtr **domains,
unsigned int flags)
@@ -5959,11 +5946,7 @@ testDomainManagedSave(virDomainPtr dom, unsigned int flags)
ret = 0;
cleanup:
virDomainObjEndAPI(&vm);
- if (event) {
- testDriverLock(privconn);
- testObjectEventQueue(privconn, event);
- testDriverUnlock(privconn);
- }
+ testObjectEventQueueUnlocked(privconn, event);
return ret;
}
@@ -6476,11 +6459,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain,
}
virDomainObjEndAPI(&vm);
}
- if (event) {
- testDriverLock(privconn);
- testObjectEventQueue(privconn, event);
- testDriverUnlock(privconn);
- }
+ testObjectEventQueueUnlocked(privconn, event);
virDomainSnapshotDefFree(def);
return snapshot;
}
@@ -6720,8 +6699,7 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT);
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
goto load;
}
@@ -6800,8 +6778,7 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
/* Flush first event, now do transition 2 or 3 */
bool paused = (flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED) != 0;
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STARTED,
VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT);
@@ -6818,8 +6795,7 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
cleanup:
if (event) {
testObjectEventQueue(privconn, event);
- if (event2)
- testObjectEventQueue(privconn, event2);
+ testObjectEventQueue(privconn, event2);
} else {
virObjectUnref(event2);
}
--
2.4.1
4:44 p.m.
New subject: [libvirt] [PATCH 4/8] test: Refactor test driver event sending
On 16.06.2015 19:43, Peter Krempa wrote:
Make testObjectEventQueue tolerant to NULL @event and move it so that
it
does not require a prototype. Additionally add
testObjectEventQueueUnlocked that will lock @driver before sending the
event.
Refactor the rest of the codebase to make use of the above features
---
src/test/test_driver.c | 112 +++++++++++++++++++------------------------------
1 file changed, 44 insertions(+), 68 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index dc6e49a..20d3c71 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -137,8 +137,6 @@ static const virNodeInfo defaultNodeInfo = {
@@ -3385,8 +3386,7 @@ static virNetworkPtr
testNetworkCreateXML(virConnectPtr conn, const char *xml)
cleanup:
virNetworkDefFree(def);
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
Unlocked()
virNetworkObjEndAPI(&net);
return ret;
}
@@ -3415,8 +3415,7 @@ virNetworkPtr testNetworkDefineXML(virConnectPtr conn, const char
*xml)
cleanup:
virNetworkDefFree(def);
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
Unlocked()
virNetworkObjEndAPI(&net);
return ret;
}
@@ -3449,8 +3448,7 @@ static int testNetworkUndefine(virNetworkPtr network)
ret = 0;
cleanup:
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
Unlocked()
virNetworkObjEndAPI(&privnet);
return ret;
}
@@ -3527,8 +3525,7 @@ static int testNetworkCreate(virNetworkPtr network)
ret = 0;
cleanup:
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
Unlocked()
virNetworkObjEndAPI(&privnet);
return ret;
}
@@ -3556,8 +3553,7 @@ static int testNetworkDestroy(virNetworkPtr network)
ret = 0;
cleanup:
- if (event)
- testObjectEventQueue(privconn, event);
+ testObjectEventQueue(privconn, event);
Unlocked()
virNetworkObjEndAPI(&privnet);
return ret;
}
@@ -5840,15 +5836,6 @@ testConnectNetworkEventDeregisterAny(virConnectPtr conn,
return ret;
}
ACK with those nits fixed.
Michal
7:43 p.m.
New subject: [libvirt] [PATCH 5/8] test: group domain APIs together
The test driver groups the API groups together, but some domain APIs
were scattered around.
---
src/test/test_driver.c | 225 +++++++++++++++++++++++++------------------------
1 file changed, 113 insertions(+), 112 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 20d3c71..59e2031 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3231,6 +3231,119 @@ static int testDomainInterfaceStats(virDomainPtr domain,
return ret;
}
+static int testConnectListAllDomains(virConnectPtr conn,
+ virDomainPtr **domains,
+ unsigned int flags)
+{
+ testConnPtr privconn = conn->privateData;
+ int ret;
+
+ virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ALL, -1);
+
+ testDriverLock(privconn);
+ ret = virDomainObjListExport(privconn->domains, conn, domains,
+ NULL, flags);
+ testDriverUnlock(privconn);
+
+ return ret;
+}
+
+static char *
+testDomainScreenshot(virDomainPtr dom ATTRIBUTE_UNUSED,
+ virStreamPtr st,
+ unsigned int screen ATTRIBUTE_UNUSED,
+ unsigned int flags)
+{
+ char *ret = NULL;
+
+ virCheckFlags(0, NULL);
+
+ if (VIR_STRDUP(ret, "image/png") < 0)
+ return NULL;
+
+ if (virFDStreamOpenFile(st, PKGDATADIR "/libvirtLogo.png", 0, 0, O_RDONLY)
< 0)
+ VIR_FREE(ret);
+
+ return ret;
+}
+
+static int
+testDomainManagedSave(virDomainPtr dom, unsigned int flags)
+{
+ testConnPtr privconn = dom->conn->privateData;
+ virDomainObjPtr vm = NULL;
+ virObjectEventPtr event = NULL;
+ int ret = -1;
+
+ virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE |
+ VIR_DOMAIN_SAVE_RUNNING |
+ VIR_DOMAIN_SAVE_PAUSED, -1);
+
+ if (!(vm = testDomObjFromDomain(dom)))
+ return -1;
+
+ if (!virDomainObjIsActive(vm)) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is not running"));
+ goto cleanup;
+ }
+
+ if (!vm->persistent) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("cannot do managed save for transient domain"));
+ goto cleanup;
+ }
+
+ testDomainShutdownState(dom, vm, VIR_DOMAIN_SHUTOFF_SAVED);
+ event = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_STOPPED,
+ VIR_DOMAIN_EVENT_STOPPED_SAVED);
+ vm->hasManagedSave = true;
+
+ ret = 0;
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ testObjectEventQueueUnlocked(privconn, event);
+
+ return ret;
+}
+
+static int
+testDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags)
+{
+ virDomainObjPtr vm;
+ int ret;
+
+ virCheckFlags(0, -1);
+
+ if (!(vm = testDomObjFromDomain(dom)))
+ return -1;
+
+ ret = vm->hasManagedSave;
+
+ virDomainObjEndAPI(&vm);
+ return ret;
+}
+
+static int
+testDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
+{
+ virDomainObjPtr vm;
+
+ virCheckFlags(0, -1);
+
+ if (!(vm = testDomObjFromDomain(dom)))
+ return -1;
+
+ vm->hasManagedSave = false;
+
+ virDomainObjEndAPI(&vm);
+ return 0;
+}
+
+/*
+ * Network APIs
+ */
static virNetworkPtr testNetworkLookupByUUID(virConnectPtr conn,
const unsigned char *uuid)
@@ -5836,23 +5949,6 @@ testConnectNetworkEventDeregisterAny(virConnectPtr conn,
return ret;
}
-static int testConnectListAllDomains(virConnectPtr conn,
- virDomainPtr **domains,
- unsigned int flags)
-{
- testConnPtr privconn = conn->privateData;
- int ret;
-
- virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ALL, -1);
-
- testDriverLock(privconn);
- ret = virDomainObjListExport(privconn->domains, conn, domains,
- NULL, flags);
- testDriverUnlock(privconn);
-
- return ret;
-}
-
static int
testNodeGetCPUMap(virConnectPtr conn,
unsigned char **cpumap,
@@ -5881,25 +5977,6 @@ testNodeGetCPUMap(virConnectPtr conn,
return ret;
}
-static char *
-testDomainScreenshot(virDomainPtr dom ATTRIBUTE_UNUSED,
- virStreamPtr st,
- unsigned int screen ATTRIBUTE_UNUSED,
- unsigned int flags)
-{
- char *ret = NULL;
-
- virCheckFlags(0, NULL);
-
- if (VIR_STRDUP(ret, "image/png") < 0)
- return NULL;
-
- if (virFDStreamOpenFile(st, PKGDATADIR "/libvirtLogo.png", 0, 0, O_RDONLY)
< 0)
- VIR_FREE(ret);
-
- return ret;
-}
-
static int
testConnectGetCPUModelNames(virConnectPtr conn ATTRIBUTE_UNUSED,
const char *arch,
@@ -5910,82 +5987,6 @@ testConnectGetCPUModelNames(virConnectPtr conn ATTRIBUTE_UNUSED,
return cpuGetModels(arch, models);
}
-static int
-testDomainManagedSave(virDomainPtr dom, unsigned int flags)
-{
- testConnPtr privconn = dom->conn->privateData;
- virDomainObjPtr vm = NULL;
- virObjectEventPtr event = NULL;
- int ret = -1;
-
- virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE |
- VIR_DOMAIN_SAVE_RUNNING |
- VIR_DOMAIN_SAVE_PAUSED, -1);
-
- if (!(vm = testDomObjFromDomain(dom)))
- return -1;
-
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
- goto cleanup;
- }
-
- if (!vm->persistent) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot do managed save for transient domain"));
- goto cleanup;
- }
-
- testDomainShutdownState(dom, vm, VIR_DOMAIN_SHUTOFF_SAVED);
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_STOPPED,
- VIR_DOMAIN_EVENT_STOPPED_SAVED);
- vm->hasManagedSave = true;
-
- ret = 0;
- cleanup:
- virDomainObjEndAPI(&vm);
- testObjectEventQueueUnlocked(privconn, event);
-
- return ret;
-}
-
-
-static int
-testDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags)
-{
- virDomainObjPtr vm;
- int ret;
-
- virCheckFlags(0, -1);
-
- if (!(vm = testDomObjFromDomain(dom)))
- return -1;
-
- ret = vm->hasManagedSave;
-
- virDomainObjEndAPI(&vm);
- return ret;
-}
-
-static int
-testDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
-{
- virDomainObjPtr vm;
-
- virCheckFlags(0, -1);
-
- if (!(vm = testDomObjFromDomain(dom)))
- return -1;
-
- vm->hasManagedSave = false;
-
- virDomainObjEndAPI(&vm);
- return 0;
-}
-
-
/*
* Snapshot APIs
*/
--
2.4.1
7:43 p.m.
New subject: [libvirt] [PATCH 6/8] lib: setvcpus: Remove bogous flag check
Since VIR_DOMAIN_AFFECT_CURRENT is 0 the flag check does not make sense
as masking @flags with 0 will always equal to false.
---
src/libvirt-domain.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 7e6d749..4d7b88a 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -7267,12 +7267,6 @@ virDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus,
VIR_DOMAIN_AFFECT_CONFIG,
error);
- VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_CURRENT,
- VIR_DOMAIN_AFFECT_LIVE,
- error);
- VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_CURRENT,
- VIR_DOMAIN_AFFECT_CONFIG,
- error);
VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_VCPU_GUEST,
VIR_DOMAIN_AFFECT_CONFIG,
error);
--
2.4.1
10:13 a.m.
New subject: [libvirt] [PATCH 6/8] lib: setvcpus: Remove bogous flag check
On Tue, Jun 16, 2015 at 07:43:18PM +0200, Peter Krempa wrote:
Since VIR_DOMAIN_AFFECT_CURRENT is 0 the flag check does not make
sense
as masking @flags with 0 will always equal to false.
---
src/libvirt-domain.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 7e6d749..4d7b88a 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -7267,12 +7267,6 @@ virDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus,
VIR_DOMAIN_AFFECT_CONFIG,
error);
- VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_CURRENT,
- VIR_DOMAIN_AFFECT_LIVE,
- error);
- VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_CURRENT,
- VIR_DOMAIN_AFFECT_CONFIG,
- error);
VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_VCPU_GUEST,
VIR_DOMAIN_AFFECT_CONFIG,
error);
--
2.4.1
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
ACK
Pavel
7:43 p.m.
New subject: [libvirt] [PATCH 7/8] test: Refactor testDomainSetVcpusFlags
Remove the bogous flag check and refactor the code by using
virDomainObjGetDefs instead of virDomainObjGetPersistentDef.
---
src/test/test_driver.c | 68 +++++++++++++++++---------------------------------
1 file changed, 23 insertions(+), 45 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 59e2031..cfec122 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2509,6 +2509,7 @@ testDomainSetVcpusFlags(virDomainPtr domain, unsigned int nrCpus,
{
testConnPtr privconn = domain->conn->privateData;
virDomainObjPtr privdom = NULL;
+ virDomainDefPtr def;
virDomainDefPtr persistentDef;
int ret = -1, maxvcpus;
@@ -2516,72 +2517,49 @@ testDomainSetVcpusFlags(virDomainPtr domain, unsigned int nrCpus,
VIR_DOMAIN_AFFECT_CONFIG |
VIR_DOMAIN_VCPU_MAXIMUM, -1);
- /* At least one of LIVE or CONFIG must be set. MAXIMUM cannot be
- * mixed with LIVE. */
- if ((flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG)) == 0 ||
- (flags & (VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_AFFECT_LIVE)) ==
- (VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_AFFECT_LIVE)) {
- virReportError(VIR_ERR_INVALID_ARG,
- _("invalid flag combination: (0x%x)"), flags);
+ if ((maxvcpus = testConnectGetMaxVcpus(domain->conn, NULL)) == -1)
return -1;
- }
- if (!nrCpus || (maxvcpus = testConnectGetMaxVcpus(domain->conn, NULL)) <
nrCpus) {
+
+ if (nrCpus > maxvcpus) {
virReportError(VIR_ERR_INVALID_ARG,
- _("argument out of range: %d"), nrCpus);
+ _("requested cpu amount exceeds maximum supported amount
"
+ "(%d > %d)"), nrCpus, maxvcpus);
return -1;
}
if (!(privdom = testDomObjFromDomain(domain)))
return -1;
- if (!virDomainObjIsActive(privdom) && (flags & VIR_DOMAIN_AFFECT_LIVE))
{
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("cannot hotplug vcpus for an inactive
domain"));
+ if (virDomainObjGetDefs(privdom, flags, &def, &persistentDef) < 0)
goto cleanup;
- }
-
- /* We allow more cpus in guest than host, but not more than the
- * domain's starting limit. */
- if (!(flags & (VIR_DOMAIN_VCPU_MAXIMUM)) &&
- privdom->def->maxvcpus < maxvcpus)
- maxvcpus = privdom->def->maxvcpus;
- if (nrCpus > maxvcpus) {
+ if ((def &&
+ def->maxvcpus < nrCpus) ||
+ (persistentDef &&
+ !(flags & VIR_DOMAIN_VCPU_MAXIMUM) &&
+ persistentDef->maxvcpus < nrCpus)) {
virReportError(VIR_ERR_INVALID_ARG,
_("requested cpu amount exceeds maximum (%d > %d)"),
nrCpus, maxvcpus);
goto cleanup;
}
- if (!(persistentDef = virDomainObjGetPersistentDef(privconn->caps,
- privconn->xmlopt,
- privdom)))
+ if (def &&
+ testDomainUpdateVCPUs(privconn, privdom, nrCpus, 0) < 0)
goto cleanup;
- switch (flags) {
- case VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_AFFECT_CONFIG:
- persistentDef->maxvcpus = nrCpus;
- if (nrCpus < persistentDef->vcpus)
- persistentDef->vcpus = nrCpus;
- ret = 0;
- break;
-
- case VIR_DOMAIN_AFFECT_CONFIG:
- persistentDef->vcpus = nrCpus;
- ret = 0;
- break;
-
- case VIR_DOMAIN_AFFECT_LIVE:
- ret = testDomainUpdateVCPUs(privconn, privdom, nrCpus, 0);
- break;
-
- case VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG:
- ret = testDomainUpdateVCPUs(privconn, privdom, nrCpus, 0);
- if (ret == 0)
+ if (persistentDef) {
+ if (flags & VIR_DOMAIN_VCPU_MAXIMUM) {
+ persistentDef->maxvcpus = nrCpus;
+ if (nrCpus < persistentDef->vcpus)
+ persistentDef->vcpus = nrCpus;
+ } else {
persistentDef->vcpus = nrCpus;
- break;
+ }
}
+ ret = 0;
+
cleanup:
virDomainObjEndAPI(&privdom);
return ret;
--
2.4.1
4:56 p.m.
New subject: [libvirt] [PATCH 7/8] test: Refactor testDomainSetVcpusFlags
On Tue, Jun 16, 2015 at 07:43:19PM +0200, Peter Krempa wrote:
Remove the bogous flag check and refactor the code by using
s/bogous/bogus
virDomainObjGetDefs instead of virDomainObjGetPersistentDef.
---
src/test/test_driver.c | 68 +++++++++++++++++---------------------------------
1 file changed, 23 insertions(+), 45 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 59e2031..cfec122 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
- if (nrCpus > maxvcpus) {
+ if ((def &&
+ def->maxvcpus < nrCpus) ||
+ (persistentDef &&
+ !(flags & VIR_DOMAIN_VCPU_MAXIMUM) &&
+ persistentDef->maxvcpus < nrCpus)) {
virReportError(VIR_ERR_INVALID_ARG,
_("requested cpu amount exceeds maximum (%d >
%d)"),
nrCpus, maxvcpus);
The comparison uses {persistentD,d}ef->maxvcpus, but the error message
shows maxvcpus.
goto cleanup;
}
- if (!(persistentDef = virDomainObjGetPersistentDef(privconn->caps,
- privconn->xmlopt,
- privdom)))
+ if (def &&
+ testDomainUpdateVCPUs(privconn, privdom, nrCpus, 0) < 0)
goto cleanup;
ACK with those fixes.
Jan
7:43 p.m.
New subject: [libvirt] [PATCH 8/8] test: Fix lock ordering in testDomainRevertToSnapshot
The test driver lock should not be acquired while a domain object lock
is held. Tweak the lock ordering to avoid possible deadlock.
---
src/test/test_driver.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index cfec122..9e617a2 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -6603,13 +6603,13 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
* and use of FORCE can cause multiple transitions.
*/
- if (!(vm = testDomObjFromSnapshot(snapshot)))
- return -1;
+ testDriverLock(privconn);
- if (!(snap = testSnapObjFromSnapshot(vm, snapshot)))
+ if (!(vm = testDomObjFromDomainLocked(privconn, snapshot->domain)))
goto cleanup;
- testDriverLock(privconn);
+ if (!(snap = testSnapObjFromSnapshot(vm, snapshot)))
+ goto cleanup;
if (!vm->persistent &&
snap->def->state != VIR_DOMAIN_RUNNING &&
--
2.4.1
4:45 p.m.
New subject: [libvirt] [PATCH 8/8] test: Fix lock ordering in testDomainRevertToSnapshot
On 16.06.2015 19:43, Peter Krempa wrote:
The test driver lock should not be acquired while a domain object
lock
is held. Tweak the lock ordering to avoid possible deadlock.
---
src/test/test_driver.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index cfec122..9e617a2 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -6603,13 +6603,13 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
* and use of FORCE can cause multiple transitions.
*/
- if (!(vm = testDomObjFromSnapshot(snapshot)))
- return -1;
+ testDriverLock(privconn);
- if (!(snap = testSnapObjFromSnapshot(vm, snapshot)))
+ if (!(vm = testDomObjFromDomainLocked(privconn, snapshot->domain)))
goto cleanup;
- testDriverLock(privconn);
+ if (!(snap = testSnapObjFromSnapshot(vm, snapshot)))
+ goto cleanup;
if (!vm->persistent &&
snap->def->state != VIR_DOMAIN_RUNNING &&
Fortunately, the public API already checked that @snapshot is not NULL
and snapshot->domain is of virDomainClass class.
ACK
Michal
4:45 p.m.
On 16.06.2015 19:43, Peter Krempa wrote:
Note that patch 1/8 of this series depends on patch 2/13 of the
"vCPU pinning and related refactors - Part 1.5" series:
http://www.redhat.com/archives/libvir-list/2015-June/msg00678.html
Peter Krempa (8):
test: Reuse virDomainObjGetOneDef in testDomainGetVcpusFlags
test: Switch to reference counting with testDomObjFromDomain
test: Refactor test driver domain object retrieval
test: Refactor test driver event sending
test: group domain APIs together
lib: setvcpus: Remove bogous flag check
test: Refactor testDomainSetVcpusFlags
test: Fix lock ordering in testDomainRevertToSnapshot
src/libvirt-domain.c | 6 -
src/test/test_driver.c | 941 +++++++++++++++++--------------------------------
2 files changed, 324 insertions(+), 623 deletions(-)
Basically ACK series, but see my comments.
Michal
3380
days inactive
3383
days old
15 comments
4 participants
participants (4)
-
Ján Tomko -
Michal Privoznik -
Pavel Hrdina -
Peter Krempa