On 09/26/2013 10:44 AM, Michal Privoznik wrote:
On 25.09.2013 21:15, Cole Robinson wrote:
Also add a <test:hasmanagedsave> element to set this data when starting the connection. --- v3: Mandate managedsave have runstate=shutoff
src/test/test_driver.c | 134 ++++++++++++++++++++++++++++++++++++++++++++++++- tests/virshtest.c | 2 +- 2 files changed, 134 insertions(+), 2 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index c51c7ca..7e60716 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -160,6 +160,7 @@ typedef testDomainNamespaceDef *testDomainNamespaceDefPtr; struct _testDomainNamespaceDef { int runstate; bool transient; + bool hasManagedSave; };
static void @@ -197,6 +198,13 @@ testDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, } nsdata->transient = tmp;
+ tmp = virXPathBoolean("boolean(./test:hasmanagedsave)", ctxt); + if (tmp == -1) { + virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid hasmanagedsave")); + goto error; + } + nsdata->hasManagedSave = tmp; + tmp = virXPathUInt("string(./test:runstate)", ctxt, &tmpuint); if (tmp == 0) { if (tmpuint >= VIR_DOMAIN_LAST) { @@ -218,6 +226,11 @@ testDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, _("transient domain cannot have runstate 'shutoff'")); goto error; } + if (nsdata->hasManagedSave && nsdata->runstate != VIR_DOMAIN_SHUTOFF) { + virReportError(VIR_ERR_XML_ERROR,
s/$/ "%s",/
+ _("domain with managedsave data can only have runstate 'shutoff'")); + goto error; + }
@@ -2836,6 +2851,15 @@ static int testDomainUndefineFlags(virDomainPtr domain, goto cleanup; }
+ if (privdom->hasManagedSave && + !(flags & VIR_DOMAIN_UNDEFINE_MANAGED_SAVE)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Refusing to undefine while domain managed " + "save image exists")); + goto cleanup; + } + privdom->hasManagedSave = false;
While this assignment is safe for now (there's not goto cleanup afterwards) I'd rather see it more closer to the ret = 0; line. What if in the future we insert something in between?
+ event = virDomainEventNewFromObj(privdom, VIR_DOMAIN_EVENT_UNDEFINED, VIR_DOMAIN_EVENT_UNDEFINED_REMOVED); @@ -5969,6 +5993,111 @@ 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; + virDomainEventPtr event = NULL; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE | + 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 (!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 = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_STOPPED, + VIR_DOMAIN_EVENT_STOPPED_SAVED); + vm->hasManagedSave = true; + + ret = 0;
This ^^^ is what I have in my mind.
+cleanup: + if (vm) + virObjectUnlock(vm); + if (event) { + testDriverLock(privconn); + testDomainEventQueue(privconn, event); + testDriverUnlock(privconn); + } + + return ret; +} + +
+static int +testDomainHasManagedSaveImage(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; + } + + ret = vm->hasManagedSave; +cleanup: + if (vm) + virObjectUnlock(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; + } + + vm->hasManagedSave = false; + ret = 0; +cleanup: + if (vm) + virObjectUnlock(vm); + testDriverUnlock(privconn); + return ret; +}
The driver can be unlocked right after virDomainObjListFindByName(). But who cares about throughput in test driver, right?
ACK if you squash this in:
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 77c3d23..f40a841 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -227,7 +227,7 @@ testDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, goto error; } if (nsdata->hasManagedSave && nsdata->runstate != VIR_DOMAIN_SHUTOFF) { - virReportError(VIR_ERR_XML_ERROR, + virReportError(VIR_ERR_XML_ERROR, "%s", _("domain with managedsave data can only have runstate 'shutoff'")); goto error; }
Michal
Thanks, pushed with that bit squashed in, and the above mentioned assignment moved after the UNDEFINE event. - Cole