10:22 a.m.
Just like VM saved state images (virsh save), snapshots MUST
track the inactive domain xml to detect any ABI incompatibilities.
The indentation is not perfect, but functionality comes before form.
Later patches will actually supply a full domain; for now, this
wires up the storage to support one, but doesn't ever generate one
in dumpxml output.
Happily, libvirt.c was already rejecting use of VIR_DOMAIN_XML_SECURE
from read-only connections, even though before this patch, there was
no one ever using that flag and there was no information to be
secured by the use of that flag.
* src/libvirt.c (virDomainSnapshotGetXMLDesc): Document flag.
* src/conf/domain_conf.h (_virDomainSnapshotDef): Add member.
(virDomainSnapshotDefParseString, virDomainSnapshotDefFormat):
Update signature.
* src/conf/domain_conf.c (virDomainSnapshotDefFree): Clean up.
(virDomainSnapshotDefParseString): Optionally parse domain.
(virDomainSnapshotDefFormat): Output full domain.
* src/esx/esx_driver.c (esxDomainSnapshotCreateXML)
(esxDomainSnapshotGetXMLDesc): Update callers.
* src/vbox/vbox_tmpl.c (vboxDomainSnapshotCreateXML)
(vboxDomainSnapshotGetXMLDesc): Likewise.
* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML)
(qemuDomainSnapshotLoad, qemuDomainSnapshotGetXMLDesc)
(qemuDomainSnapshotWriteMetadata): Likewise.
* docs/formatsnapshot.html.in: Rework doc example.
Based on a patch by Philipp Hahn.
---
docs/formatsnapshot.html.in | 47 ++++++++++++++++++++++++++++------------
src/conf/domain_conf.c | 50 +++++++++++++++++++++++++++++++++++++------
src/conf/domain_conf.h | 7 +++++-
src/esx/esx_driver.c | 4 +-
src/libvirt.c | 7 +++++-
src/qemu/qemu_driver.c | 20 +++++++++++++----
src/vbox/vbox_tmpl.c | 4 +-
7 files changed, 107 insertions(+), 32 deletions(-)
diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in
index 79ed1d2..edf20e8 100644
--- a/docs/formatsnapshot.html.in
+++ b/docs/formatsnapshot.html.in
@@ -46,27 +46,46 @@
snapshots. Readonly.
</dd>
<dt><code>domain</code></dt>
- <dd>The domain that this snapshot was taken against. This
- element contains exactly one child element, uuid. This
- specifies the uuid of the domain that this snapshot was taken
- against. Readonly.
+ <dd>The domain that this snapshot was taken against. Older
+ versions of libvirt stored only a single child element, uuid;
+ reverting to a snapshot like this is risky if the current
+ state of the domain differs from the state that the domain was
+ created in, and requires the use of the
+ <code>VIR_DOMAIN_SNAPSHOT_REVERT_FORCE</code> flag
+ in <code>virDomainRevertToSnapshot()</code>. Newer versions
+ of libvirt store the entire
+ inactive <a href="formatdomain.html">domain
configuration</a>
+ at the time of the snapshot. Readonly.
</dd>
</dl>
- <h2><a name="example">Example</a></h2>
+ <h2><a name="example">Examples</a></h2>
+ <p>Using this XML on creation:</p>
<pre>
<domainsnapshot>
- <name>os-updates</name>
<description>Snapshot of OS install and
updates</description>
- <state>running</state>
- <creationTime>1270477159</creationTime>
- <parent>
- <name>bare-os-install</name>
- </parent>
- <domain>
-
<uuid>93a5c045-6457-2c09-e56c-927cdf34e178</uuid>
- </domain>
</domainsnapshot></pre>
+
+ <p>Will result in XML similar to this from
+ virDomainSnapshotGetXMLDesc:</p>
+ <pre>
+<domainsnapshot>
+ <name>1270477159</name>
+ <description>Snapshot of OS install and
updates</description>
+ <state>running</state>
+ <creationTime>1270477159</creationTime>
+ <parent>
+ <name>bare-os-install</name>
+ </parent>
+ <domain>
+ <name>fedora</name>
+ <uuid>93a5c045-6457-2c09-e56c-927cdf34e178</uuid>
+ <memory>1048576</memory>
+ ...
+ </devices>
+ </domain>
+</domainsnapshot></pre>
+
</body>
</html>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 27c77fe..d2a9849 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10953,11 +10953,17 @@ void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def)
VIR_FREE(def->name);
VIR_FREE(def->description);
VIR_FREE(def->parent);
+ virDomainDefFree(def->dom);
VIR_FREE(def);
}
-virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
- int newSnapshot)
+/* If newSnapshot is true, caps, expectedVirtTypes, and flags are ignored. */
+virDomainSnapshotDefPtr
+virDomainSnapshotDefParseString(const char *xmlStr,
+ int newSnapshot,
+ virCapsPtr caps,
+ unsigned int expectedVirtTypes,
+ unsigned int flags)
{
xmlXPathContextPtr ctxt = NULL;
xmlDocPtr xml = NULL;
@@ -10966,6 +10972,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char
*xmlStr,
char *creation = NULL, *state = NULL;
struct timeval tv;
int active;
+ char *tmp;
xml = virXMLParseCtxt(NULL, xmlStr, "domainsnapshot.xml", &ctxt);
if (!xml) {
@@ -11028,9 +11035,29 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const
char *xmlStr,
goto cleanup;
}
def->current = active != 0;
- }
- else
+
+ /* Older snapshots were created with just <domain>/<uuid>, and
+ * lack domain/@type. In that case, leave dom NULL, and
+ * clients will have to decide between best effort
+ * initialization or outright failure. */
+ if ((tmp = virXPathString("string(./domain/@type)", ctxt))) {
+ VIR_FREE(tmp);
+ xmlNodePtr domainNode = virXPathNode("./domain", ctxt);
+ if (!domainNode) {
+ virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing domain in snapshot"));
+ goto cleanup;
+ }
+ def->dom = virDomainDefParseNode(caps, xml, domainNode,
+ expectedVirtTypes, flags);
+ if (!def->dom)
+ goto cleanup;
+ } else {
+ VIR_WARN("parsing older snapshot that lacks domain");
+ }
+ } else {
def->creationTime = tv.tv_sec;
+ }
ret = def;
@@ -11047,10 +11074,15 @@ cleanup:
char *virDomainSnapshotDefFormat(char *domain_uuid,
virDomainSnapshotDefPtr def,
+ unsigned int flags,
int internal)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
+ virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL);
+
+ flags |= VIR_DOMAIN_XML_INACTIVE;
+
virBufferAddLit(&buf, "<domainsnapshot>\n");
virBufferAsprintf(&buf, " <name>%s</name>\n",
def->name);
if (def->description)
@@ -11065,9 +11097,13 @@ char *virDomainSnapshotDefFormat(char *domain_uuid,
}
virBufferAsprintf(&buf, "
<creationTime>%lld</creationTime>\n",
def->creationTime);
- virBufferAddLit(&buf, " <domain>\n");
- virBufferAsprintf(&buf, " <uuid>%s</uuid>\n",
domain_uuid);
- virBufferAddLit(&buf, " </domain>\n");
+ if (def->dom) {
+ virDomainDefFormatInternal(def->dom, flags, &buf);
+ } else {
+ virBufferAddLit(&buf, " <domain>\n");
+ virBufferAsprintf(&buf, " <uuid>%s</uuid>\n",
domain_uuid);
+ virBufferAddLit(&buf, " </domain>\n");
+ }
if (internal)
virBufferAsprintf(&buf, " <active>%d</active>\n",
def->current);
virBufferAddLit(&buf, "</domainsnapshot>\n");
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 503fb58..93bb7c8 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1302,6 +1302,7 @@ struct _virDomainSnapshotDef {
char *parent;
long long creationTime; /* in seconds */
int state;
+ virDomainDefPtr dom;
/* Internal use. */
bool current; /* At most one snapshot in the list should have this set */
@@ -1325,10 +1326,14 @@ struct _virDomainSnapshotObjList {
};
virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
- int newSnapshot);
+ int newSnapshot,
+ virCapsPtr caps,
+ unsigned int expectedVirtTypes,
+ unsigned int flags);
void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def);
char *virDomainSnapshotDefFormat(char *domain_uuid,
virDomainSnapshotDefPtr def,
+ unsigned int flags,
int internal);
virDomainSnapshotObjPtr virDomainSnapshotAssignDef(virDomainSnapshotObjListPtr
snapshots,
const virDomainSnapshotDefPtr def);
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 90f55c3..8fbbe8a 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -4223,7 +4223,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char
*xmlDesc,
return NULL;
}
- def = virDomainSnapshotDefParseString(xmlDesc, 1);
+ def = virDomainSnapshotDefParseString(xmlDesc, 1, NULL, 0, 0);
if (def == NULL) {
return NULL;
@@ -4319,7 +4319,7 @@ esxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot,
virUUIDFormat(snapshot->domain->uuid, uuid_string);
- xml = virDomainSnapshotDefFormat(uuid_string, &def, 0);
+ xml = virDomainSnapshotDefFormat(uuid_string, &def, flags, 0);
cleanup:
esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotList);
diff --git a/src/libvirt.c b/src/libvirt.c
index b0160fb..fac2047 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -15517,10 +15517,15 @@ error:
/**
* virDomainSnapshotGetXMLDesc:
* @snapshot: a domain snapshot object
- * @flags: unused flag parameters; callers should pass 0
+ * @flags: bitwise-OR of subset of virDomainXMLFlags
*
* Provide an XML description of the domain snapshot.
*
+ * No security-sensitive data will be included unless @flags contains
+ * VIR_DOMAIN_XML_SECURE; this flag is rejected on read-only
+ * connections. For this API, @flags should not contain either
+ * VIR_DOMAIN_XML_INACTIVE or VIR_DOMAIN_XML_UPDATE_CPU.
+ *
* Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error.
* the caller must free() the returned value.
*/
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8d769c9..7cf945a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -337,7 +337,10 @@ static void qemuDomainSnapshotLoad(void *payload,
continue;
}
- def = virDomainSnapshotDefParseString(xmlStr, 0);
+ def = virDomainSnapshotDefParseString(xmlStr, 0, qemu_driver->caps,
+ QEMU_EXPECTED_VIRT_TYPES,
+ (VIR_DOMAIN_XML_INACTIVE |
+ VIR_DOMAIN_XML_SECURE));
if (def == NULL) {
/* Nothing we can do here, skip this one */
VIR_ERROR(_("Failed to parse snapshot XML from file
'%s'"),
@@ -1610,7 +1613,8 @@ qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm,
char uuidstr[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(vm->def->uuid, uuidstr);
- newxml = virDomainSnapshotDefFormat(uuidstr, snapshot->def, 1);
+ newxml = virDomainSnapshotDefFormat(uuidstr, snapshot->def,
+ VIR_DOMAIN_XML_SECURE, 1);
if (newxml == NULL) {
virReportOOMError();
return -1;
@@ -1636,6 +1640,12 @@ qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm,
_("failed to create snapshot file '%s'"),
snapFile);
goto cleanup;
}
+ /* XXX need virsh snapshot-edit, before this makes sense:
+ * char *tmp;
+ * virAsprintf(&tmp, "snapshot-edit %s", vm->def->name);
+ * virEmitXMLWarning(fd, snapshot->def->name, tmp);
+ * VIR_FREE(tmp);
+ */
if (safewrite(fd, newxml, strlen(newxml)) != strlen(newxml)) {
virReportSystemError(errno, _("Failed to write snapshot data to %s"),
snapFile);
@@ -8681,7 +8691,7 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr
domain,
if (!qemuDomainSnapshotIsAllowed(vm))
goto cleanup;
- if (!(def = virDomainSnapshotDefParseString(xmlDesc, 1)))
+ if (!(def = virDomainSnapshotDefParseString(xmlDesc, 1, NULL, 0, 0)))
goto cleanup;
if (!(snap = virDomainSnapshotAssignDef(&vm->snapshots, def)))
@@ -8908,7 +8918,7 @@ static char *qemuDomainSnapshotGetXMLDesc(virDomainSnapshotPtr
snapshot,
virDomainSnapshotObjPtr snap = NULL;
char uuidstr[VIR_UUID_STRING_BUFLEN];
- virCheckFlags(0, NULL);
+ virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL);
qemuDriverLock(driver);
virUUIDFormat(snapshot->domain->uuid, uuidstr);
@@ -8927,7 +8937,7 @@ static char *qemuDomainSnapshotGetXMLDesc(virDomainSnapshotPtr
snapshot,
goto cleanup;
}
- xml = virDomainSnapshotDefFormat(uuidstr, snap->def, 0);
+ xml = virDomainSnapshotDefFormat(uuidstr, snap->def, flags, 0);
cleanup:
if (vm)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 9e1c6e3..0dd5efa 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -5661,7 +5661,7 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom,
virCheckFlags(0, NULL);
- if (!(def = virDomainSnapshotDefParseString(xmlDesc, 1)))
+ if (!(def = virDomainSnapshotDefParseString(xmlDesc, 1, NULL, 0, 0)))
goto cleanup;
vboxIIDFromUUID(&domiid, dom->uuid);
@@ -5843,7 +5843,7 @@ vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot,
def->state = VIR_DOMAIN_SHUTOFF;
virUUIDFormat(dom->uuid, uuidstr);
- ret = virDomainSnapshotDefFormat(uuidstr, def, 0);
+ ret = virDomainSnapshotDefFormat(uuidstr, def, flags, 0);
cleanup:
virDomainSnapshotDefFree(def);
--
1.7.4.4