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. * 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. Based on a patch by Philipp Hahn. --- Differences from Philipp's v1: rebase to latest libvirt.git caps are only needed when parsing an internal xml (that is, esx and vbox no longer have a to-do) on format, pass flags down from user src/conf/domain_conf.c | 49 +++++++++++++++++++++++++++++++++++++++++------ src/conf/domain_conf.h | 7 +++++- src/esx/esx_driver.c | 4 +- src/qemu/qemu_driver.c | 18 +++++++++++++--- src/vbox/vbox_tmpl.c | 4 +- 5 files changed, 66 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 881350e..295b92f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10915,11 +10915,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; @@ -10927,6 +10933,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, virDomainSnapshotDefPtr ret = NULL; char *creation = NULL, *state = NULL; struct timeval tv; + char *tmp; xml = virXMLParse(NULL, xmlStr, "domainsnapshot.xml"); if (!xml) { @@ -10995,9 +11002,28 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, _("Could not find 'active' element")); goto cleanup; } - } - 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"); + } def->creationTime = tv.tv_sec; + } ret = def; @@ -11014,10 +11040,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) @@ -11032,9 +11063,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>%ld</active>\n", def->active); virBufferAddLit(&buf, "</domainsnapshot>\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e37cf26..0241838 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1291,6 +1291,7 @@ struct _virDomainSnapshotDef { char *parent; long long creationTime; /* in seconds */ int state; + virDomainDefPtr dom; long active; /* XXX make this internal use only to identify current snap */ }; @@ -1313,10 +1314,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 c71b1b3..eb1633d 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/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index da2703e..565f578 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -336,7 +336,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'"), fullpath); @@ -8467,7 +8470,8 @@ static int 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; @@ -8493,6 +8497,12 @@ static int 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); @@ -8688,7 +8698,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))) @@ -8929,7 +8939,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 91a5423..b8deed3 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list