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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list