[libvirt] [PATCH v2 0/9] test: Mock snapshot APIs and misc improvements

This series implements snapshot and managed save APIs for the test driver, and adds some misc improvements, like specifying domain state in the passed in driver XML. v2: Use a separate XML namespace for the custom XML additions Add snapshot REDEFINE support, sharing the generic logic with qemu Cole Robinson (9): test: Allow specifying object runstate in driver XML test: Allow specifying object transient state in driver XML test: Wire up managed save APIs test: Implement readonly snapshot APIs test: Implement snapshot create/delete/revert APIs qemu: snapshots: Simplify REDEFINE flag check qemu: snapshot: Break out redefine preparation to shared function test: snapshot: Add REDEFINE support test: Allow specifying domainsnapshot XML src/conf/domain_conf.c | 3 +- src/conf/snapshot_conf.c | 150 ++++++ src/conf/snapshot_conf.h | 7 + src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 145 +----- src/test/test_driver.c | 1236 +++++++++++++++++++++++++++++++++++++++++++++- tests/virshtest.c | 2 +- 7 files changed, 1399 insertions(+), 145 deletions(-) -- 1.8.3.1

When passing in custom driver XML, allow a block like <domain xmlns:test='http://libvirt.org/schemas/domain/test/1.0'> ... <test:runstate>5</test:runstate> </domain> This is only read at initial driver start time, and sets the initial run state of the object. This is handy for UI testing. It's only wired up for domains, since that's the only conf/ infrastructure that supports namespaces at the moment. --- src/test/test_driver.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 84 insertions(+), 7 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index c225618..fdfb13a 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -30,6 +30,7 @@ #include <unistd.h> #include <sys/stat.h> #include <libxml/xmlsave.h> +#include <libxml/xpathInternals.h> #include "virerror.h" @@ -151,13 +152,81 @@ static void testDomainObjPrivateFree(void *data) VIR_FREE(priv); } +#define TEST_NAMESPACE_HREF "http://libvirt.org/schemas/domain/test/1.0" + +typedef struct _testDomainNamespaceDef testDomainNamespaceDef; +typedef testDomainNamespaceDef *testDomainNamespaceDefPtr; +struct _testDomainNamespaceDef { + int runstate; +}; + +static void +testDomainDefNamespaceFree(void *data) +{ + testDomainNamespaceDefPtr nsdata = data; + VIR_FREE(nsdata); +} + +static int +testDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, + xmlNodePtr root ATTRIBUTE_UNUSED, + xmlXPathContextPtr ctxt, + void **data) +{ + testDomainNamespaceDefPtr nsdata = NULL; + int tmp; + unsigned int tmpuint; + + if (xmlXPathRegisterNs(ctxt, BAD_CAST "test", + BAD_CAST TEST_NAMESPACE_HREF) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to register xml namespace '%s'"), + TEST_NAMESPACE_HREF); + return -1; + } + + if (VIR_ALLOC(nsdata) < 0) + return -1; + + tmp = virXPathUInt("string(./test:runstate)", ctxt, &tmpuint); + if (tmp == 0) { + if (tmpuint >= VIR_DOMAIN_LAST) { + virReportError(VIR_ERR_XML_ERROR, + _("runstate '%d' out of range'"), tmpuint); + goto error; + } + nsdata->runstate = tmpuint; + } else if (tmp == -1) { + nsdata->runstate = VIR_DOMAIN_RUNNING; + } else if (tmp == -2) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid runstate")); + goto error; + } + + *data = nsdata; + return 0; + +error: + testDomainDefNamespaceFree(nsdata); + return -1; +} static virDomainXMLOptionPtr testBuildXMLConfig(void) { - virDomainXMLPrivateDataCallbacks priv = { .alloc = testDomainObjPrivateAlloc, - .free = testDomainObjPrivateFree }; - return virDomainXMLOptionNew(NULL, &priv, NULL); + virDomainXMLPrivateDataCallbacks priv = { + .alloc = testDomainObjPrivateAlloc, + .free = testDomainObjPrivateFree + }; + + /* All our XML extensions are write only, so we only need to parse */ + virDomainXMLNamespace ns = { + .parse = testDomainDefNamespaceParse, + .free = testDomainDefNamespaceFree, + }; + + return virDomainXMLOptionNew(NULL, &priv, &ns); } @@ -830,6 +899,7 @@ testParseDomains(testConnPtr privconn, for (i = 0; i < num; i++) { virDomainDefPtr def; + testDomainNamespaceDefPtr nsdata; xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file, "domain"); if (!node) goto error; @@ -850,12 +920,19 @@ testParseDomains(testConnPtr privconn, goto error; } + nsdata = def->namespaceData; obj->persistent = 1; - if (testDomainStartState(privconn, obj, - VIR_DOMAIN_RUNNING_BOOTED) < 0) { - virObjectUnlock(obj); - goto error; + + if (nsdata->runstate != VIR_DOMAIN_SHUTOFF) { + if (testDomainStartState(privconn, obj, + VIR_DOMAIN_RUNNING_BOOTED) < 0) { + virObjectUnlock(obj); + goto error; + } + } else { + testDomainShutdownState(NULL, obj, 0); } + virDomainObjSetState(obj, nsdata->runstate, 0); virObjectUnlock(obj); } -- 1.8.3.1

On 08/30/2013 10:03 AM, Cole Robinson wrote:
When passing in custom driver XML, allow a block like
<domain xmlns:test='http://libvirt.org/schemas/domain/test/1.0'> ... <test:runstate>5</test:runstate>
Since the enum virDomainState is part of our public API, I'm not worried about the numbers ever being tied to the wrong state. But wouldn't it be nicer for the XML to use a string conversion of a name, rather than just a raw number? On the other hand, this is just for testing purposes, so I can live with it.
</domain>
This is only read at initial driver start time, and sets the initial run state of the object. This is handy for UI testing.
It's only wired up for domains, since that's the only conf/ infrastructure that supports namespaces at the moment. --- src/test/test_driver.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 84 insertions(+), 7 deletions(-)
+ + tmp = virXPathUInt("string(./test:runstate)", ctxt, &tmpuint); + if (tmp == 0) { + if (tmpuint >= VIR_DOMAIN_LAST) { + virReportError(VIR_ERR_XML_ERROR, + _("runstate '%d' out of range'"), tmpuint); + goto error; + } + nsdata->runstate = tmpuint;
Should we also reject VIR_DOMAIN_NOSTATE?
+ }; + + /* All our XML extensions are write only, so we only need to parse */
Maybe s/write only/input only/ ACK, but not until after the release. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 08/30/2013 12:43 PM, Eric Blake wrote:
On 08/30/2013 10:03 AM, Cole Robinson wrote:
When passing in custom driver XML, allow a block like
<domain xmlns:test='http://libvirt.org/schemas/domain/test/1.0'> ... <test:runstate>5</test:runstate>
Since the enum virDomainState is part of our public API, I'm not worried about the numbers ever being tied to the wrong state. But wouldn't it be nicer for the XML to use a string conversion of a name, rather than just a raw number? On the other hand, this is just for testing purposes, so I can live with it.
Yeah I think for something like this the only people who will use it are capable of getting the domain state numbers, and it keeps the code simpler.
</domain>
This is only read at initial driver start time, and sets the initial run state of the object. This is handy for UI testing.
It's only wired up for domains, since that's the only conf/ infrastructure that supports namespaces at the moment. --- src/test/test_driver.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 84 insertions(+), 7 deletions(-)
+ + tmp = virXPathUInt("string(./test:runstate)", ctxt, &tmpuint); + if (tmp == 0) { + if (tmpuint >= VIR_DOMAIN_LAST) { + virReportError(VIR_ERR_XML_ERROR, + _("runstate '%d' out of range'"), tmpuint); + goto error; + } + nsdata->runstate = tmpuint;
Should we also reject VIR_DOMAIN_NOSTATE?
But isn't it a valid state? I know xen used to return it quite a bit but that was ages ago. But if any libvirt version returned it then it's useful for apps to test.
+ }; + + /* All our XML extensions are write only, so we only need to parse */
Maybe s/write only/input only/
ACK, but not until after the release.
Thanks, pushed now with that change. - Cole

