Re: [libvirt] [PATCH v3 2/8] test: Wire up managed save APIs

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