[libvirt] [PATCH v2] snapshot: Store both config and live XML in the snapshot domain
Daniel Henrique Barboza
danielhb413 at gmail.com
Wed Jul 31 19:45:07 UTC 2019
Max,
Code looks ok. Two tests (virsh-checkpoint and virsh-snapshot) are
failing, but they are also failing on master, thus I say this patch get a
pass because it didn't break anything else.
Reviewed-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
On 7/23/19 4:47 PM, Maxiwell S. Garcia wrote:
> The snapshot-create operation of running guests saves the live
> XML and uses it to replace the active and inactive domain in
> case of revert. So, the config XML is ignored by the snapshot
> process. This commit changes it and adds the config XML in the
> snapshot XML as the <inactiveDomain> entry.
>
> In case of offline guest, the behavior remains the same and the
> config XML is saved in the snapshot XML as <domain> entry. The
> behavior of older snapshots of running guests, that don't have
> the new <inactiveDomain>, remains the same too. The revert, in
> this case, overrides both active and inactive domain with the
> <domain> entry. So, the <inactiveDomain> in the snapshot XML is
> not required to snapshot work, but it's useful to preserve the
> config XML of running guests.
>
> Signed-off-by: Maxiwell S. Garcia <maxiwell at linux.ibm.com>
> ---
> src/conf/moment_conf.c | 1 +
> src/conf/moment_conf.h | 11 +++++++++
> src/conf/snapshot_conf.c | 48 +++++++++++++++++++++++++++++++++++-----
> src/qemu/qemu_driver.c | 27 +++++++++++++++++-----
> src/util/virxml.c | 45 +++++++++++++++++++++++++++++++++++++
> src/util/virxml.h | 8 +++++++
> 6 files changed, 130 insertions(+), 10 deletions(-)
>
> diff --git a/src/conf/moment_conf.c b/src/conf/moment_conf.c
> index fea13f0f97..f54a44b33e 100644
> --- a/src/conf/moment_conf.c
> +++ b/src/conf/moment_conf.c
> @@ -66,6 +66,7 @@ virDomainMomentDefDispose(void *obj)
> VIR_FREE(def->description);
> VIR_FREE(def->parent_name);
> virDomainDefFree(def->dom);
> + virDomainDefFree(def->inactiveDom);
> }
>
> /* Provide defaults for creation time and moment name after parsing XML */
> diff --git a/src/conf/moment_conf.h b/src/conf/moment_conf.h
> index 9fdbef2172..70cc47bd70 100644
> --- a/src/conf/moment_conf.h
> +++ b/src/conf/moment_conf.h
> @@ -36,7 +36,18 @@ struct _virDomainMomentDef {
> char *parent_name;
> long long creationTime; /* in seconds */
>
> + /*
> + * Store the active domain definition in case of online
> + * guest and the inactive domain definition in case of
> + * offline guest
> + */
> virDomainDefPtr dom;
> +
> + /*
> + * Store the inactive domain definition in case of online
> + * guest and leave NULL in case of offline guest
> + */
> + virDomainDefPtr inactiveDom;
> };
>
> virClassPtr virClassForDomainMomentDef(void);
> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
> index 324901a560..8aeac9ab20 100644
> --- a/src/conf/snapshot_conf.c
> +++ b/src/conf/snapshot_conf.c
> @@ -243,6 +243,8 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
> char *memoryFile = NULL;
> bool offline = !!(flags & VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE);
> virSaveCookieCallbacksPtr saveCookie = virDomainXMLOptionGetSaveCookie(xmlopt);
> + int domainflags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
> + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
>
> if (!(def = virDomainSnapshotDefNew()))
> return NULL;
> @@ -292,8 +294,6 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
> * clients will have to decide between best effort
> * initialization or outright failure. */
> if ((tmp = virXPathString("string(./domain/@type)", ctxt))) {
> - int domainflags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
> - VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
> xmlNodePtr domainNode = virXPathNode("./domain", ctxt);
>
> VIR_FREE(tmp);
> @@ -309,6 +309,20 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
> } else {
> VIR_WARN("parsing older snapshot that lacks domain");
> }
> +
> + /* /inactiveDomain entry saves the config XML present in a running
> + * VM. In case of absent, leave parent.inactiveDom NULL and use
> + * parent.dom for config and live XML. */
> + if (virXPathString("string(./inactiveDomain/@type)", ctxt)) {
> + xmlNodePtr domainNode = virXPathNode("./inactiveDomain", ctxt);
> +
> + if (domainNode) {
> + def->parent.inactiveDom = virDomainDefParseNode(ctxt->node->doc, domainNode,
> + caps, xmlopt, NULL, domainflags);
> + if (!def->parent.inactiveDom)
> + goto cleanup;
> + }
> + }
> } else if (virDomainXMLOptionRunMomentPostParse(xmlopt, &def->parent) < 0) {
> goto cleanup;
> }
> @@ -845,6 +859,10 @@ virDomainSnapshotDefFormatInternal(virBufferPtr buf,
> {
> size_t i;
> int domainflags = VIR_DOMAIN_DEF_FORMAT_INACTIVE;
> + virBuffer inactivedom_buf = VIR_BUFFER_INITIALIZER;
> + xmlXPathContextPtr inactivedom_ctxt = NULL;
> + char *inactivedom_str = NULL;
> + int ret = -1;
>
> if (flags & VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE)
> domainflags |= VIR_DOMAIN_DEF_FORMAT_SECURE;
> @@ -903,6 +921,20 @@ virDomainSnapshotDefFormatInternal(virBufferPtr buf,
> virBufferAddLit(buf, "</domain>\n");
> }
>
> + if (def->parent.inactiveDom) {
> + if (virDomainDefFormatInternal(def->parent.inactiveDom, caps,
> + domainflags, &inactivedom_buf, xmlopt) < 0)
> + goto error;
> +
> + inactivedom_ctxt = virXPathBuildContext(&inactivedom_buf);
> + if (!(inactivedom_str = virXPathRenameNode("/domain", "inactiveDomain",
> + inactivedom_ctxt)))
> + goto error;
> +
> + virBufferAddStr(buf, inactivedom_str);
> + virBufferAddLit(buf, "\n");
> + }
> +
> if (virSaveCookieFormatBuf(buf, def->cookie,
> virDomainXMLOptionGetSaveCookie(xmlopt)) < 0)
> goto error;
> @@ -917,11 +949,17 @@ virDomainSnapshotDefFormatInternal(virBufferPtr buf,
> if (virBufferCheckError(buf) < 0)
> goto error;
>
> - return 0;
> + ret = 0;
>
> error:
> - virBufferFreeAndReset(buf);
> - return -1;
> + VIR_FREE(inactivedom_str);
> + xmlXPathFreeContext(inactivedom_ctxt);
> + virBufferFreeAndReset(&inactivedom_buf);
> +
> + if (ret < 0)
> + virBufferFreeAndReset(buf);
> +
> + return ret;
> }
>
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 482f915b67..9b95e9b766 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -15697,6 +15697,13 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
> VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
> goto endjob;
>
> + if (vm->newDef) {
> + def->parent.inactiveDom = virDomainDefCopy(vm->newDef, caps,
> + driver->xmlopt, NULL, true);
> + if (!def->parent.inactiveDom)
> + goto endjob;
> + }
> +
> if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) {
> align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
> align_match = false;
> @@ -16231,6 +16238,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
> qemuDomainObjPrivatePtr priv;
> int rc;
> virDomainDefPtr config = NULL;
> + virDomainDefPtr inactiveConfig = NULL;
> virQEMUDriverConfigPtr cfg = NULL;
> virCapsPtr caps = NULL;
> bool was_stopped = false;
> @@ -16331,17 +16339,22 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
> * in the failure cases where we know there was no change? */
> }
>
> - /* Prepare to copy the snapshot inactive xml as the config of this
> - * domain.
> - *
> - * XXX Should domain snapshots track live xml rather
> - * than inactive xml? */
> + /* Prepare to copy the snapshot inactive domain as the config XML
> + * and the snapshot domain as the live XML. In case of inactive domain
> + * NULL, both config and live XML will be copied from snapshot domain.
> + */
> if (snap->def->dom) {
> config = virDomainDefCopy(snap->def->dom, caps,
> driver->xmlopt, NULL, true);
> if (!config)
> goto endjob;
> }
> + if (snap->def->inactiveDom) {
> + inactiveConfig = virDomainDefCopy(snap->def->inactiveDom, caps,
> + driver->xmlopt, NULL, true);
> + if (!inactiveConfig)
> + goto endjob;
> + }
>
> cookie = (qemuDomainSaveCookiePtr) snapdef->cookie;
>
> @@ -16592,6 +16605,10 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
> goto endjob;
> }
>
> + if (inactiveConfig) {
> + virDomainDefFree(vm->newDef);
> + VIR_STEAL_PTR(vm->newDef, inactiveConfig);
> + }
> ret = 0;
>
> endjob:
> diff --git a/src/util/virxml.c b/src/util/virxml.c
> index f55b9a362c..756c0eedbc 100644
> --- a/src/util/virxml.c
> +++ b/src/util/virxml.c
> @@ -1408,3 +1408,48 @@ virXPathContextNodeRestore(virXPathContextNodeSavePtr save)
>
> save->ctxt->node = save->node;
> }
> +
> +
> +/**
> + * virXPathBuildContext: convert an parent buffer to an
> + * XPath context ptr. The caller has to free the ptr.
> + */
> +xmlXPathContextPtr
> +virXPathBuildContext(virBufferPtr root)
> +{
> + xmlDocPtr doc;
> +
> + if (!root)
> + return NULL;
> +
> + doc = virXMLParseString(virBufferCurrentContent(root), NULL);
> + if (!doc)
> + return NULL;
> +
> + return xmlXPathNewContext(doc);
> +}
> +
> +
> +/**
> + * virXPathRenameNode: get the XML node using the 'xpath' and
> + * rename it with the 'newname' string.
> + *
> + * Returns the XML string of the node found by 'xpath' or NULL
> + * on error. The caller has to free the string.
> + */
> +char *
> +virXPathRenameNode(const char *xpath,
> + const char *newname,
> + xmlXPathContextPtr ctxt)
> +{
> + xmlNodePtr node;
> +
> + if (!xpath || !newname || !ctxt)
> + return NULL;
> +
> + if (!(node = virXPathNode(xpath, ctxt)))
> + return NULL;
> +
> + xmlNodeSetName(node, (xmlChar *) newname);
> + return virXMLNodeToString(node->doc, node);
> +}
> diff --git a/src/util/virxml.h b/src/util/virxml.h
> index 6208977dd1..48a507c3c1 100644
> --- a/src/util/virxml.h
> +++ b/src/util/virxml.h
> @@ -220,6 +220,14 @@ virXMLFormatElement(virBufferPtr buf,
> virBufferPtr childBuf)
> ATTRIBUTE_RETURN_CHECK;
>
> +xmlXPathContextPtr
> +virXPathBuildContext(virBufferPtr root);
> +
> +char *
> +virXPathRenameNode(const char *xpath,
> + const char *newname,
> + xmlXPathContextPtr ctxt);
> +
> struct _virXPathContextNodeSave {
> xmlXPathContextPtr ctxt;
> xmlNodePtr node;
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190731/0dd92ada/attachment-0001.htm>
More information about the libvir-list
mailing list