Similar to the runstate commit, allow a boolean <test:transient/> element for setting domain persistence at driver startup. --- src/test/test_driver.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index fdfb13a..a181a9b 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -158,6 +158,7 @@ typedef struct _testDomainNamespaceDef testDomainNamespaceDef; typedef testDomainNamespaceDef *testDomainNamespaceDefPtr; struct _testDomainNamespaceDef { int runstate; + bool transient; }; static void @@ -188,6 +189,13 @@ testDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, if (VIR_ALLOC(nsdata) < 0) return -1; + tmp = virXPathBoolean("boolean(./test:transient)", ctxt); + if (tmp == -1) { + virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid transient")); + goto error; + } + nsdata->transient = tmp; + tmp = virXPathUInt("string(./test:runstate)", ctxt, &tmpuint); if (tmp == 0) { if (tmpuint >= VIR_DOMAIN_LAST) { @@ -921,7 +929,7 @@ testParseDomains(testConnPtr privconn, } nsdata = def->namespaceData; - obj->persistent = 1; + obj->persistent = !nsdata->transient; if (nsdata->runstate != VIR_DOMAIN_SHUTOFF) { if (testDomainStartState(privconn, obj, -- 1.8.3.1

On 08/30/2013 10:03 AM, Cole Robinson wrote:
Similar to the runstate commit, allow a boolean <test:transient/> element for setting domain persistence at driver startup. --- src/test/test_driver.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
What happens if I specify <test:runstate>5</test:runstate><test:transient/> (ie. a transient shutoff domain)? That should be rejected as an unsupported combination, but I don't see that here. ACK with that fixed, but post-1.1.2. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Also add a <test:hasmanagedsave> element to set this data when starting the connection. --- src/test/test_driver.c | 129 ++++++++++++++++++++++++++++++++++++++++++++++++- tests/virshtest.c | 2 +- 2 files changed, 129 insertions(+), 2 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index a181a9b..2bcdd64 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -159,6 +159,7 @@ typedef testDomainNamespaceDef *testDomainNamespaceDefPtr; struct _testDomainNamespaceDef { int runstate; bool transient; + bool hasManagedSave; }; static void @@ -196,6 +197,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) { @@ -581,6 +589,7 @@ testDomainStartState(testConnPtr privconn, goto cleanup; } + dom->hasManagedSave = false; ret = 0; cleanup: if (ret < 0) @@ -930,6 +939,7 @@ testParseDomains(testConnPtr privconn, nsdata = def->namespaceData; obj->persistent = !nsdata->transient; + obj->hasManagedSave = nsdata->hasManagedSave; if (nsdata->runstate != VIR_DOMAIN_SHUTOFF) { if (testDomainStartState(privconn, obj, @@ -2752,7 +2762,7 @@ static int testDomainUndefineFlags(virDomainPtr domain, virDomainEventPtr event = NULL; int ret = -1; - virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE, -1); testDriverLock(privconn); privdom = virDomainObjListFindByName(privconn->domains, @@ -2763,6 +2773,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; + event = virDomainEventNewFromObj(privdom, VIR_DOMAIN_EVENT_UNDEFINED, VIR_DOMAIN_EVENT_UNDEFINED_REMOVED); @@ -5887,6 +5906,111 @@ testDomainScreenshot(virDomainPtr dom ATTRIBUTE_UNUSED, } +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; +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; +} + + static virDriver testDriver = { .no = VIR_DRV_TEST, .name = "Test", @@ -5957,6 +6081,9 @@ static virDriver testDriver = { .connectIsAlive = testConnectIsAlive, /* 0.9.8 */ .nodeGetCPUMap = testNodeGetCPUMap, /* 1.0.0 */ .domainScreenshot = testDomainScreenshot, /* 1.0.5 */ + .domainManagedSave = testDomainManagedSave, /* 1.1.2 */ + .domainHasManagedSaveImage = testDomainHasManagedSaveImage, /* 1.1.2 */ + .domainManagedSaveRemove = testDomainManagedSaveRemove, /* 1.1.2 */ }; static virNetworkDriver testNetworkDriver = { diff --git a/tests/virshtest.c b/tests/virshtest.c index ca35bb0..fe255d3 100644 --- a/tests/virshtest.c +++ b/tests/virshtest.c @@ -34,7 +34,7 @@ Max memory: 261072 KiB\n\ Used memory: 131072 KiB\n\ Persistent: yes\n\ Autostart: disable\n\ -Managed save: unknown\n\ +Managed save: no\n\ \n"; static const char *domuuid_fc4 = DOM_UUID "\n\n"; static const char *domid_fc4 = "2\n\n"; -- 1.8.3.1

On 08/30/2013 10:03 AM, Cole Robinson wrote:
Also add a <test:hasmanagedsave> element to set this data when starting the connection. --- src/test/test_driver.c | 129 ++++++++++++++++++++++++++++++++++++++++++++++++- tests/virshtest.c | 2 +- 2 files changed, 129 insertions(+), 2 deletions(-)
Cool. Same problem as in patch 2 - managedsave makes sense only for a shutoff persistent domain...
+ tmp = virXPathBoolean("boolean(./test:hasmanagedsave)", ctxt); + if (tmp == -1) { + virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid hasmanagedsave")); + goto error; + } + nsdata->hasManagedSave = tmp;
...so the input parser should reject the flag if it is inconsistent with the domain state or specified at the same time as transient. ACK with that fixed. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

This is just stolen from qemu_driver.c with tweaks to fit the test driver. --- src/test/test_driver.c | 392 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 392 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 2bcdd64..fd2ff1b 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -45,6 +45,7 @@ #include "interface_conf.h" #include "domain_conf.h" #include "domain_event.h" +#include "snapshot_conf.h" #include "fdstream.h" #include "storage_conf.h" #include "node_device_conf.h" @@ -411,6 +412,27 @@ static const unsigned long long defaultPoolAlloc = 0; static int testStoragePoolObjSetDefaults(virStoragePoolObjPtr pool); static int testNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info); +static virDomainObjPtr +testDomObjFromDomain(virDomainPtr domain) +{ + virDomainObjPtr vm; + testConnPtr driver = domain->conn->privateData; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + testDriverLock(driver); + vm = virDomainObjListFindByUUID(driver->domains, domain->uuid); + if (!vm) { + virUUIDFormat(domain->uuid, uuidstr); + virReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s' (%s)"), + uuidstr, domain->name); + vm = NULL; + } + + testDriverUnlock(driver); + return vm; +} + static char * testDomainGenerateIfname(virDomainDefPtr domdef) { int maxif = 1024; @@ -6011,6 +6033,362 @@ cleanup: } +/* + * Snapshot APIs + */ + +static virDomainSnapshotObjPtr +testSnapObjFromName(virDomainObjPtr vm, + const char *name) +{ + virDomainSnapshotObjPtr snap = NULL; + snap = virDomainSnapshotFindByName(vm->snapshots, name); + if (!snap) + virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, + _("no domain snapshot with matching name '%s'"), + name); + return snap; +} + +static virDomainSnapshotObjPtr +testSnapObjFromSnapshot(virDomainObjPtr vm, + virDomainSnapshotPtr snapshot) +{ + return testSnapObjFromName(vm, snapshot->name); +} + +static virDomainObjPtr +testDomObjFromSnapshot(virDomainSnapshotPtr snapshot) +{ + return testDomObjFromDomain(snapshot->domain); +} + +static int +testDomainSnapshotNum(virDomainPtr domain, unsigned int flags) +{ + virDomainObjPtr vm = NULL; + int n = -1; + + virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_ROOTS | + VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); + + if (!(vm = testDomObjFromDomain(domain))) + goto cleanup; + + n = virDomainSnapshotObjListNum(vm->snapshots, NULL, flags); + +cleanup: + if (vm) + virObjectUnlock(vm); + return n; +} + +static int +testDomainSnapshotListNames(virDomainPtr domain, + char **names, + int nameslen, + unsigned int flags) +{ + virDomainObjPtr vm = NULL; + int n = -1; + + virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_ROOTS | + VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); + + if (!(vm = testDomObjFromDomain(domain))) + goto cleanup; + + n = virDomainSnapshotObjListGetNames(vm->snapshots, NULL, names, nameslen, + flags); + +cleanup: + if (vm) + virObjectUnlock(vm); + return n; +} + +static int +testDomainListAllSnapshots(virDomainPtr domain, + virDomainSnapshotPtr **snaps, + unsigned int flags) +{ + virDomainObjPtr vm = NULL; + int n = -1; + + virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_ROOTS | + VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); + + if (!(vm = testDomObjFromDomain(domain))) + goto cleanup; + + n = virDomainListSnapshots(vm->snapshots, NULL, domain, snaps, flags); + +cleanup: + if (vm) + virObjectUnlock(vm); + return n; +} + +static int +testDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot, + char **names, + int nameslen, + unsigned int flags) +{ + virDomainObjPtr vm = NULL; + virDomainSnapshotObjPtr snap = NULL; + int n = -1; + + virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS | + VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); + + if (!(vm = testDomObjFromSnapshot(snapshot))) + goto cleanup; + + if (!(snap = testSnapObjFromSnapshot(vm, snapshot))) + goto cleanup; + + n = virDomainSnapshotObjListGetNames(vm->snapshots, snap, names, nameslen, + flags); + +cleanup: + if (vm) + virObjectUnlock(vm); + return n; +} + +static int +testDomainSnapshotNumChildren(virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + virDomainObjPtr vm = NULL; + virDomainSnapshotObjPtr snap = NULL; + int n = -1; + + virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS | + VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); + + if (!(vm = testDomObjFromSnapshot(snapshot))) + goto cleanup; + + if (!(snap = testSnapObjFromSnapshot(vm, snapshot))) + goto cleanup; + + n = virDomainSnapshotObjListNum(vm->snapshots, snap, flags); + +cleanup: + if (vm) + virObjectUnlock(vm); + return n; +} + +static int +testDomainSnapshotListAllChildren(virDomainSnapshotPtr snapshot, + virDomainSnapshotPtr **snaps, + unsigned int flags) +{ + virDomainObjPtr vm = NULL; + virDomainSnapshotObjPtr snap = NULL; + int n = -1; + + virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS | + VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); + + if (!(vm = testDomObjFromSnapshot(snapshot))) + goto cleanup; + + if (!(snap = testSnapObjFromSnapshot(vm, snapshot))) + goto cleanup; + + n = virDomainListSnapshots(vm->snapshots, snap, snapshot->domain, snaps, + flags); + +cleanup: + if (vm) + virObjectUnlock(vm); + return n; +} + +static virDomainSnapshotPtr +testDomainSnapshotLookupByName(virDomainPtr domain, + const char *name, + unsigned int flags) +{ + virDomainObjPtr vm; + virDomainSnapshotObjPtr snap = NULL; + virDomainSnapshotPtr snapshot = NULL; + + virCheckFlags(0, NULL); + + if (!(vm = testDomObjFromDomain(domain))) + goto cleanup; + + if (!(snap = testSnapObjFromName(vm, name))) + goto cleanup; + + snapshot = virGetDomainSnapshot(domain, snap->def->name); + +cleanup: + if (vm) + virObjectUnlock(vm); + return snapshot; +} + +static int +testDomainHasCurrentSnapshot(virDomainPtr domain, + unsigned int flags) +{ + virDomainObjPtr vm; + int ret = -1; + + virCheckFlags(0, -1); + + if (!(vm = testDomObjFromDomain(domain))) + goto cleanup; + + ret = (vm->current_snapshot != NULL); + +cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + +static virDomainSnapshotPtr +testDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + virDomainObjPtr vm; + virDomainSnapshotObjPtr snap = NULL; + virDomainSnapshotPtr parent = NULL; + + virCheckFlags(0, NULL); + + if (!(vm = testDomObjFromSnapshot(snapshot))) + goto cleanup; + + if (!(snap = testSnapObjFromSnapshot(vm, snapshot))) + goto cleanup; + + if (!snap->def->parent) { + virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, + _("snapshot '%s' does not have a parent"), + snap->def->name); + goto cleanup; + } + + parent = virGetDomainSnapshot(snapshot->domain, snap->def->parent); + +cleanup: + if (vm) + virObjectUnlock(vm); + return parent; +} + +static virDomainSnapshotPtr +testDomainSnapshotCurrent(virDomainPtr domain, + unsigned int flags) +{ + virDomainObjPtr vm; + virDomainSnapshotPtr snapshot = NULL; + + virCheckFlags(0, NULL); + + if (!(vm = testDomObjFromDomain(domain))) + goto cleanup; + + if (!vm->current_snapshot) { + virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, "%s", + _("the domain does not have a current snapshot")); + goto cleanup; + } + + snapshot = virGetDomainSnapshot(domain, vm->current_snapshot->def->name); + +cleanup: + if (vm) + virObjectUnlock(vm); + return snapshot; +} + +static char * +testDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + virDomainObjPtr vm = NULL; + char *xml = NULL; + virDomainSnapshotObjPtr snap = NULL; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL); + + if (!(vm = testDomObjFromSnapshot(snapshot))) + goto cleanup; + + if (!(snap = testSnapObjFromSnapshot(vm, snapshot))) + goto cleanup; + + virUUIDFormat(snapshot->domain->uuid, uuidstr); + + xml = virDomainSnapshotDefFormat(uuidstr, snap->def, flags, 0); + +cleanup: + if (vm) + virObjectUnlock(vm); + return xml; +} + +static int +testDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + virDomainObjPtr vm = NULL; + int ret = -1; + virDomainSnapshotObjPtr snap = NULL; + + virCheckFlags(0, -1); + + if (!(vm = testDomObjFromSnapshot(snapshot))) + goto cleanup; + + if (!(snap = testSnapObjFromSnapshot(vm, snapshot))) + goto cleanup; + + ret = (vm->current_snapshot && + STREQ(snapshot->name, vm->current_snapshot->def->name)); + +cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + + +static int +testDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + virDomainObjPtr vm = NULL; + int ret = -1; + virDomainSnapshotObjPtr snap = NULL; + + virCheckFlags(0, -1); + + if (!(vm = testDomObjFromSnapshot(snapshot))) + goto cleanup; + + if (!(snap = testSnapObjFromSnapshot(vm, snapshot))) + goto cleanup; + + ret = 1; + +cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + + static virDriver testDriver = { .no = VIR_DRV_TEST, .name = "Test", @@ -6084,6 +6462,20 @@ static virDriver testDriver = { .domainManagedSave = testDomainManagedSave, /* 1.1.2 */ .domainHasManagedSaveImage = testDomainHasManagedSaveImage, /* 1.1.2 */ .domainManagedSaveRemove = testDomainManagedSaveRemove, /* 1.1.2 */ + + .domainSnapshotNum = testDomainSnapshotNum, /* 1.1.2 */ + .domainSnapshotListNames = testDomainSnapshotListNames, /* 1.1.2 */ + .domainListAllSnapshots = testDomainListAllSnapshots, /* 1.1.2 */ + .domainSnapshotGetXMLDesc = testDomainSnapshotGetXMLDesc, /* 1.1.2 */ + .domainSnapshotNumChildren = testDomainSnapshotNumChildren, /* 1.1.2 */ + .domainSnapshotListChildrenNames = testDomainSnapshotListChildrenNames, /* 1.1.2 */ + .domainSnapshotListAllChildren = testDomainSnapshotListAllChildren, /* 1.1.2 */ + .domainSnapshotLookupByName = testDomainSnapshotLookupByName, /* 1.1.2 */ + .domainHasCurrentSnapshot = testDomainHasCurrentSnapshot, /* 1.1.2 */ + .domainSnapshotGetParent = testDomainSnapshotGetParent, /* 1.1.2 */ + .domainSnapshotCurrent = testDomainSnapshotCurrent, /* 1.1.2 */ + .domainSnapshotIsCurrent = testDomainSnapshotIsCurrent, /* 1.1.2 */ + .domainSnapshotHasMetadata = testDomainSnapshotHasMetadata, /* 1.1.2 */ }; static virNetworkDriver testNetworkDriver = { -- 1.8.3.1

On 08/30/2013 10:03 AM, Cole Robinson wrote:
This is just stolen from qemu_driver.c with tweaks to fit the test driver. --- src/test/test_driver.c | 392 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 392 insertions(+)
Looks fairly straightforward. ACK post-1.1.2. (Of course, with this patch alone, there are no snapshots to be listed, so the new API don't do anything interesting yet... :)
+ +static int +testDomainSnapshotListNames(virDomainPtr domain, + char **names, + int nameslen, + unsigned int flags) +{ + virDomainObjPtr vm = NULL; + int n = -1; + + virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_ROOTS | + VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); +
Are we sure that all of the filters are supported? I guess all of them will work, even if some of them always land one way and not another (for example, if you declare that all test snapshots have no metadata, then the VIR_DOMAIN_SNAPSHOT_LIST_METADATA filter will be rather boring).
+ .domainSnapshotNum = testDomainSnapshotNum, /* 1.1.2 */ + .domainSnapshotListNames = testDomainSnapshotListNames, /* 1.1.2 */ + .domainListAllSnapshots = testDomainListAllSnapshots, /* 1.1.2 */ + .domainSnapshotGetXMLDesc = testDomainSnapshotGetXMLDesc, /* 1.1.2 */ + .domainSnapshotNumChildren = testDomainSnapshotNumChildren, /* 1.1.2 */ + .domainSnapshotListChildrenNames = testDomainSnapshotListChildrenNames, /* 1.1.2 */ + .domainSnapshotListAllChildren = testDomainSnapshotListAllChildren, /* 1.1.2 */ + .domainSnapshotLookupByName = testDomainSnapshotLookupByName, /* 1.1.2 */ + .domainHasCurrentSnapshot = testDomainHasCurrentSnapshot, /* 1.1.2 */ + .domainSnapshotGetParent = testDomainSnapshotGetParent, /* 1.1.2 */ + .domainSnapshotCurrent = testDomainSnapshotCurrent, /* 1.1.2 */ + .domainSnapshotIsCurrent = testDomainSnapshotIsCurrent, /* 1.1.2 */ + .domainSnapshotHasMetadata = testDomainSnapshotHasMetadata, /* 1.1.2 */
Of course, since this is a new feature being posted after freeze, and I'm requesting that it go in post-release, that implies touching these up to say 1.1.3. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Again stolen from qemu_driver.c, but dropping all the unneeded bits. This aims to copy all the current qemu validation checks since that's the most commonly used real driver, but some of the checks are completely artificial in the test driver. This only supports creation of internal snapshots for initial simplicity. --- src/conf/domain_conf.c | 3 +- src/test/test_driver.c | 504 ++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 505 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f8fbf79..c67d14d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13408,7 +13408,8 @@ virDomainDefCheckABIStability(virDomainDefPtr src, virArchToString(src->os.arch)); return false; } - if (STRNEQ(src->os.machine, dst->os.machine)) { + if (src->os.machine && dst->os.machine && + STRNEQ(src->os.machine, dst->os.machine)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain OS type %s does not match source %s"), dst->os.machine, src->os.machine); diff --git a/src/test/test_driver.c b/src/test/test_driver.c index fd2ff1b..e0055b5 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2782,9 +2782,11 @@ static int testDomainUndefineFlags(virDomainPtr domain, testConnPtr privconn = domain->conn->privateData; virDomainObjPtr privdom; virDomainEventPtr event = NULL; + int nsnapshots; int ret = -1; - virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE, -1); + virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE | + VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1); testDriverLock(privconn); privdom = virDomainObjListFindByName(privconn->domains, @@ -2804,6 +2806,24 @@ static int testDomainUndefineFlags(virDomainPtr domain, } privdom->hasManagedSave = false; + /* Requiring an inactive VM is part of the documented API for + * UNDEFINE_SNAPSHOTS_METADATA + */ + if (!virDomainObjIsActive(privdom) && + (nsnapshots = virDomainSnapshotObjListNum(privdom->snapshots, + NULL, 0))) { + if (!(flags & VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("cannot delete inactive domain with %d " + "snapshots"), + nsnapshots); + goto cleanup; + } + + /* There isn't actually anything to do, we are just emulating qemu + * behavior here. */ + } + event = virDomainEventNewFromObj(privdom, VIR_DOMAIN_EVENT_UNDEFINED, VIR_DOMAIN_EVENT_UNDEFINED_REMOVED); @@ -6388,6 +6408,485 @@ cleanup: return ret; } +static int +testDomainSnapshotAlignDisks(virDomainObjPtr vm, + virDomainSnapshotDefPtr def, + unsigned int flags) +{ + int align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL; + int align_match = true; + + if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) { + align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; + align_match = false; + if (virDomainObjIsActive(vm)) + def->state = VIR_DOMAIN_DISK_SNAPSHOT; + else + def->state = VIR_DOMAIN_SHUTOFF; + def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE; + } else if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { + def->state = virDomainObjGetState(vm, NULL); + align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; + align_match = false; + } else { + def->state = virDomainObjGetState(vm, NULL); + def->memory = (def->state == VIR_DOMAIN_SHUTOFF ? + VIR_DOMAIN_SNAPSHOT_LOCATION_NONE : + VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL); + } + + return virDomainSnapshotAlignDisks(def, align_location, align_match); +} + +static virDomainSnapshotPtr +testDomainSnapshotCreateXML(virDomainPtr domain, + const char *xmlDesc, + unsigned int flags) +{ + testConnPtr privconn = domain->conn->privateData; + virDomainObjPtr vm = NULL; + virDomainSnapshotDefPtr def = NULL; + virDomainSnapshotObjPtr snap = NULL; + virDomainSnapshotPtr snapshot = NULL; + virDomainEventPtr event = NULL; + char *xml = NULL; + unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS; + + /* + * REDEFINE + CURRENT: Not implemented yet + * DISK_ONLY: Not implemented yet + * REUSE_EXT: Not implemented yet + * + * NO_METADATA: Explicitly not implemented + * + * HALT: Implemented + * QUIESCE: Nothing to do + * ATOMIC: Nothing to do + * LIVE: Nothing to do + */ + virCheckFlags( + VIR_DOMAIN_SNAPSHOT_CREATE_HALT | + VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE | + VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC | + VIR_DOMAIN_SNAPSHOT_CREATE_LIVE, NULL); + + if (!(vm = testDomObjFromDomain(domain))) + goto cleanup; + + testDriverLock(privconn); + + if (!vm->persistent && (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot halt after transient domain snapshot")); + goto cleanup; + } + + if (!(def = virDomainSnapshotDefParseString(xmlDesc, + privconn->caps, + privconn->xmlopt, + 1 << VIR_DOMAIN_VIRT_TEST, + parse_flags))) + goto cleanup; + + if (!(xml = virDomainDefFormat(vm->def, VIR_DOMAIN_XML_INACTIVE)) || + !(def->dom = virDomainDefParseString(xml, + privconn->caps, + privconn->xmlopt, + 1 << VIR_DOMAIN_VIRT_TEST, + VIR_DOMAIN_XML_INACTIVE))) + goto cleanup; + + if (testDomainSnapshotAlignDisks(vm, def, flags) < 0) + goto cleanup; + + if (!(snap = virDomainSnapshotAssignDef(vm->snapshots, def))) + goto cleanup; + def = NULL; + + if (vm->current_snapshot) { + if (!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) && + VIR_STRDUP(snap->def->parent, vm->current_snapshot->def->name) < 0) + goto cleanup; + } + + if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT) && + virDomainObjIsActive(vm)) { + testDomainShutdownState(domain, vm, VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT); + event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, + VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT); + } + + + snapshot = virGetDomainSnapshot(domain, snap->def->name); +cleanup: + VIR_FREE(xml); + if (vm) { + if (snapshot) { + virDomainSnapshotObjPtr other; + vm->current_snapshot = snap; + other = virDomainSnapshotFindByName(vm->snapshots, + snap->def->parent); + snap->parent = other; + other->nchildren++; + snap->sibling = other->first_child; + other->first_child = snap; + } + virObjectUnlock(vm); + } + if (event) { + testDomainEventQueue(privconn, event); + } + testDriverUnlock(privconn); + virDomainSnapshotDefFree(def); + return snapshot; +} + + +typedef struct _testSnapRemoveData testSnapRemoveData; +typedef testSnapRemoveData *testSnapRemoveDataPtr; +struct _testSnapRemoveData { + virDomainObjPtr vm; + bool current; +}; + +static void +testDomainSnapshotDiscard(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *data) +{ + virDomainSnapshotObjPtr snap = payload; + testSnapRemoveDataPtr curr = data; + + if (snap->def->current) + curr->current = true; + virDomainSnapshotObjListRemove(curr->vm->snapshots, snap); +} + +typedef struct _testSnapReparentData testSnapReparentData; +typedef testSnapReparentData *testSnapReparentDataPtr; +struct _testSnapReparentData { + virDomainSnapshotObjPtr parent; + virDomainObjPtr vm; + int err; + virDomainSnapshotObjPtr last; +}; + +static void +testDomainSnapshotReparentChildren(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *data) +{ + virDomainSnapshotObjPtr snap = payload; + testSnapReparentDataPtr rep = data; + + if (rep->err < 0) { + return; + } + + VIR_FREE(snap->def->parent); + snap->parent = rep->parent; + + if (rep->parent->def && + VIR_STRDUP(snap->def->parent, rep->parent->def->name) < 0) { + rep->err = -1; + return; + } + + if (!snap->sibling) + rep->last = snap; +} + +static int +testDomainSnapshotDelete(virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + virDomainObjPtr vm = NULL; + virDomainSnapshotObjPtr snap = NULL; + virDomainSnapshotObjPtr parentsnap = NULL; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN | + VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY, -1); + + if (!(vm = testDomObjFromSnapshot(snapshot))) + return -1; + + if (!(snap = testSnapObjFromSnapshot(vm, snapshot))) + goto cleanup; + + if (flags & (VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN | + VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY)) { + testSnapRemoveData rem; + rem.vm = vm; + rem.current = false; + virDomainSnapshotForEachDescendant(snap, + testDomainSnapshotDiscard, + &rem); + if (rem.current) { + if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) { + snap->def->current = true; + } + vm->current_snapshot = snap; + } + } else if (snap->nchildren) { + testSnapReparentData rep; + rep.parent = snap->parent; + rep.vm = vm; + rep.err = 0; + rep.last = NULL; + virDomainSnapshotForEachChild(snap, + testDomainSnapshotReparentChildren, + &rep); + if (rep.err < 0) + goto cleanup; + + /* Can't modify siblings during ForEachChild, so do it now. */ + snap->parent->nchildren += snap->nchildren; + rep.last->sibling = snap->parent->first_child; + snap->parent->first_child = snap->first_child; + } + + if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) { + snap->nchildren = 0; + snap->first_child = NULL; + } else { + virDomainSnapshotDropParent(snap); + if (snap == vm->current_snapshot) { + if (snap->def->parent) { + parentsnap = virDomainSnapshotFindByName(vm->snapshots, + snap->def->parent); + if (!parentsnap) { + VIR_WARN("missing parent snapshot matching name '%s'", + snap->def->parent); + } else { + parentsnap->def->current = true; + } + } + vm->current_snapshot = parentsnap; + } + virDomainSnapshotObjListRemove(vm->snapshots, snap); + } + + ret = 0; +cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + +static int +testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + testConnPtr privconn = snapshot->domain->conn->privateData; + virDomainObjPtr vm = NULL; + virDomainSnapshotObjPtr snap = NULL; + virDomainEventPtr event = NULL; + virDomainEventPtr event2 = NULL; + virDomainDefPtr config = NULL; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING | + VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED | + VIR_DOMAIN_SNAPSHOT_REVERT_FORCE, -1); + + /* We have the following transitions, which create the following events: + * 1. inactive -> inactive: none + * 2. inactive -> running: EVENT_STARTED + * 3. inactive -> paused: EVENT_STARTED, EVENT_PAUSED + * 4. running -> inactive: EVENT_STOPPED + * 5. running -> running: none + * 6. running -> paused: EVENT_PAUSED + * 7. paused -> inactive: EVENT_STOPPED + * 8. paused -> running: EVENT_RESUMED + * 9. paused -> paused: none + * Also, several transitions occur even if we fail partway through, + * and use of FORCE can cause multiple transitions. + */ + + if (!(vm = testDomObjFromSnapshot(snapshot))) + return -1; + + if (!(snap = testSnapObjFromSnapshot(vm, snapshot))) + goto cleanup; + + testDriverLock(privconn); + + if (!vm->persistent && + snap->def->state != VIR_DOMAIN_RUNNING && + snap->def->state != VIR_DOMAIN_PAUSED && + (flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING | + VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) == 0) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("transient domain needs to request run or pause " + "to revert to inactive snapshot")); + goto cleanup; + } + + if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) { + if (!snap->def->dom) { + virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, + _("snapshot '%s' lacks domain '%s' rollback info"), + snap->def->name, vm->def->name); + goto cleanup; + } + if (virDomainObjIsActive(vm) && + !(snap->def->state == VIR_DOMAIN_RUNNING + || snap->def->state == VIR_DOMAIN_PAUSED) && + (flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING | + VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) { + virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s", + _("must respawn guest to start inactive snapshot")); + goto cleanup; + } + } + + + if (vm->current_snapshot) { + vm->current_snapshot->def->current = false; + vm->current_snapshot = NULL; + } + + snap->def->current = true; + config = virDomainDefCopy(snap->def->dom, + privconn->caps, privconn->xmlopt, true); + if (!config) + goto cleanup; + + if (snap->def->state == VIR_DOMAIN_RUNNING || + snap->def->state == VIR_DOMAIN_PAUSED) { + /* Transitions 2, 3, 5, 6, 8, 9 */ + bool was_running = false; + bool was_stopped = false; + + if (virDomainObjIsActive(vm)) { + /* Transitions 5, 6, 8, 9 */ + /* Check for ABI compatibility. */ + if (!virDomainDefCheckABIStability(vm->def, config)) { + virErrorPtr err = virGetLastError(); + + if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) { + /* Re-spawn error using correct category. */ + if (err->code == VIR_ERR_CONFIG_UNSUPPORTED) + virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s", + err->str2); + goto cleanup; + } + + virResetError(err); + testDomainShutdownState(snapshot->domain, vm, + VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT); + event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_STOPPED, + VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT); + if (event) + testDomainEventQueue(privconn, event); + goto load; + } + + if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { + /* Transitions 5, 6 */ + was_running = true; + virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, + VIR_DOMAIN_PAUSED_FROM_SNAPSHOT); + /* Create an event now in case the restore fails, so + * that user will be alerted that they are now paused. + * If restore later succeeds, we might replace this. */ + event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_SUSPENDED, + VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT); + } + virDomainObjAssignDef(vm, config, false, NULL); + + } else { + /* Transitions 2, 3 */ + load: + was_stopped = true; + virDomainObjAssignDef(vm, config, false, NULL); + if (testDomainStartState(privconn, vm, + VIR_DOMAIN_RUNNING_FROM_SNAPSHOT) < 0) + goto cleanup; + event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_STARTED, + VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT); + } + + /* Touch up domain state. */ + if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING) && + (snap->def->state == VIR_DOMAIN_PAUSED || + (flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) { + /* Transitions 3, 6, 9 */ + virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, + VIR_DOMAIN_PAUSED_FROM_SNAPSHOT); + if (was_stopped) { + /* Transition 3, use event as-is and add event2 */ + event2 = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_SUSPENDED, + VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT); + } /* else transition 6 and 9 use event as-is */ + } else { + /* Transitions 2, 5, 8 */ + virDomainEventFree(event); + event = NULL; + + if (was_stopped) { + /* Transition 2 */ + event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_STARTED, + VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT); + } else if (was_running) { + /* Transition 8 */ + event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_RESUMED, + VIR_DOMAIN_EVENT_RESUMED); + } + } + } else { + /* Transitions 1, 4, 7 */ + virDomainObjAssignDef(vm, config, false, NULL); + + if (virDomainObjIsActive(vm)) { + /* Transitions 4, 7 */ + testDomainShutdownState(snapshot->domain, vm, + VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT); + event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_STOPPED, + VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT); + } + + if (flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING | + VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) { + /* Flush first event, now do transition 2 or 3 */ + bool paused = (flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED) != 0; + + if (event) + testDomainEventQueue(privconn, event); + event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_STARTED, + VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT); + if (paused) { + event2 = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_SUSPENDED, + VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT); + } + } + } + + vm->current_snapshot = snap; + ret = 0; +cleanup: + if (event) { + testDomainEventQueue(privconn, event); + if (event2) + testDomainEventQueue(privconn, event2); + } + virObjectUnlock(vm); + testDriverUnlock(privconn); + + return ret; +} + + static virDriver testDriver = { .no = VIR_DRV_TEST, @@ -6476,6 +6975,9 @@ static virDriver testDriver = { .domainSnapshotCurrent = testDomainSnapshotCurrent, /* 1.1.2 */ .domainSnapshotIsCurrent = testDomainSnapshotIsCurrent, /* 1.1.2 */ .domainSnapshotHasMetadata = testDomainSnapshotHasMetadata, /* 1.1.2 */ + .domainSnapshotCreateXML = testDomainSnapshotCreateXML, /* 1.1.2 */ + .domainRevertToSnapshot = testDomainRevertToSnapshot, /* 1.1.2 */ + .domainSnapshotDelete = testDomainSnapshotDelete, /* 1.1.2 */ }; static virNetworkDriver testNetworkDriver = { -- 1.8.3.1

On 08/30/2013 10:03 AM, Cole Robinson wrote:
Again stolen from qemu_driver.c, but dropping all the unneeded bits. This aims to copy all the current qemu validation checks since that's the most commonly used real driver, but some of the checks are completely artificial in the test driver.
This only supports creation of internal snapshots for initial simplicity. --- src/conf/domain_conf.c | 3 +- src/test/test_driver.c | 504 ++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 505 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f8fbf79..c67d14d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13408,7 +13408,8 @@ virDomainDefCheckABIStability(virDomainDefPtr src, virArchToString(src->os.arch)); return false; } - if (STRNEQ(src->os.machine, dst->os.machine)) { + if (src->os.machine && dst->os.machine && + STRNEQ(src->os.machine, dst->os.machine)) {
Wouldn't STRNEQ_NULLABLE() work better here (to make sure that if os.machine is omitted in one place, it is omitted in both)?
+ + /* + * REDEFINE + CURRENT: Not implemented yet + * DISK_ONLY: Not implemented yet + * REUSE_EXT: Not implemented yet + * + * NO_METADATA: Explicitly not implemented + * + * HALT: Implemented + * QUIESCE: Nothing to do + * ATOMIC: Nothing to do + * LIVE: Nothing to do + */ + virCheckFlags( + VIR_DOMAIN_SNAPSHOT_CREATE_HALT | + VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE | + VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC | + VIR_DOMAIN_SNAPSHOT_CREATE_LIVE, NULL);
Very handy breakdown explaining your choices. It sounds like your intent is that test:/// snapshots _always_ have metadata (since there is no side effects of modifying disk images, so metadata is the only thing that actually tracks a test snapshot). Looks like a decent skimmed down version of qemu, and should be very useful for testing purposes. ACK post-1.1.2 (with appropriate touchups to the list of when things are added). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Makes things more readable IMO --- src/qemu/qemu_driver.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ed29373..e7212c1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12283,6 +12283,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, char uuidstr[VIR_UUID_STRING_BUFLEN]; virDomainSnapshotDefPtr def = NULL; bool update_current = true; + bool redefine = flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE; unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS; virDomainSnapshotObjPtr other = NULL; int align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL; @@ -12307,11 +12308,10 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, return NULL; } - if (((flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) && - !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT)) || + if ((redefine && !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT)) || (flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) update_current = false; - if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) + if (redefine) parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE; virUUIDFormat(domain->uuid, uuidstr); @@ -12376,14 +12376,14 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_LIVE && (!virDomainObjIsActive(vm) || def->memory != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL || - flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE)) { + redefine)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("live snapshot creation is supported only " "with external checkpoints")); goto cleanup; } - if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) { + if (redefine) { /* Prevent circular chains */ if (def->parent) { if (STREQ(def->name, def->parent)) { @@ -12554,7 +12554,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, if (update_current) snap->def->current = true; if (vm->current_snapshot) { - if (!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) && + if (!redefine && VIR_STRDUP(snap->def->parent, vm->current_snapshot->def->name) < 0) goto cleanup; if (update_current) { @@ -12567,7 +12567,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, } /* actually do the snapshot */ - if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) { + if (redefine) { /* XXX Should we validate that the redefined snapshot even * makes sense, such as checking that qemu-img recognizes the * snapshot name in at least one of the domain's disks? */ -- 1.8.3.1

--- src/conf/snapshot_conf.c | 150 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/snapshot_conf.h | 7 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 131 +---------------------------------------- 4 files changed, 160 insertions(+), 129 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 45d6af4..f24aa22 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -1099,3 +1099,153 @@ virDomainSnapshotIsExternal(virDomainSnapshotObjPtr snap) { return virDomainSnapshotDefIsExternal(snap->def); } + +int +virDomainSnapshotRedefinePrep(virDomainPtr domain, + virDomainObjPtr vm, + virDomainSnapshotDefPtr *defptr, + virDomainSnapshotObjPtr *snap, + bool *update_current, + unsigned int flags) +{ + virDomainSnapshotDefPtr def = *defptr; + int ret = -1; + int align_location; + int align_match; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virDomainSnapshotObjPtr other; + + virUUIDFormat(domain->uuid, uuidstr); + + /* Prevent circular chains */ + if (def->parent) { + if (STREQ(def->name, def->parent)) { + virReportError(VIR_ERR_INVALID_ARG, + _("cannot set snapshot %s as its own parent"), + def->name); + goto cleanup; + } + other = virDomainSnapshotFindByName(vm->snapshots, def->parent); + if (!other) { + virReportError(VIR_ERR_INVALID_ARG, + _("parent %s for snapshot %s not found"), + def->parent, def->name); + goto cleanup; + } + while (other->def->parent) { + if (STREQ(other->def->parent, def->name)) { + virReportError(VIR_ERR_INVALID_ARG, + _("parent %s would create cycle to %s"), + other->def->name, def->name); + goto cleanup; + } + other = virDomainSnapshotFindByName(vm->snapshots, + other->def->parent); + if (!other) { + VIR_WARN("snapshots are inconsistent for %s", + vm->def->name); + break; + } + } + } + + /* Check that any replacement is compatible */ + if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) && + def->state != VIR_DOMAIN_DISK_SNAPSHOT) { + virReportError(VIR_ERR_INVALID_ARG, + _("disk-only flag for snapshot %s requires " + "disk-snapshot state"), + def->name); + goto cleanup; + + } + + if (def->dom && + memcmp(def->dom->uuid, domain->uuid, VIR_UUID_BUFLEN)) { + virReportError(VIR_ERR_INVALID_ARG, + _("definition for snapshot %s must use uuid %s"), + def->name, uuidstr); + goto cleanup; + } + + other = virDomainSnapshotFindByName(vm->snapshots, def->name); + if (other) { + if ((other->def->state == VIR_DOMAIN_RUNNING || + other->def->state == VIR_DOMAIN_PAUSED) != + (def->state == VIR_DOMAIN_RUNNING || + def->state == VIR_DOMAIN_PAUSED)) { + virReportError(VIR_ERR_INVALID_ARG, + _("cannot change between online and offline " + "snapshot state in snapshot %s"), + def->name); + goto cleanup; + } + + if ((other->def->state == VIR_DOMAIN_DISK_SNAPSHOT) != + (def->state == VIR_DOMAIN_DISK_SNAPSHOT)) { + virReportError(VIR_ERR_INVALID_ARG, + _("cannot change between disk snapshot and " + "system checkpoint in snapshot %s"), + def->name); + goto cleanup; + } + + if (other->def->dom) { + if (def->dom) { + if (!virDomainDefCheckABIStability(other->def->dom, + def->dom)) + goto cleanup; + } else { + /* Transfer the domain def */ + def->dom = other->def->dom; + other->def->dom = NULL; + } + } + + if (def->dom) { + if (def->state == VIR_DOMAIN_DISK_SNAPSHOT || + virDomainSnapshotDefIsExternal(def)) { + align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; + align_match = false; + } + + if (virDomainSnapshotAlignDisks(def, align_location, + align_match) < 0) { + /* revert stealing of the snapshot domain definition */ + if (def->dom && !other->def->dom) { + other->def->dom = def->dom; + def->dom = NULL; + } + goto cleanup; + } + } + + if (other == vm->current_snapshot) { + *update_current = true; + vm->current_snapshot = NULL; + } + + /* Drop and rebuild the parent relationship, but keep all + * child relations by reusing snap. */ + virDomainSnapshotDropParent(other); + virDomainSnapshotDefFree(other->def); + other->def = def; + *defptr = NULL; + *snap = other; + } else { + if (def->dom) { + if (def->state == VIR_DOMAIN_DISK_SNAPSHOT || + def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { + align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; + align_match = false; + } + if (virDomainSnapshotAlignDisks(def, align_location, + align_match) < 0) + goto cleanup; + } + } + + ret = 0; +cleanup: + return ret; +} diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index 0b8d18a..e8f8179 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -176,6 +176,13 @@ int virDomainListSnapshots(virDomainSnapshotObjListPtr snapshots, bool virDomainSnapshotDefIsExternal(virDomainSnapshotDefPtr def); bool virDomainSnapshotIsExternal(virDomainSnapshotObjPtr snap); +int virDomainSnapshotRedefinePrep(virDomainPtr domain, + virDomainObjPtr vm, + virDomainSnapshotDefPtr *def, + virDomainSnapshotObjPtr *snap, + bool *update_current, + unsigned int flags); + VIR_ENUM_DECL(virDomainSnapshotLocation) VIR_ENUM_DECL(virDomainSnapshotState) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 35f0f1b..76fedaf 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -650,6 +650,7 @@ virDomainSnapshotLocationTypeToString; virDomainSnapshotObjListGetNames; virDomainSnapshotObjListNum; virDomainSnapshotObjListRemove; +virDomainSnapshotRedefinePrep; virDomainSnapshotStateTypeFromString; virDomainSnapshotStateTypeToString; virDomainSnapshotUpdateRelations; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e7212c1..e9dcc09 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12280,7 +12280,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, char *xml = NULL; virDomainSnapshotObjPtr snap = NULL; virDomainSnapshotPtr snapshot = NULL; - char uuidstr[VIR_UUID_STRING_BUFLEN]; virDomainSnapshotDefPtr def = NULL; bool update_current = true; bool redefine = flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE; @@ -12314,8 +12313,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, if (redefine) parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE; - virUUIDFormat(domain->uuid, uuidstr); - if (!(vm = qemuDomObjFromDomain(domain))) goto cleanup; @@ -12384,133 +12381,9 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, } if (redefine) { - /* Prevent circular chains */ - if (def->parent) { - if (STREQ(def->name, def->parent)) { - virReportError(VIR_ERR_INVALID_ARG, - _("cannot set snapshot %s as its own parent"), - def->name); - goto cleanup; - } - other = virDomainSnapshotFindByName(vm->snapshots, def->parent); - if (!other) { - virReportError(VIR_ERR_INVALID_ARG, - _("parent %s for snapshot %s not found"), - def->parent, def->name); - goto cleanup; - } - while (other->def->parent) { - if (STREQ(other->def->parent, def->name)) { - virReportError(VIR_ERR_INVALID_ARG, - _("parent %s would create cycle to %s"), - other->def->name, def->name); - goto cleanup; - } - other = virDomainSnapshotFindByName(vm->snapshots, - other->def->parent); - if (!other) { - VIR_WARN("snapshots are inconsistent for %s", - vm->def->name); - break; - } - } - } - - /* Check that any replacement is compatible */ - if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) && - def->state != VIR_DOMAIN_DISK_SNAPSHOT) { - virReportError(VIR_ERR_INVALID_ARG, - _("disk-only flag for snapshot %s requires " - "disk-snapshot state"), - def->name); - goto cleanup; - - } - - if (def->dom && - memcmp(def->dom->uuid, domain->uuid, VIR_UUID_BUFLEN)) { - virReportError(VIR_ERR_INVALID_ARG, - _("definition for snapshot %s must use uuid %s"), - def->name, uuidstr); + if (!virDomainSnapshotRedefinePrep(domain, vm, &def, &snap, + &update_current, flags) < 0) goto cleanup; - } - - other = virDomainSnapshotFindByName(vm->snapshots, def->name); - if (other) { - if ((other->def->state == VIR_DOMAIN_RUNNING || - other->def->state == VIR_DOMAIN_PAUSED) != - (def->state == VIR_DOMAIN_RUNNING || - def->state == VIR_DOMAIN_PAUSED)) { - virReportError(VIR_ERR_INVALID_ARG, - _("cannot change between online and offline " - "snapshot state in snapshot %s"), - def->name); - goto cleanup; - } - - if ((other->def->state == VIR_DOMAIN_DISK_SNAPSHOT) != - (def->state == VIR_DOMAIN_DISK_SNAPSHOT)) { - virReportError(VIR_ERR_INVALID_ARG, - _("cannot change between disk snapshot and " - "system checkpoint in snapshot %s"), - def->name); - goto cleanup; - } - - if (other->def->dom) { - if (def->dom) { - if (!virDomainDefCheckABIStability(other->def->dom, - def->dom)) - goto cleanup; - } else { - /* Transfer the domain def */ - def->dom = other->def->dom; - other->def->dom = NULL; - } - } - - if (def->dom) { - if (def->state == VIR_DOMAIN_DISK_SNAPSHOT || - virDomainSnapshotDefIsExternal(def)) { - align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; - align_match = false; - } - - if (virDomainSnapshotAlignDisks(def, align_location, - align_match) < 0) { - /* revert stealing of the snapshot domain definition */ - if (def->dom && !other->def->dom) { - other->def->dom = def->dom; - def->dom = NULL; - } - goto cleanup; - } - } - - if (other == vm->current_snapshot) { - update_current = true; - vm->current_snapshot = NULL; - } - - /* Drop and rebuild the parent relationship, but keep all - * child relations by reusing snap. */ - virDomainSnapshotDropParent(other); - virDomainSnapshotDefFree(other->def); - other->def = def; - def = NULL; - snap = other; - } else { - if (def->dom) { - if (def->state == VIR_DOMAIN_DISK_SNAPSHOT || - def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { - align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; - align_match = false; - } - if (virDomainSnapshotAlignDisks(def, align_location, - align_match) < 0) - goto cleanup; - } - } } else { /* Easiest way to clone inactive portion of vm->def is via * conversion in and back out of xml. */ -- 1.8.3.1

--- src/test/test_driver.c | 67 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 43 insertions(+), 24 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index e0055b5..076f326 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -6450,26 +6450,35 @@ testDomainSnapshotCreateXML(virDomainPtr domain, virDomainSnapshotPtr snapshot = NULL; virDomainEventPtr event = NULL; char *xml = NULL; + bool update_current = true; + bool redefine = flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE; unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS; /* - * REDEFINE + CURRENT: Not implemented yet * DISK_ONLY: Not implemented yet * REUSE_EXT: Not implemented yet * * NO_METADATA: Explicitly not implemented * + * REDEFINE + CURRENT: Implemented * HALT: Implemented * QUIESCE: Nothing to do * ATOMIC: Nothing to do * LIVE: Nothing to do */ virCheckFlags( + VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE | + VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT | VIR_DOMAIN_SNAPSHOT_CREATE_HALT | VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE | VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC | VIR_DOMAIN_SNAPSHOT_CREATE_LIVE, NULL); + if ((redefine && !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT))) + update_current = false; + if (redefine) + parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE; + if (!(vm = testDomObjFromDomain(domain))) goto cleanup; @@ -6488,34 +6497,43 @@ testDomainSnapshotCreateXML(virDomainPtr domain, parse_flags))) goto cleanup; - if (!(xml = virDomainDefFormat(vm->def, VIR_DOMAIN_XML_INACTIVE)) || - !(def->dom = virDomainDefParseString(xml, - privconn->caps, - privconn->xmlopt, - 1 << VIR_DOMAIN_VIRT_TEST, - VIR_DOMAIN_XML_INACTIVE))) - goto cleanup; - - if (testDomainSnapshotAlignDisks(vm, def, flags) < 0) - goto cleanup; - - if (!(snap = virDomainSnapshotAssignDef(vm->snapshots, def))) - goto cleanup; - def = NULL; + if (redefine) { + if (!virDomainSnapshotRedefinePrep(domain, vm, &def, &snap, + &update_current, flags) < 0) + goto cleanup; + } else { + if (!(xml = virDomainDefFormat(vm->def, VIR_DOMAIN_XML_INACTIVE)) || + !(def->dom = virDomainDefParseString(xml, + privconn->caps, + privconn->xmlopt, + 1 << VIR_DOMAIN_VIRT_TEST, + VIR_DOMAIN_XML_INACTIVE))) + goto cleanup; - if (vm->current_snapshot) { - if (!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) && - VIR_STRDUP(snap->def->parent, vm->current_snapshot->def->name) < 0) + if (testDomainSnapshotAlignDisks(vm, def, flags) < 0) goto cleanup; } - if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT) && - virDomainObjIsActive(vm)) { - testDomainShutdownState(domain, vm, VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT); - event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, - VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT); + if (!snap) { + if (!(snap = virDomainSnapshotAssignDef(vm->snapshots, def))) + goto cleanup; + def = NULL; } + if (!redefine) { + if (vm->current_snapshot && + (VIR_STRDUP(snap->def->parent, + vm->current_snapshot->def->name) < 0)) + goto cleanup; + + if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT) && + virDomainObjIsActive(vm)) { + testDomainShutdownState(domain, vm, + VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT); + event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, + VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT); + } + } snapshot = virGetDomainSnapshot(domain, snap->def->name); cleanup: @@ -6523,7 +6541,8 @@ cleanup: if (vm) { if (snapshot) { virDomainSnapshotObjPtr other; - vm->current_snapshot = snap; + if (update_current) + vm->current_snapshot = snap; other = virDomainSnapshotFindByName(vm->snapshots, snap->def->parent); snap->parent = other; -- 1.8.3.1

The user can pass it as a <test:domainsnapshot> subelement of a <domain>. --- src/test/test_driver.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 98 insertions(+), 1 deletion(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 076f326..f0ac523 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -161,12 +161,24 @@ struct _testDomainNamespaceDef { int runstate; bool transient; bool hasManagedSave; + + unsigned int num_snap_nodes; + xmlNodePtr *snap_nodes; }; static void testDomainDefNamespaceFree(void *data) { testDomainNamespaceDefPtr nsdata = data; + size_t i; + + if (!nsdata) + return; + + for (i = 0; i < nsdata->num_snap_nodes; i++) + xmlFreeNode(nsdata->snap_nodes[i]); + + VIR_FREE(nsdata->snap_nodes); VIR_FREE(nsdata); } @@ -177,7 +189,9 @@ testDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, void **data) { testDomainNamespaceDefPtr nsdata = NULL; - int tmp; + xmlNodePtr *nodes = NULL; + int tmp, n; + size_t i; unsigned int tmpuint; if (xmlXPathRegisterNs(ctxt, BAD_CAST "test", @@ -191,6 +205,26 @@ testDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, if (VIR_ALLOC(nsdata) < 0) return -1; + n = virXPathNodeSet("./test:domainsnapshot", ctxt, &nodes); + if (n < 0) + goto error; + + if (n && VIR_ALLOC_N(nsdata->snap_nodes, n) < 0) + goto error; + + nsdata->num_snap_nodes = 0; + for (i = 0; i < n; i++) { + xmlNodePtr newnode = xmlCopyNode(nodes[i], 1); + if (!newnode) { + virReportOOMError(); + goto error; + } + + nsdata->snap_nodes[nsdata->num_snap_nodes] = newnode; + nsdata->num_snap_nodes++; + } + VIR_FREE(nodes); + tmp = virXPathBoolean("boolean(./test:transient)", ctxt); if (tmp == -1) { virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid transient")); @@ -225,6 +259,7 @@ testDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, return 0; error: + VIR_FREE(nodes); testDomainDefNamespaceFree(nsdata); return -1; } @@ -922,6 +957,63 @@ error: } static int +testParseDomainSnapshots(testConnPtr privconn, + virDomainObjPtr domobj, + const char *file, + xmlXPathContextPtr ctxt) +{ + size_t i; + int ret = -1; + testDomainNamespaceDefPtr nsdata = domobj->def->namespaceData; + xmlNodePtr *nodes = nsdata->snap_nodes; + + for (i = 0; i < nsdata->num_snap_nodes; i++) { + virDomainSnapshotObjPtr snap; + virDomainSnapshotDefPtr def; + xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file, + "domainsnapshot"); + if (!node) + goto error; + + def = virDomainSnapshotDefParseNode(ctxt->doc, node, + privconn->caps, + privconn->xmlopt, + 1 << VIR_DOMAIN_VIRT_TEST, + VIR_DOMAIN_SNAPSHOT_PARSE_DISKS | + VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL | + VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE); + if (!def) + goto error; + + if (!(snap = virDomainSnapshotAssignDef(domobj->snapshots, def))) { + virDomainSnapshotDefFree(def); + goto error; + } + + if (def->current) { + if (domobj->current_snapshot) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("more than one snapshot claims to be active")); + goto error; + } + + domobj->current_snapshot = snap; + } + } + + if (virDomainSnapshotUpdateRelations(domobj->snapshots) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Snapshots have inconsistent relations for " + "domain %s"), domobj->def->name); + goto error; + } + + ret = 0; +error: + return ret; +} + +static int testParseDomains(testConnPtr privconn, const char *file, xmlXPathContextPtr ctxt) @@ -959,6 +1051,11 @@ testParseDomains(testConnPtr privconn, goto error; } + if (testParseDomainSnapshots(privconn, obj, file, ctxt) < 0) { + virObjectUnlock(obj); + goto error; + } + nsdata = def->namespaceData; obj->persistent = !nsdata->transient; obj->hasManagedSave = nsdata->hasManagedSave; -- 1.8.3.1
participants (2)
-
Cole Robinson
-
Eric Blake