[libvirt] [PATCH 01/20] snapshot: new XML for external system checkpoint
Osier Yang
jyang at redhat.com
Wed Oct 24 03:41:00 UTC 2012
On 2012年10月23日 23:12, Peter Krempa wrote:
> From: Eric Blake<eblake at redhat.com>
>
> 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;
> + }
Okay, coding style improvements.
> }
>
> 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&&
I think def->memory checking is redundant here. Not big deal though.
> + def->memory != VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("memory state cannot be saved with offline snapshot"));
"VM state" is used both in the commit message and docs. Better
to keep consistent, I prefer "memory state" more, as it's more clear.
"VM state" could include disk state too. Correct me if I'm not right.
Others looks just very sanity, ACK.
Regards,
Osier
More information about the libvir-list
mailing list