From f609cb85ca11d6d178972e46ebeb9985bf4ccfaf Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 12 Aug 2011 19:19:47 -0600 Subject: [PATCH] snapshot: allow full domain xml in snapshot 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 information to be secured by the use of that flag. And while we're at it, mark the libvirt snapshot metadata files as internal-use only. * 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 | 31 ++++++++++++++++++----- src/conf/domain_conf.c | 50 +++++++++++++++++++++++++++++++++---- src/conf/domain_conf.h | 4 +++ src/esx/esx_driver.c | 4 +-- src/libvirt.c | 7 +++++- src/qemu/qemu_driver.c | 24 +++++++++++++----- src/vbox/vbox_tmpl.c | 4 +-- 7 files changed, 102 insertions(+), 22 deletions(-) diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in index dc5873a822..91799b42c7 100644 --- a/docs/formatsnapshot.html.in +++ b/docs/formatsnapshot.html.in @@ -63,18 +63,33 @@ snapshots, as described above. Readonly.
domain
-
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. +
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 + VIR_DOMAIN_SNAPSHOT_REVERT_FORCE flag + in virDomainRevertToSnapshot(). Newer versions + of libvirt store the entire + inactive domain configuration + at the time of the snapshot (since + 0.9.5). Readonly.
-

Example

+

Examples

+

Using this XML on creation:

 <domainsnapshot>
-  <name>os-updates</name>
+  <description>Snapshot of OS install and updates</description>
+</domainsnapshot>
+ +

will result in XML similar to this from + virDomainSnapshotGetXMLDesc:

+ 
+<domainsnapshot>
+  <name>1270477159</name>
   <description>Snapshot of OS install and updates</description>
   <state>running</state>
   <creationTime>1270477159</creationTime>
@@ -82,7 +97,11 @@
     <name>bare-os-install</name>
   </parent>
   <domain>
+    <name>fedora</name>
     <uuid>93a5c045-6457-2c09-e56c-927cdf34e178</uuid>
+    <memory>1048576</memory>
+    ...
+    </devices>
   </domain>
 </domainsnapshot>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5a13e43dd1..1b57a55552 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11377,12 +11377,19 @@ void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def) VIR_FREE(def->name); VIR_FREE(def->description); VIR_FREE(def->parent); + virDomainDefFree(def->dom); VIR_FREE(def); } -/* flags are from virDomainSnapshotParseFlags */ +/* flags is bitwise-or of virDomainSnapshotParseFlags. + * If flags does not include VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE, then + * caps and expectedVirtTypes are ignored. + */ virDomainSnapshotDefPtr -virDomainSnapshotDefParseString(const char *xmlStr, unsigned int flags) +virDomainSnapshotDefParseString(const char *xmlStr, + virCapsPtr caps, + unsigned int expectedVirtTypes, + unsigned int flags) { xmlXPathContextPtr ctxt = NULL; xmlDocPtr xml = NULL; @@ -11391,6 +11398,7 @@ virDomainSnapshotDefParseString(const char *xmlStr, unsigned int flags) char *creation = NULL, *state = NULL; struct timeval tv; int active; + char *tmp; xml = virXMLParseCtxt(NULL, xmlStr, "domainsnapshot.xml", &ctxt); if (!xml) { @@ -11454,6 +11462,29 @@ virDomainSnapshotDefParseString(const char *xmlStr, unsigned int flags) state); goto cleanup; } + + /* Older snapshots were created with just /, 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))) { + xmlNodePtr domainNode = virXPathNode("./domain", ctxt); + + VIR_FREE(tmp); + if (!domainNode) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing domain in snapshot")); + goto cleanup; + } + def->dom = virDomainDefParseNode(caps, xml, domainNode, + expectedVirtTypes, + (VIR_DOMAIN_XML_INACTIVE | + VIR_DOMAIN_XML_SECURE)); + if (!def->dom) + goto cleanup; + } else { + VIR_WARN("parsing older snapshot that lacks domain"); + } } else { def->creationTime = tv.tv_sec; } @@ -11482,10 +11513,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, "\n"); virBufferAsprintf(&buf, " %s\n", def->name); if (def->description) @@ -11500,9 +11536,13 @@ char *virDomainSnapshotDefFormat(char *domain_uuid, } virBufferAsprintf(&buf, " %lld\n", def->creationTime); - virBufferAddLit(&buf, " \n"); - virBufferAsprintf(&buf, " %s\n", domain_uuid); - virBufferAddLit(&buf, " \n"); + if (def->dom) { + virDomainDefFormatInternal(def->dom, flags, &buf); + } else { + virBufferAddLit(&buf, " \n"); + virBufferAsprintf(&buf, " %s\n", domain_uuid); + virBufferAddLit(&buf, " \n"); + } if (internal) virBufferAsprintf(&buf, " %d\n", def->current); virBufferAddLit(&buf, "\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cef0d9ee93..e218a30244 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1385,6 +1385,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 */ @@ -1413,10 +1414,13 @@ typedef enum { } virDomainSnapshotParseFlags; virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, + 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 2bdb5d0cf2..18edc454fd 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -4220,7 +4220,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, return NULL; } - def = virDomainSnapshotDefParseString(xmlDesc, 0); + def = virDomainSnapshotDefParseString(xmlDesc, NULL, 0, 0); if (def == NULL) { return NULL; @@ -4316,7 +4316,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 0b8f1d328c..4d80e2fe2f 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -15698,10 +15698,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 0e022ad4aa..a1557b1198 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -340,7 +340,9 @@ static void qemuDomainSnapshotLoad(void *payload, continue; } - def = virDomainSnapshotDefParseString(xmlStr, flags); + def = virDomainSnapshotDefParseString(xmlStr, qemu_driver->caps, + QEMU_EXPECTED_VIRT_TYPES, + flags); if (def == NULL) { /* Nothing we can do here, skip this one */ VIR_ERROR(_("Failed to parse snapshot XML from file '%s'"), @@ -1610,9 +1612,11 @@ qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, char *snapDir = NULL; char *snapFile = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; + char *tmp; 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; @@ -1638,6 +1642,14 @@ qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, _("failed to create snapshot file '%s'"), snapFile); goto cleanup; } + + if (virAsprintf(&tmp, "snapshot-edit %s", vm->def->name) < 0) { + virReportOOMError(); + goto cleanup; + } + 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); @@ -8781,7 +8793,9 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain, if (!qemuDomainSnapshotIsAllowed(vm)) goto cleanup; - if (!(def = virDomainSnapshotDefParseString(xmlDesc, parse_flags))) + if (!(def = virDomainSnapshotDefParseString(xmlDesc, driver->caps, + QEMU_EXPECTED_VIRT_TYPES, + parse_flags))) goto cleanup; if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) { @@ -9083,8 +9097,6 @@ static char *qemuDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, virDomainSnapshotObjPtr snap = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; - /* XXX Actually wire this up once we return domain xml; for now, - * it is trivially safe to ignore this flag. */ virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL); qemuDriverLock(driver); @@ -9104,7 +9116,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 ebed4d9071..cbe34e8f5e 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -5658,7 +5658,7 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom, /* VBox has no snapshot metadata, so this flag is trivial. */ virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA, NULL); - if (!(def = virDomainSnapshotDefParseString(xmlDesc, 0))) + if (!(def = virDomainSnapshotDefParseString(xmlDesc, NULL, 0, 0))) goto cleanup; vboxIIDFromUUID(&domiid, dom->uuid); @@ -5840,7 +5840,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);