On 2012年10月23日 23:12, Peter Krempa wrote:
From: Eric Blake<eblake@xxxxxxxxxx>
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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list