Re: [libvirt] [PATCH 01/20] snapshot: new XML for external system checkpoint

From: Eric Blake&lt;eblake(a)redhat.com&gt; Each<domainsnapshot> can now contain an optional<memory> element that describes how the VM state was handled, similar to disk snapshots. The new element will always appear in output; for back-compat, an input that lacks the element will assume 'no' or 'internal' according to the domain state. Along with this change, it is now possible to pass<disks> in the XML for an offline snapshot; this also needs to be wired up in a future patch, to make it possible to choose internal vs. external on a per-disk basis for each disk in an offline domain. At that point, using the --disk-only flag for an offline domain will be able to work. For some examples below, remember that qemu supports the following snapshot actions: qemu-img: offline external and internal disk savevm: online internal VM and disk migrate: online external VM transaction: online external disk ===== <domainsnapshot> <memory snapshot='no'/> ... </domainsnapshot> implies that there is no VM state saved (mandatory for offline and disk-only snapshots, not possible otherwise); using qemu-img for offline domains and transaction for online. ===== <domainsnapshot> <memory snapshot='internal'/> ... </domainsnapshot> state is saved inside one of the disks (as in qemu's 'savevm' system checkpoint implementation). If needed in the future, we can also add an attribute pointing out _which_ disk saved the internal state; maybe disk='vda'. ===== <domainsnapshot> <memory snapshot='external' file='/path/to/state'/> ... </domainsnapshot> This is not wired up yet, but future patches will allow this to control a combination of 'virsh save /path/to/state' plus disk snapshots from the same point in time. ===== So for 0.10.2, I plan to implement this table of combinations, with '*' designating new code and '+' designating existing code reached through new combinations of xml and/or the existing DISK_ONLY flag: domain memory disk disk-only | result ----------------------------------------- offline omit omit any | memory=no disk=int, via qemu-img offline no omit any |+memory=no disk=int, via qemu-img offline omit/no no any | invalid combination (nothing to snapshot) offline omit/no int any |+memory=no disk=int, via qemu-img offline omit/no ext any |*memory=no disk=ext, via qemu-img offline int/ext any any | invalid combination (no memory to save) online omit omit off | memory=int disk=int, via savevm online omit omit on | memory=no disk=default, via transaction online omit no/ext off | unsupported for now online omit no on | invalid combination (nothing to snapshot) online omit ext on | memory=no disk=ext, via transaction online omit int off |+memory=int disk=int, via savevm online omit int on | unsupported for now online no omit any |+memory=no disk=default, via transaction online no no any | invalid combination (nothing to snapshot) online no int any | unsupported for now online no ext any |+memory=no disk=ext, via transaction online int/ext any on | invalid combination (disk-only vs. memory) online int omit off |+memory=int disk=int, via savevm online int no/ext off | unsupported for now online int int off |+memory=int disk=int, via savevm online ext omit off |*memory=ext disk=default, via migrate+trans online ext no off |+memory=ext disk=no, via migrate online ext int off | unsupported for now online ext ext off |*memory=ext disk=ext, via migrate+transaction * docs/schemas/domainsnapshot.rng (memory): New RNG element. * docs/formatsnapshot.html.in: Document it. * src/conf/snapshot_conf.h (virDomainSnapshotDef): New fields. * src/conf/domain_conf.c (virDomainSnapshotDefFree) (virDomainSnapshotDefParseString, virDomainSnapshotDefFormat): Manage new fields. * tests/domainsnapshotxml2xmltest.c: New test. * tests/domainsnapshotxml2xmlin/*.xml: Update existing tests. * tests/domainsnapshotxml2xmlout/*.xml: Likewise. --- docs/formatsnapshot.html.in | 11 ++++ docs/schemas/domainsnapshot.rng | 23 ++++++++ src/conf/snapshot_conf.c | 66 +++++++++++++++++----- src/conf/snapshot_conf.h | 4 ++ tests/domainsnapshotxml2xmlin/external_vm.xml | 10 ++++ tests/domainsnapshotxml2xmlin/noparent.xml | 9 +++ tests/domainsnapshotxml2xmlout/all_parameters.xml | 1 + tests/domainsnapshotxml2xmlout/disk_snapshot.xml | 1 + tests/domainsnapshotxml2xmlout/external_vm.xml | 43 ++++++++++++++ tests/domainsnapshotxml2xmlout/full_domain.xml | 1 + tests/domainsnapshotxml2xmlout/metadata.xml | 1 + tests/domainsnapshotxml2xmlout/noparent.xml | 1 + .../noparent_nodescription.xml | 1 + .../noparent_nodescription_noactive.xml | 1 + tests/domainsnapshotxml2xmltest.c | 1 + 15 files changed, 161 insertions(+), 13 deletions(-) create mode 100644 tests/domainsnapshotxml2xmlin/external_vm.xml create mode 100644 tests/domainsnapshotxml2xmlin/noparent.xml create mode 100644 tests/domainsnapshotxml2xmlout/external_vm.xml diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in index ec5ebf3..2af32bc 100644 --- a/docs/formatsnapshot.html.in +++ b/docs/formatsnapshot.html.in @@ -106,6 +106,17 @@ description is omitted when initially creating the snapshot, then this field will be empty. </dd> +<dt><code>memory</code></dt> +<dd>On input, this is an optional request for how to handle VM + state. For an offline domain or a disk-only snapshot, + attribute<code>snapshot</code> must be<code>no</code>, since + there is no VM state saved; otherwise, the attribute can + be<code>internal</code> if the VM state is piggy-backed with + other internal disk state, or<code>external</code> along with + a second attribute<code>file</code> giving the absolute path + of the file holding the VM state.<span class="since">Since + 0.10.2</span> +</dd> <dt><code>disks</code></dt> <dd>On input, this is an optional listing of specific instructions for disk snapshots; it is needed when making a diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng index ecaafe9..45d55b5 100644 --- a/docs/schemas/domainsnapshot.rng +++ b/docs/schemas/domainsnapshot.rng @@ -31,6 +31,29 @@ </element> </optional> <optional> +<element name='memory'> +<choice> +<attribute name='snapshot'> +<choice> +<value>no</value> +<value>internal</value> +</choice> +</attribute> +<group> +<optional> +<attribute name='snapshot'> +<value>external</value> +</attribute> +</optional> +<attribute name='file'> +<ref name='absFilePath'/> +</attribute> +</group> +</choice> +<empty/> +</element> +</optional> +<optional> <element name='disks'> <zeroOrMore> <ref name='disksnapshot'/> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 16c844d..6f77026 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -94,6 +94,7 @@ void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def) VIR_FREE(def->name); VIR_FREE(def->description); VIR_FREE(def->parent); + VIR_FREE(def->file); for (i = 0; i< def->ndisks; i++) virDomainSnapshotDiskDefClear(&def->disks[i]); VIR_FREE(def->disks); @@ -182,6 +183,9 @@ virDomainSnapshotDefParseString(const char *xmlStr, int active; char *tmp; int keepBlanksDefault = xmlKeepBlanksDefault(0); + char *memorySnapshot = NULL; + char *memoryFile = NULL; + bool offline = !!(flags& VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE); xml = virXMLParseCtxt(NULL, xmlStr, _("(domain_snapshot)"),&ctxt); if (!xml) { @@ -208,15 +212,11 @@ virDomainSnapshotDefParseString(const char *xmlStr, virReportError(VIR_ERR_XML_ERROR, "%s", _("a redefined snapshot must have a name")); goto cleanup; - } else { - ignore_value(virAsprintf(&def->name, "%lld", - (long long)tv.tv_sec)); } - } - - if (def->name == NULL) { - virReportOOMError(); - goto cleanup; + if (virAsprintf(&def->name, "%lld", (long long)tv.tv_sec)< 0) { + virReportOOMError(); + goto cleanup; + }

} def->description = virXPathString("string(./description)", ctxt); @@ -247,6 +247,8 @@ virDomainSnapshotDefParseString(const char *xmlStr, state); goto cleanup; } + offline = (def->state == VIR_DOMAIN_SHUTOFF || + def->state == VIR_DOMAIN_DISK_SNAPSHOT); /* Older snapshots were created with just<domain>/<uuid>, and * lack domain/@type. In that case, leave dom NULL, and @@ -274,11 +276,42 @@ virDomainSnapshotDefParseString(const char *xmlStr, def->creationTime = tv.tv_sec; } + memorySnapshot = virXPathString("string(./memory/@snapshot)", ctxt); + memoryFile = virXPathString("string(./memory/@file)", ctxt); + if (memorySnapshot) { + def->memory = virDomainSnapshotLocationTypeFromString(memorySnapshot); + if (def->memory<= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown memory snapshot setting '%s'"), + memorySnapshot); + goto cleanup; + } + if (memoryFile&& + def->memory != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { + virReportError(VIR_ERR_XML_ERROR, + _("memory filename '%s' requires external snapshot"), + memoryFile); + goto cleanup; + } + } else if (memoryFile) { + def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; + } else if (flags& VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) { + def->memory = (offline ? + VIR_DOMAIN_SNAPSHOT_LOCATION_NONE : + VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL); + } + if (offline&& def->memory&&

+ def->memory != VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("memory state cannot be saved with offline snapshot"));