Prepare for new checkpoint APIs by describing the XML that will
represent a checkpoint. The checkpoint XML is modeled heavily after
virDomainSnapshotPtr. See the docs for more details.
Add testsuite coverage for some minimal uses of the XML (bare minimum,
the sample from html, and a full dumpxml). Although use of the
REDEFINE flag will require the <domain> subelement to be present, it
is easier for most of the tests to provide counterpart output produced
with the NO_DOMAIN flag (particularly since synthesizing a valid
<domain> during testing is not trivial).
Signed-off-by: Eric Blake <eblake@redhat.com>
---
docs/docs.html.in | 3 +-
docs/format.html.in | 1 +
docs/formatcheckpoint.html.in | 218 ++++++++++++++++++
docs/formatsnapshot.html.in | 4 +-
docs/index.html.in | 3 +-
docs/schemas/domaincheckpoint.rng | 95 ++++++++
libvirt.spec.in | 1 +
mingw-libvirt.spec.in | 2 +
tests/Makefile.am | 2 +
tests/qemudomaincheckpointxml2xmlin/empty.xml | 1 +
.../qemudomaincheckpointxml2xmlin/sample.xml | 7 +
tests/qemudomaincheckpointxml2xmlin/size.xml | 4 +
.../qemudomaincheckpointxml2xmlout/empty.xml | 8 +
.../redefine.xml | 64 +++++
.../qemudomaincheckpointxml2xmlout/sample.xml | 13 ++
tests/qemudomaincheckpointxml2xmlout/size.xml | 12 +
tests/virschematest.c | 2 +
17 files changed, 437 insertions(+), 3 deletions(-)
create mode 100644 docs/formatcheckpoint.html.in
create mode 100644 docs/schemas/domaincheckpoint.rng
create mode 100644 tests/qemudomaincheckpointxml2xmlin/empty.xml
create mode 100644 tests/qemudomaincheckpointxml2xmlin/sample.xml
create mode 100644 tests/qemudomaincheckpointxml2xmlin/size.xml
create mode 100644 tests/qemudomaincheckpointxml2xmlout/empty.xml
create mode 100644 tests/qemudomaincheckpointxml2xmlout/redefine.xml
create mode 100644 tests/qemudomaincheckpointxml2xmlout/sample.xml
create mode 100644 tests/qemudomaincheckpointxml2xmlout/size.xml
diff --git a/docs/docs.html.in b/docs/docs.html.in
index c8674e1457..55cbab5f15 100644
--- a/docs/docs.html.in
+++ b/docs/docs.html.in
@@ -81,7 +81,8 @@
<a href="formatstoragecaps.html">storage pool capabilities</a>,
<a href="formatnode.html">node devices</a>,
<a href="formatsecret.html">secrets</a>,
- <a href="formatsnapshot.html">snapshots</a></dd>
+ <a href="formatsnapshot.html">snapshots</a>,
+ <a href="formatcheckpoint.html">checkpoints</a></dd>
<dt><a href="uri.html">URI format</a></dt>
<dd>The URI formats used for connecting to libvirt</dd>
diff --git a/docs/format.html.in b/docs/format.html.in
index 640a9957ee..20f9ef3348 100644
--- a/docs/format.html.in
+++ b/docs/format.html.in
@@ -25,6 +25,7 @@
<li><a href="formatnode.html">Node devices</a></li>
<li><a href="formatsecret.html">Secrets</a></li>
<li><a href="formatsnapshot.html">Snapshots</a></li>
+ <li><a href="formatcheckpoint.html">Checkpoints</a></li>
</ul>
<h2>Command line validation</h2>
diff --git a/docs/formatcheckpoint.html.in b/docs/formatcheckpoint.html.in
new file mode 100644
index 0000000000..ef5f8a826b
--- /dev/null
+++ b/docs/formatcheckpoint.html.in
@@ -0,0 +1,218 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE html>
+<html xmlns="http://www.w3.org/1999/xhtml">
+ <body>
+ <h1>Checkpoint XML format</h1>
+
+ <ul id="toc"></ul>
+
+ <h2><a id="CheckpointAttributes">Checkpoint XML</a></h2>
+
+ <p>
+ One method of capturing domain disk backups is via the use of
+ incremental backups. Right now, incremental backups are only
+ supported for the QEMU hypervisor when using qcow2 disks at the
+ active layer; if other disk formats are in use, capturing disk
+ backups requires different libvirt APIs.
+ </p>
+ <p>
+ Libvirt is able to facilitate incremental backups by tracking
+ disk checkpoints, which are points in time against which it is
+ easy to compute which portion of the disk has changed. Given a
+ full backup (a backup created from the creation of the disk to a
+ given point in time), coupled with the creation of a disk
+ checkpoint at that time, and an incremental backup (a backup
+ created from just the dirty portion of the disk between the
+ first checkpoint and the second backup operation), it is
+ possible to do an offline reconstruction of the state of the
+ disk at the time of the second backup without having to copy as
+ much data as a second full backup would require. Future API
+ additions will make it possible to create checkpoints in
+ conjunction with a backup
+ via <code>virDomainBackupBegin()</code> or with an external
+ snapshot via <code>virDomainSnapshotCreateXML2</code>; but for
+ now, libvirt exposes enough support to create disk checkpoints
+ independently from a backup operation
+ via <code>virDomainCheckpointCreateXML()</code>.
+ </p>
+ <p>
+ Attributes of libvirt checkpoints are stored as child elements
+ of the <code>domaincheckpoint</code> element. At checkpoint
+ creation time, normally only
+ the <code>name</code>, <code>description</code>,
+ and <code>disks</code> elements are settable. The rest of the
+ fields are ignored on creation and will be filled in by libvirt
+ in for informational purposes
+ by <code>virDomainCheckpointGetXMLDesc()</code>. However, when
+ redefining a checkpoint, with
+ the <code>VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE</code> flag
+ of <code>virDomainCheckpointCreateXML()</code>, all of the XML
+ fields described here are relevant on input, even the fields
+ that are normally described as readonly for output.
+ </p>
+ <p>
+ A checkpoint is marked current if the hypervisor is actively
+ updating the checkpoint as guest operations modify the disk
+ after the point where the checkpoint was created. When tracking
+ multiple checkpoints, a hypervisor may choose to either have all
+ relevant checkpoints be current (a guest write updates the
+ associated tracking for each checkpoint), or to have a single
+ checkpoint be current (older checkpoints stop tracking changes
+ when a newer one is created, and describing all changes since an
+ older checkpoint then involves libvirt automatically combining
+ the changes from the chain of checkpoints from the current back
+ to the checkpoint in question; the QEMU hypervisor uses this
+ method). Checkpoints are maintained in a hierarchy to facilitate
+ hypervisors that use this latter method, where libvirt
+ automatically updates the parent-child relationship as well as
+ which checkpoint is current when creating or deleting
+ checkpoints. For now, the creation of checkpoints is not
+ possible when external snapshots exist, but this may be added in
+ the future.
+ </p>
+ <p>
+ The top-level <code>domaincheckpoint</code> element may contain
+ the following elements:
+ </p>
+ <dl>
+ <dt><code>name</code></dt>
+ <dd>The optional name for this checkpoint. If the name is
+ omitted, libvirt will create a name based on the time of the
+ creation.
+ </dd>
+ <dt><code>description</code></dt>
+ <dd>An optional human-readable description of the checkpoint.
+ If the description is omitted when initially creating the
+ checkpoint, then this field will be empty.
+ </dd>
+ <dt><code>disks</code></dt>
+ <dd>On input, this is an optional listing of specific
+ instructions for disk checkpoints; it is needed when making a
+ checkpoint on only a subset of the disks associated with a
+ domain. In particular, since QEMU checkpoints require qcow2
+ disks, this element may be needed on input for excluding guest
+ disks that are not in qcow2 format. If the entire element was
+ omitted on input, then all disks participate in the
+ checkpoint, otherwise, only the disks explicitly listed which
+ do not also use <code>checkpoint='no'</code> will
+ participate. On output, this is the checkpoint state of each
+ of the domain's disks.
+ <dl>
+ <dt><code>disk</code></dt>
+ <dd>This sub-element describes the checkpoint properties of
+ a specific disk with the following attributes:
+ <dl>
+ <dt><code>name</code></dt>
+ <dd>A mandatory attribute which must match either
+ the <code><target dev='name'/></code> or an
+ unambiguous <code><source file='name'/></code>
+ of one of
+ the <a href="formatdomain.html#elementsDisks">disk
+ devices</a> specified for the domain at the time of
+ the checkpoint.</dd>
+ <dt><code>checkpoint</code></dt>
+ <dd>An optional attribute; possible values
+ are <code>no</code> when the disk does not participate
+ in this checkpoint; or <code>bitmap</code> if the disk
+ will track all changes since the creation of this
+ checkpoint via a bitmap.</dd>
+ <dt><code>bitmap</code></dt>
+ <dd>The attribute <code>bitmap</code> is only valid
+ if <code>checkpoint='bitmap'</code>; it describes the
+ name of the tracking bitmap (defaulting to the
+ checkpoint name).</dd>
+ <dt><code>size</code></dt>
+ <dd>The attribute <code>size</code> is ignored on input;
+ on output, it is only present if
+ the <code>VIR_DOMAIN_CHECKPOINT_XML_SIZE</code> flag
+ was used to perform a dynamic query of the estimated
+ size in bytes of the changes made since the checkpoint
+ was created.</dd>
+ </dl>
+ </dd>
+ </dl>
+ </dd>
+ <dt><code>creationTime</code></dt>
+ <dd>A readonly representation of the time this checkpoint was
+ created. The time is specified in seconds since the Epoch,
+ UTC (i.e. Unix time).
+ </dd>
+ <dt><code>parent</code></dt>
+ <dd>An optional readonly representation of the parent of this
+ checkpoint. If present, this element contains exactly one
+ child element, <code>name</code>.
+ </dd>
+ <dt><code>current</code></dt>
+ <dd>A readonly integer, 1 if this checkpoint is current (that
+ is, actively tracking guest changes) or 0 if the checkpoint is
+ inactive because a child checkpoint is now tracking changes.
+ </dd>
+ <dt><code>domain</code></dt>
+ <dd>A readonly representation of the
+ inactive <a href="formatdomain.html">domain configuration</a>
+ at the time the checkpoint was created. This element may be
+ omitted for output brevity by supplying
+ the <code>VIR_DOMAIN_CHECKPOINT_XML_NO_DOMAIN</code> flag, but
+ the resulting XML is no longer viable for use with
+ the <code>VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE</code> flag
+ of <code>virDomainCheckpointCreateXML()</code>. The domain
+ will have security-sensitive information omitted unless the
+ flag <code>VIR_DOMAIN_CHECKPOINT_XML_SECURE</code> is provided
+ on a read-write connection.
+ </dd>
+ </dl>
+
+ <h2><a id="example">Examples</a></h2>
+
+ <p>Using this XML to create a checkpoint of just vda on a qemu
+ domain with two disks and a prior checkpoint:</p>
+ <pre>
+<domaincheckpoint>
+ <description>Completion of updates after OS install</description>
+ <disks>
+ <disk name='vda' checkpoint='bitmap'/>
+ <disk name='vdb' checkpoint='no'/>
+ </disks>
+</domaincheckpoint></pre>
+
+ <p>will result in XML similar to this from
+ <code>virDomainCheckpointGetXMLDesc()</code>:</p>
+ <pre>
+<domaincheckpoint>
+ <name>1525889631</name>
+ <description>Completion of updates after OS install</description>
+ <parent>
+ <name>1525111885</name>
+ </parent>
+ <creationTime>1525889631</creationTime>
+ <disks>
+ <disk name='vda' checkpoint='bitmap' bitmap='1525889631'/>
+ <disk name='vdb' checkpoint='no'/>
+ </disks>
+ <domain type='qemu'>
+ <name>fedora</name>
+ <uuid>93a5c045-6457-2c09-e56c-927cdf34e178</uuid>
+ <memory>1048576</memory>
+ ...
+ <devices>
+ <disk type='file' device='disk'>
+ <driver name='qemu' type='qcow2'/>
+ <source file='/path/to/file1'/>
+ <target dev='vda' bus='virtio'/>
+ </disk>
+ <disk type='file' device='disk' snapshot='external'>
+ <driver name='qemu' type='raw'/>
+ <source file='/path/to/file2'/>
+ <target dev='vdb' bus='virtio'/>
+ </disk>
+ ...
+ </devices>
+ </domain>
+</domaincheckpoint></pre>
+
+ <p>With that checkpoint created, the qcow2 image is now tracking
+ all changes that occur in the image since the checkpoint via
+ the persistent bitmap named <code>1525889631</code>.
+ </p>
+ </body>
+</html>
diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in
index 92cc566467..2bfb69cf49 100644
--- a/docs/formatsnapshot.html.in
+++ b/docs/formatsnapshot.html.in
@@ -91,7 +91,9 @@
sets that snapshot as current, and the prior current snapshot is
the parent of the new snapshot. Branches in the hierarchy can
be formed by reverting to a snapshot with a child, then creating
- another snapshot.
+ another snapshot. For now, the creation of external snapshots
+ when checkpoints exist is forbidden, although future work will
+ make it possible to integrate these two concepts.
</p>
<p>
The top-level <code>domainsnapshot</code> element may contain
diff --git a/docs/index.html.in b/docs/index.html.in
index 2bd647f8dd..7d0ab650e3 100644
--- a/docs/index.html.in
+++ b/docs/index.html.in
@@ -58,7 +58,8 @@
<a href="formatstoragecaps.html">storage pool capabilities</a>,
<a href="formatnode.html">node devices</a>,
<a href="formatsecret.html">secrets</a>,
- <a href="formatsnapshot.html">snapshots</a></dd>
+ <a href="formatsnapshot.html">snapshots</a>,
+ <a href="formatcheckpoint.html">checkpoints</a></dd>
<dt><a href="http://wiki.libvirt.org">Wiki</a></dt>
<dd>Read further community contributed content</dd>
</dl>
diff --git a/docs/schemas/domaincheckpoint.rng b/docs/schemas/domaincheckpoint.rng
new file mode 100644
index 0000000000..ceb5436aa3
--- /dev/null
+++ b/docs/schemas/domaincheckpoint.rng
@@ -0,0 +1,95 @@
+<?xml version="1.0"?>
+<!-- A Relax NG schema for the libvirt domain checkpoint properties XML format -->
+<grammar xmlns="http://relaxng.org/ns/structure/1.0">
+ <start>
+ <ref name='domaincheckpoint'/>
+ </start>
+
+ <include href='domaincommon.rng'/>
+
+ <define name='domaincheckpoint'>
+ <element name='domaincheckpoint'>
+ <interleave>
+ <optional>
+ <element name='name'>
+ <text/>
+ </element>
+ </optional>
+ <optional>
+ <element name='description'>
+ <text/>
+ </element>
+ </optional>
+ <optional>
+ <element name='creationTime'>
+ <text/>
+ </element>
+ </optional>
+ <optional>
+ <element name='current'>
+ <choice>
+ <value>0</value>
+ <value>1</value>
+ </choice>
+ </element>
+ </optional>
+ <optional>
+ <element name='disks'>
+ <oneOrMore>
+ <ref name='diskcheckpoint'/>
+ </oneOrMore>
+ </element>
+ </optional>
+ <optional>
+ <!-- Nested grammar ensures that any of our overrides of
+ storagecommon/domaincommon defines do not conflict
+ with any domain.rng overrides. -->
+ <grammar>
+ <include href='domain.rng'/>
+ </grammar>
+ </optional>
+ <optional>
+ <element name='parent'>
+ <element name='name'>
+ <text/>
+ </element>
+ </element>
+ </optional>
+ </interleave>
+ </element>
+ </define>
+
+ <define name='diskcheckpoint'>
+ <element name='disk'>
+ <attribute name='name'>
+ <choice>
+ <ref name='diskTarget'/>
+ <ref name='absFilePath'/>
+ </choice>
+ </attribute>
+ <choice>
+ <attribute name='checkpoint'>
+ <value>no</value>
+ </attribute>
+ <group>
+ <optional>
+ <attribute name='checkpoint'>
+ <value>bitmap</value>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name='bitmap'>
+ <text/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name='size'>
+ <ref name="unsignedLong"/>
+ </attribute>
+ </optional>
+ </group>
+ </choice>
+ </element>
+ </define>
+
+</grammar>
diff --git a/libvirt.spec.in b/libvirt.spec.in
index d54f58f1d4..dece22688d 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1780,6 +1780,7 @@ exit 0
%{_datadir}/libvirt/schemas/cputypes.rng
%{_datadir}/libvirt/schemas/domain.rng
%{_datadir}/libvirt/schemas/domaincaps.rng
+%{_datadir}/libvirt/schemas/domaincheckpoint.rng
%{_datadir}/libvirt/schemas/domaincommon.rng
%{_datadir}/libvirt/schemas/domainsnapshot.rng
%{_datadir}/libvirt/schemas/interface.rng
diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
index ad6d6e952e..df89d9f8b3 100644
--- a/mingw-libvirt.spec.in
+++ b/mingw-libvirt.spec.in
@@ -232,6 +232,7 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh
%{mingw32_datadir}/libvirt/schemas/cputypes.rng
%{mingw32_datadir}/libvirt/schemas/domain.rng
%{mingw32_datadir}/libvirt/schemas/domaincaps.rng
+%{mingw32_datadir}/libvirt/schemas/domaincheckpoint.rng
%{mingw32_datadir}/libvirt/schemas/domaincommon.rng
%{mingw32_datadir}/libvirt/schemas/domainsnapshot.rng
%{mingw32_datadir}/libvirt/schemas/interface.rng
@@ -321,6 +322,7 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh
%{mingw64_datadir}/libvirt/schemas/cputypes.rng
%{mingw64_datadir}/libvirt/schemas/domain.rng
%{mingw64_datadir}/libvirt/schemas/domaincaps.rng
+%{mingw64_datadir}/libvirt/schemas/domaincheckpoint.rng
%{mingw64_datadir}/libvirt/schemas/domaincommon.rng
%{mingw64_datadir}/libvirt/schemas/domainsnapshot.rng
%{mingw64_datadir}/libvirt/schemas/interface.rng
diff --git a/tests/Makefile.am b/tests/Makefile.am
index e57d0da58a..66c6ad911e 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -110,6 +110,8 @@ EXTRA_DIST = \
qemublocktestdata \
qemucapabilitiesdata \
qemucaps2xmloutdata \
+ qemudomaincheckpointxml2xmlin \
+ qemudomaincheckpointxml2xmlout \
qemudomainsnapshotxml2xmlin \
qemudomainsnapshotxml2xmlout \
qemuhotplugtestcpus \
diff --git a/tests/qemudomaincheckpointxml2xmlin/empty.xml b/tests/qemudomaincheckpointxml2xmlin/empty.xml
new file mode 100644
index 0000000000..dc36449142
--- /dev/null
+++ b/tests/qemudomaincheckpointxml2xmlin/empty.xml
@@ -0,0 +1 @@
+<domaincheckpoint/>
diff --git a/tests/qemudomaincheckpointxml2xmlin/sample.xml b/tests/qemudomaincheckpointxml2xmlin/sample.xml
new file mode 100644
index 0000000000..70ed964e1e
--- /dev/null
+++ b/tests/qemudomaincheckpointxml2xmlin/sample.xml
@@ -0,0 +1,7 @@
+<domaincheckpoint>
+ <description>Completion of updates after OS install</description>
+ <disks>
+ <disk name='vda' checkpoint='bitmap'/>
+ <disk name='vdb' checkpoint='no'/>
+ </disks>
+</domaincheckpoint>
diff --git a/tests/qemudomaincheckpointxml2xmlin/size.xml b/tests/qemudomaincheckpointxml2xmlin/size.xml
new file mode 100644
index 0000000000..99ce84c7a0
--- /dev/null
+++ b/tests/qemudomaincheckpointxml2xmlin/size.xml
@@ -0,0 +1,4 @@
+<domaincheckpoint>
+ <name>c2</name>
+ <description>something fun</description>
+</domaincheckpoint>
diff --git a/tests/qemudomaincheckpointxml2xmlout/empty.xml b/tests/qemudomaincheckpointxml2xmlout/empty.xml
new file mode 100644
index 0000000000..ff49bef17f
--- /dev/null
+++ b/tests/qemudomaincheckpointxml2xmlout/empty.xml
@@ -0,0 +1,8 @@
+<domaincheckpoint>
+ <name>1525889631</name>
+ <creationTime>1525889631</creationTime>
+ <current>0</current>
+ <disks>
+ <disk name='vda' checkpoint='bitmap' bitmap='1525889631'/>
+ </disks>
+</domaincheckpoint>
diff --git a/tests/qemudomaincheckpointxml2xmlout/redefine.xml b/tests/qemudomaincheckpointxml2xmlout/redefine.xml
new file mode 100644
index 0000000000..44310c2e1d
--- /dev/null
+++ b/tests/qemudomaincheckpointxml2xmlout/redefine.xml
@@ -0,0 +1,64 @@
+<domaincheckpoint>
+ <name>c1</name>
+ <creationTime>1553647812</creationTime>
+ <current>1</current>
+ <disks>
+ <disk name='hda' checkpoint='no'/>
+ <disk name='hdc' checkpoint='no'/>
+ <disk name='vda' checkpoint='bitmap' bitmap='c1'/>
+ <disk name='vdb' checkpoint='bitmap' bitmap='c1'/>
+ </disks>
+ <domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-i686</emulator>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+ </disk>
+ <disk type='block' device='cdrom'>
+ <driver name='qemu' type='raw'/>
+ <source dev='/dev/HostVG/QEMUGuest2'/>
+ <target dev='hdc' bus='ide'/>
+ <readonly/>
+ <address type='drive' controller='0' bus='1' target='0' unit='0'/>
+ </disk>
+ <disk type='file' device='disk'>
+ <driver name='qemu' type='qcow2'/>
+ <source file='/tmp/data.img'/>
+ <target dev='vda' bus='virtio'/>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+ </disk>
+ <disk type='file' device='disk'>
+ <driver name='qemu' type='qcow2'/>
+ <source file='/tmp/logs.img'/>
+ <target dev='vdb' bus='virtio'/>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
+ </disk>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+ </domain>
+</domaincheckpoint>
diff --git a/tests/qemudomaincheckpointxml2xmlout/sample.xml b/tests/qemudomaincheckpointxml2xmlout/sample.xml
new file mode 100644
index 0000000000..913c26d2b9
--- /dev/null
+++ b/tests/qemudomaincheckpointxml2xmlout/sample.xml
@@ -0,0 +1,13 @@
+<domaincheckpoint>
+ <name>1525889631</name>
+ <description>Completion of updates after OS install</description>
+ <parent>
+ <name>1525111885</name>
+ </parent>
+ <creationTime>1525889631</creationTime>
+ <current>0</current>
+ <disks>
+ <disk name='vda' checkpoint='bitmap' bitmap='1525889631'/>
+ <disk name='vdb' checkpoint='no'/>
+ </disks>
+</domaincheckpoint>
diff --git a/tests/qemudomaincheckpointxml2xmlout/size.xml b/tests/qemudomaincheckpointxml2xmlout/size.xml
new file mode 100644
index 0000000000..17d884983d
--- /dev/null
+++ b/tests/qemudomaincheckpointxml2xmlout/size.xml
@@ -0,0 +1,12 @@
+<domaincheckpoint>
+ <name>c2</name>
+ <description>something fun</description>
+ <parent>
+ <name>1525111885</name>
+ </parent>
+ <creationTime>1553648510</creationTime>
+ <current>1</current>
+ <disks>
+ <disk name='vda' checkpoint='bitmap' bitmap='c2' size='1048576'/>
+ </disks>
+</domaincheckpoint>
diff --git a/tests/virschematest.c b/tests/virschematest.c
index b7c2f7cfaa..8f5101df21 100644
--- a/tests/virschematest.c
+++ b/tests/virschematest.c
@@ -222,6 +222,8 @@ mymain(void)
"genericxml2xmloutdata", "xlconfigdata", "libxlxml2domconfigdata",
"qemuhotplugtestdomains");
DO_TEST_DIR("domaincaps.rng", "domaincapsschemadata");
+ DO_TEST_DIR("domaincheckpoint.rng", "qemudomaincheckpointxml2xmlin",
+ "qemudomaincheckpointxml2xmlout");
DO_TEST_DIR("domainsnapshot.rng", "qemudomainsnapshotxml2xmlin",
"qemudomainsnapshotxml2xmlout");
DO_TEST_DIR("interface.rng", "interfaceschemadata");
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Sat, Jul 06, 2019 at 22:56:01 -0500, Eric Blake wrote: > Prepare for new checkpoint APIs by describing the XML that will > represent a checkpoint. The checkpoint XML is modeled heavily after > virDomainSnapshotPtr. See the docs for more details. > > Add testsuite coverage for some minimal uses of the XML (bare minimum, > the sample from html, and a full dumpxml). Although use of the > REDEFINE flag will require the <domain> subelement to be present, it > is easier for most of the tests to provide counterpart output produced > with the NO_DOMAIN flag (particularly since synthesizing a valid > <domain> during testing is not trivial). > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- [...] > diff --git a/docs/formatcheckpoint.html.in b/docs/formatcheckpoint.html.in > new file mode 100644 > index 0000000000..ef5f8a826b > --- /dev/null > +++ b/docs/formatcheckpoint.html.in > @@ -0,0 +1,218 @@ > +<?xml version="1.0" encoding="UTF-8"?> > +<!DOCTYPE html> > +<html xmlns="http://www.w3.org/1999/xhtml"> > + <body> > + <h1>Checkpoint XML format</h1> > + > + <ul id="toc"></ul> > + > + <h2><a id="CheckpointAttributes">Checkpoint XML</a></h2> > + > + <p> > + One method of capturing domain disk backups is via the use of > + incremental backups. Right now, incremental backups are only > + supported for the QEMU hypervisor when using qcow2 disks at the > + active layer; if other disk formats are in use, capturing disk > + backups requires different libvirt APIs. Perhaps add a 'since' to note that all of this was added starting from 5.6. > + </p> > + <p> > + Libvirt is able to facilitate incremental backups by tracking > + disk checkpoints, which are points in time against which it is > + easy to compute which portion of the disk has changed. Given a > + full backup (a backup created from the creation of the disk to a > + given point in time), coupled with the creation of a disk > + checkpoint at that time, and an incremental backup (a backup > + created from just the dirty portion of the disk between the > + first checkpoint and the second backup operation), it is > + possible to do an offline reconstruction of the state of the > + disk at the time of the second backup without having to copy as > + much data as a second full backup would require. Future API > + additions will make it possible to create checkpoints in > + conjunction with a backup > + via <code>virDomainBackupBegin()</code> or with an external > + snapshot via <code>virDomainSnapshotCreateXML2</code>; but for > + now, libvirt exposes enough support to create disk checkpoints > + independently from a backup operation > + via <code>virDomainCheckpointCreateXML()</code>. So it will not happen in this release? Otherwise I don't see a point in documenting it. > + </p> > + <p> > + Attributes of libvirt checkpoints are stored as child elements > + of the <code>domaincheckpoint</code> element. At checkpoint > + creation time, normally only > + the <code>name</code>, <code>description</code>, > + and <code>disks</code> elements are settable. The rest of the > + fields are ignored on creation and will be filled in by libvirt > + in for informational purposes > + by <code>virDomainCheckpointGetXMLDesc()</code>. However, when > + redefining a checkpoint, with > + the <code>VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE</code> flag > + of <code>virDomainCheckpointCreateXML()</code>, all of the XML > + fields described here are relevant on input, even the fields > + that are normally described as readonly for output. > + </p> > + <p> > + A checkpoint is marked current if the hypervisor is actively > + updating the checkpoint as guest operations modify the disk > + after the point where the checkpoint was created. When tracking > + multiple checkpoints, a hypervisor may choose to either have all > + relevant checkpoints be current (a guest write updates thea So to reiterate. By definition all checkpoint should be "current", because they must be actively updated to be able to track the difference from the point when they were created until now. The fact whether it includes update to a bitmap and which bitmaps are updated does not seem to be relevant for outside users. In fact we may even opt to change how this is done or perhaps convert one layout of bitmaps into a different one if we insist. Thus I'm saying it again. "current" property seems to be wrong in conjunction with checkpoints. > + associated tracking for each checkpoint), or to have a single > + checkpoint be current (older checkpoints stop tracking changes NACK, internal detail. Don't document it. Also I'll object to this later in detail. > + when a newer one is created, and describing all changes since an > + older checkpoint then involves libvirt automatically combining > + the changes from the chain of checkpoints from the current back > + to the checkpoint in question; the QEMU hypervisor uses this > + method). Checkpoints are maintained in a hierarchy to facilitate Nope. All of the above seem to be internal stuff. > + hypervisors that use this latter method, where libvirt > + automatically updates the parent-child relationship as well as > + which checkpoint is current when creating or deleting > + checkpoints. For now, the creation of checkpoints is not > + possible when external snapshots exist, but this may be added in > + the future. > + </p> > + <p> > + The top-level <code>domaincheckpoint</code> element may contain > + the following elements: > + </p> > + <dl> > + <dt><code>name</code></dt> > + <dd>The optional name for this checkpoint. If the name is > + omitted, libvirt will create a name based on the time of the > + creation. > + </dd> > + <dt><code>description</code></dt> > + <dd>An optional human-readable description of the checkpoint. > + If the description is omitted when initially creating the > + checkpoint, then this field will be empty. > + </dd> > + <dt><code>disks</code></dt> > + <dd>On input, this is an optional listing of specific > + instructions for disk checkpoints; it is needed when making a > + checkpoint on only a subset of the disks associated with a > + domain. In particular, since QEMU checkpoints require qcow2 > + disks, this element may be needed on input for excluding guest > + disks that are not in qcow2 format. If the entire element was > + omitted on input, then all disks participate in the > + checkpoint, otherwise, only the disks explicitly listed which > + do not also use <code>checkpoint='no'</code> will > + participate. On output, this is the checkpoint state of each > + of the domain's disks. > + <dl> > + <dt><code>disk</code></dt> > + <dd>This sub-element describes the checkpoint properties of > + a specific disk with the following attributes: > + <dl> > + <dt><code>name</code></dt> > + <dd>A mandatory attribute which must match either > + the <code><target dev='name'/></code> or an > + unambiguous <code><source file='name'/></code> > + of one of > + the <a href="formatdomain.html#elementsDisks">disk > + devices</a> specified for the domain at the time of > + the checkpoint.</dd> > + <dt><code>checkpoint</code></dt> > + <dd>An optional attribute; possible values > + are <code>no</code> when the disk does not participate > + in this checkpoint; or <code>bitmap</code> if the disk > + will track all changes since the creation of this > + checkpoint via a bitmap.</dd> > + <dt><code>bitmap</code></dt> > + <dd>The attribute <code>bitmap</code> is only valid > + if <code>checkpoint='bitmap'</code>; it describes the > + name of the tracking bitmap (defaulting to the > + checkpoint name).</dd> > + <dt><code>size</code></dt> > + <dd>The attribute <code>size</code> is ignored on input; > + on output, it is only present if > + the <code>VIR_DOMAIN_CHECKPOINT_XML_SIZE</code> flag > + was used to perform a dynamic query of the estimated > + size in bytes of the changes made since the checkpoint > + was created.</dd> > + </dl> > + </dd> > + </dl> > + </dd> > + <dt><code>creationTime</code></dt> > + <dd>A readonly representation of the time this checkpoint was > + created. The time is specified in seconds since the Epoch, > + UTC (i.e. Unix time). > + </dd> > + <dt><code>parent</code></dt> > + <dd>An optional readonly representation of the parent of this > + checkpoint. If present, this element contains exactly one > + child element, <code>name</code>. This reads weird and does not seem to say much. > + </dd> > + <dt><code>current</code></dt> > + <dd>A readonly integer, 1 if this checkpoint is current (that > + is, actively tracking guest changes) or 0 if the checkpoint is > + inactive because a child checkpoint is now tracking changes. > + </dd> See below in the schema. The contents of this element are wrong. > + <dt><code>domain</code></dt> > + <dd>A readonly representation of the > + inactive <a href="formatdomain.html">domain configuration</a> > + at the time the checkpoint was created. This element may be > + omitted for output brevity by supplying > + the <code>VIR_DOMAIN_CHECKPOINT_XML_NO_DOMAIN</code> flag, but > + the resulting XML is no longer viable for use with > + the <code>VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE</code> flag > + of <code>virDomainCheckpointCreateXML()</code>. The domain > + will have security-sensitive information omitted unless the > + flag <code>VIR_DOMAIN_CHECKPOINT_XML_SECURE</code> is provided > + on a read-write connection. > + </dd> > + </dl> > + > + <h2><a id="example">Examples</a></h2> > + > + <p>Using this XML to create a checkpoint of just vda on a qemu > + domain with two disks and a prior checkpoint:</p> > + <pre> > +<domaincheckpoint> > + <description>Completion of updates after OS install</description> > + <disks> > + <disk name='vda' checkpoint='bitmap'/> > + <disk name='vdb' checkpoint='no'/> > + </disks> > +</domaincheckpoint></pre> > + > + <p>will result in XML similar to this from > + <code>virDomainCheckpointGetXMLDesc()</code>:</p> > + <pre> > +<domaincheckpoint> > + <name>1525889631</name> > + <description>Completion of updates after OS install</description> > + <parent> > + <name>1525111885</name> > + </parent> > + <creationTime>1525889631</creationTime> > + <disks> > + <disk name='vda' checkpoint='bitmap' bitmap='1525889631'/> > + <disk name='vdb' checkpoint='no'/> > + </disks> > + <domain type='qemu'> > + <name>fedora</name> > + <uuid>93a5c045-6457-2c09-e56c-927cdf34e178</uuid> > + <memory>1048576</memory> > + ... > + <devices> > + <disk type='file' device='disk'> > + <driver name='qemu' type='qcow2'/> > + <source file='/path/to/file1'/> > + <target dev='vda' bus='virtio'/> > + </disk> > + <disk type='file' device='disk' snapshot='external'> > + <driver name='qemu' type='raw'/> > + <source file='/path/to/file2'/> > + <target dev='vdb' bus='virtio'/> > + </disk> > + ... > + </devices> > + </domain> > +</domaincheckpoint></pre> > + > + <p>With that checkpoint created, the qcow2 image is now tracking > + all changes that occur in the image since the checkpoint via > + the persistent bitmap named <code>1525889631</code>. > + </p> > + </body> > +</html> > diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in > index 92cc566467..2bfb69cf49 100644 > --- a/docs/formatsnapshot.html.in > +++ b/docs/formatsnapshot.html.in > @@ -91,7 +91,9 @@ > sets that snapshot as current, and the prior current snapshot is > the parent of the new snapshot. Branches in the hierarchy can > be formed by reverting to a snapshot with a child, then creating > - another snapshot. > + another snapshot. For now, the creation of external snapshots > + when checkpoints exist is forbidden, although future work will > + make it possible to integrate these two concepts. > </p> > <p> > The top-level <code>domainsnapshot</code> element may contain I'd slightly prefer if the snapshot-interlock stuff is in a separate patch. [...] > diff --git a/docs/schemas/domaincheckpoint.rng b/docs/schemas/domaincheckpoint.rng > new file mode 100644 > index 0000000000..ceb5436aa3 > --- /dev/null > +++ b/docs/schemas/domaincheckpoint.rng > @@ -0,0 +1,95 @@ > +<?xml version="1.0"?> > +<!-- A Relax NG schema for the libvirt domain checkpoint properties XML format --> > +<grammar xmlns="http://relaxng.org/ns/structure/1.0"> > + <start> > + <ref name='domaincheckpoint'/> > + </start> > + > + <include href='domaincommon.rng'/> > + > + <define name='domaincheckpoint'> > + <element name='domaincheckpoint'> > + <interleave> > + <optional> > + <element name='name'> > + <text/> This should be something more sensible than '<text/>' we have plenty of types for names which exclude newlines and '/'. > + </element> > + </optional> > + <optional> > + <element name='description'> > + <text/> > + </element> > + </optional> > + <optional> > + <element name='creationTime'> > + <text/> This is supposed to be a number. > + </element> > + </optional> > + <optional> > + <element name='current'> > + <choice> > + <value>0</value> > + <value>1</value> This looks weird. I see 3 options to make it better: 1) put it as an attribute of 'domaincheckpoint' (and use 'yes'/'no' as values) 2) encode it as presence of the element 3) at least use 'yes'/'no' as values. > + </choice> > + </element> > + </optional> > + <optional> > + <element name='disks'> > + <oneOrMore> > + <ref name='diskcheckpoint'/> > + </oneOrMore> > + </element> > + </optional> > + <optional> > + <!-- Nested grammar ensures that any of our overrides of > + storagecommon/domaincommon defines do not conflict > + with any domain.rng overrides. --> > + <grammar> > + <include href='domain.rng'/> > + </grammar> > + </optional> > + <optional> > + <element name='parent'> > + <element name='name'> > + <text/> Again, this is a 'name' > + </element> > + </element> > + </optional> > + </interleave> > + </element> > + </define> > + > + <define name='diskcheckpoint'> > + <element name='disk'> > + <attribute name='name'> > + <choice> > + <ref name='diskTarget'/> > + <ref name='absFilePath'/> > + </choice> > + </attribute> > + <choice> > + <attribute name='checkpoint'> > + <value>no</value> > + </attribute> > + <group> > + <optional> > + <attribute name='checkpoint'> > + <value>bitmap</value> > + </attribute> > + </optional> > + <optional> > + <attribute name='bitmap'> > + <text/> This is also a 'name' > + </attribute> > + </optional> > + <optional> > + <attribute name='size'> > + <ref name="unsignedLong"/> > + </attribute> > + </optional> > + </group> > + </choice> > + </element> > + </define> > + > +</grammar> ACK if you sanitize the schema and avoid documenting internals in the public docs. I think that also the bitmap name field should be documented as internal. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 7/8/19 6:23 AM, Peter Krempa wrote:
> On Sat, Jul 06, 2019 at 22:56:01 -0500, Eric Blake wrote:
>> Prepare for new checkpoint APIs by describing the XML that will
>> represent a checkpoint. The checkpoint XML is modeled heavily after
>> virDomainSnapshotPtr. See the docs for more details.
>>
>> Add testsuite coverage for some minimal uses of the XML (bare minimum,
>> the sample from html, and a full dumpxml). Although use of the
>> REDEFINE flag will require the <domain> subelement to be present, it
>> is easier for most of the tests to provide counterpart output produced
>> with the NO_DOMAIN flag (particularly since synthesizing a valid
>> <domain> during testing is not trivial).
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>
> [...]
>
>> + <h2><a id="CheckpointAttributes">Checkpoint XML</a></h2>
>> +
>> + <p>
>> + One method of capturing domain disk backups is via the use of
>> + incremental backups. Right now, incremental backups are only
>> + supported for the QEMU hypervisor when using qcow2 disks at the
>> + active layer; if other disk formats are in use, capturing disk
>> + backups requires different libvirt APIs.
>
> Perhaps add a 'since' to note that all of this was added starting from
> 5.6.
Yes.
>
>> + </p>
>> + <p>
>> + Libvirt is able to facilitate incremental backups by tracking
>> + disk checkpoints, which are points in time against which it is
>> + easy to compute which portion of the disk has changed. Given a
>> + full backup (a backup created from the creation of the disk to a
>> + given point in time), coupled with the creation of a disk
>> + checkpoint at that time, and an incremental backup (a backup
>> + created from just the dirty portion of the disk between the
>> + first checkpoint and the second backup operation), it is
>> + possible to do an offline reconstruction of the state of the
>> + disk at the time of the second backup without having to copy as
>> + much data as a second full backup would require. Future API
>> + additions will make it possible to create checkpoints in
>> + conjunction with a backup
>> + via <code>virDomainBackupBegin()</code> or with an external
>> + snapshot via <code>virDomainSnapshotCreateXML2</code>; but for
>> + now, libvirt exposes enough support to create disk checkpoints
>> + independently from a backup operation
>> + via <code>virDomainCheckpointCreateXML()</code>.
>
> So it will not happen in this release? Otherwise I don't see a point in
> documenting it.
virDomainBackupBegin still has a fighting chance of making it into 5.6;
virDomainSnapshotCreateXML2 is further down the road. My most recent
post of 'backup-v9' for backup additions tweak this paragraph yet again
when virDomainBackupBegin is added:
https://www.redhat.com/archives/libvir-list/2019-July/msg00330.html
>> + <p>
>> + A checkpoint is marked current if the hypervisor is actively
>> + updating the checkpoint as guest operations modify the disk
>> + after the point where the checkpoint was created. When tracking
>> + multiple checkpoints, a hypervisor may choose to either have all
>> + relevant checkpoints be current (a guest write updates thea
>
> So to reiterate. By definition all checkpoint should be "current",
> because they must be actively updated to be able to track the difference
> from the point when they were created until now.
>
> The fact whether it includes update to a bitmap and which bitmaps are
> updated does not seem to be relevant for outside users.
>
> In fact we may even opt to change how this is done or perhaps convert
> one layout of bitmaps into a different one if we insist.
>
> Thus I'm saying it again. "current" property seems to be wrong in
> conjunction with checkpoints.
Updating this thread based on an IRC conversation (I'm not pasting the
entire conversation, but hitting the key points, so Peter may still want
to chime in):
There's two ways to do differential backups in qemu: either make all
checkpoints use an active bitmap (no parent relationship is needed, all
checkpoints would appear as roots), or change which bitmap is active on
a per-disk basis when a newer checkpoint touches that disk (checkpoints
need a parent relationship in order to reconstruct which bitmaps to
merge together when performing a differential backup). But where things
get interesting is if a user performs a checkpoint/incremental backup of
only a subset of the domain's disks.
So, consider a domain with three disks, A, B, and C, and where we have
two users interested in different incremental backup goals: user 1 only
cares about backups of disk A and B, user 2 only cares about backups of
disk B and C. Over time, we have user 1 create chk1.1, user 2 creates
chk2.1, user 1 creates chk1.2, user 2 creates chk2.2.
chk1.1 => A.chk1.1, B.chk1.1
chk2.1 => B.chk2.1, C.chk2.1
chk1.2 => A.chk1.2, B.chk1.2
chk2.2 => B.chk2.2, C.chk2.2
then what happens when user 1 wants to do what they think is an
incremental backup starting from chk1.2 or a differential backup
starting from chk1.1. If we always leave bitmaps active, then we don't
need to track any parent relationships; the incremental or differential
backup is trivial: use the single bitmap recorded in the checkpoint
being used as the starting point. However, disk B has a lot of overhead
with every guest write potentially updating 4 bitmaps.
Or, we can take the approach that Checkpoints are always created with
the parent object being the most-recent previously-created checkpoint
(this has to be tracked somewhere, but not necessarily exposed through
the XML nor through specialized API - even though the specialized API
was how the Snapshot was implemented), AND with the notion that every
disk has at most one active bitmap, but the active bitmap might not be
in the most-recent checkpoint. When a checkpoint is created, it's
parent is assigned to be the most-recent existing checkpoint, and for
every disk in the new checkpoint, any earlier checkpoint (possibly
further back than the parent) touching the same disk has that earlier
bitmap disabled.
Or using the above figure for reference, the act of creating chk1.2
touches only disks A and B, but sets chk2.1 as the immediate parent
(even though it covers a different set of disks). Furthermore, in
addition to creating new bitmaps for disk A and B, it disables A.chk1.1
(found in the grandparent checkpoint) and B.chk2.1 (in the parent, since
B.chk1.1 is already disabled). Then, after chk2.2 is created, and the
user requests chk1.2 as the point of reference for a backup operation,
the user is requesting an incremental backup of disk A (can use bitmap
A.chk1.2 in isolation) at the same time as a differential backup of
disk B (the merge of bitmaps B.chk.1.2 and B.chk2.2). Thus, both the
act of creating a checkpoint and the act of starting a backup operation
have to concern themselves with performing a per-disk search along the
checkpoint chain from the current checkpoint back through ancestors
(creation chases until the chain ends, backup chases until the
point-of-reference checkpoint). What's more, a checkpoint being current
is orthogonal to whether a bitmap is active, so we really never do have
more than one current checkpoint, and the documentation attempt here is
wrong (current is not about which bitmaps are active, but about which
checkpoint becomes the parent when a new checkpoint is added).
However, one observation we made on IRC was the following: when
snapshots were first created, we needed a way to recreate the libvirt
metadata, and the list APIs listed snapshots in an arbitrary order. That
meant that when using the REDEFINE flag, you needed to pass in a second
flag CURRENT to mark which snapshot being redefined was to become the
domain's current snapshot (and thus the implicit parent of any future
snapshot) - and as a result of that API, a side-effect was that you
could arbitrarily change the current snapshot ('virsh snapshot-current'
is rather gross, but shows this abuse). Also, snapshots have a revert
operation, which changes the current snapshot and makes it very easy to
have a current snapshot which is not a leaf node. On the other hand,
with checkpoints and our current interlock against mixing with snapshot
revert, the current node is always a leaf node, and if you perform your
listing of all checkpoints with the recently introduced TOPOLOGICAL
flag, then it is just as easy to state that when using REDEFINE in the
same order, the last checkpoint thus redefined becomes the current node.
It may still be possible to create multiple child checkpoints to a
single parent once we work out how to enable revert-to-snapshots
combined with checkpoints. And when that happens, we may need to add a
CHECKPOINT_CURRENT flag for use with REDEFINE at that time, but it does
not have to be today. In other words, for as long as our checkpoint
chain is linear and the current node is the (one and only) leaf, we
don't need any API for managing a current checkpoint.
The IRC conversation also pointed out that before I push things, I _do_
also need to interlock such that the existence of checkpoints prevents
blockpull/blockcommit, in case libvirt is started with an external
backing chain of bitmaps created outside of libvirt, even though libvirt
itself will refuse to create such a chain.
>
>> + associated tracking for each checkpoint), or to have a single
>> + checkpoint be current (older checkpoints stop tracking changes
>
> NACK, internal detail. Don't document it. Also I'll object to this later
> in detail.
>
>> + when a newer one is created, and describing all changes since an
>> + older checkpoint then involves libvirt automatically combining
>> + the changes from the chain of checkpoints from the current back
>> + to the checkpoint in question; the QEMU hypervisor uses this
>> + method). Checkpoints are maintained in a hierarchy to facilitate
>
> Nope. All of the above seem to be internal stuff.
So for my next spin of this patch, my goal is to eliminate as much
public API/XML related to a current checkpoint as possible, and see if I
can implement it completely internally by using the one-and-only leaf
node as the current checkpoint.
>> + </dd>
>> + <dt><code>parent</code></dt>
>> + <dd>An optional readonly representation of the parent of this
>> + checkpoint. If present, this element contains exactly one
>> + child element, <code>name</code>.
>
> This reads weird and does not seem to say much.
Copying from snapshots. I kept the same XML for consistency:
<domaincheckpoint>
<parent>
<name>name_of_parent</name>
</parent>
...
even though if I were designing it from scratch, I would prefer:
<domaincheckpoint>
<parent>name_of_parent</parent>
...
But maybe I can tweak the wording in that paragraph (and its counterpart
in the snapshot docs) to be easier to understand.
>
>> + </dd>
>> + <dt><code>current</code></dt>
>> + <dd>A readonly integer, 1 if this checkpoint is current (that
>> + is, actively tracking guest changes) or 0 if the checkpoint is
>> + inactive because a child checkpoint is now tracking changes.
>> + </dd>
>
> See below in the schema. The contents of this element are wrong.
If I can get by without any mention of current in the public XML
(perhaps by relying on the rule that as long as we have interlocks with
snapshots, we can't branch, and therefore there is at most one leaf
node, and simply define that the leaf node IS the current node), then
this goes away.
>> +++ b/docs/formatsnapshot.html.in
>> @@ -91,7 +91,9 @@
>> sets that snapshot as current, and the prior current snapshot is
>> the parent of the new snapshot. Branches in the hierarchy can
>> be formed by reverting to a snapshot with a child, then creating
>> - another snapshot.
>> + another snapshot. For now, the creation of external snapshots
>> + when checkpoints exist is forbidden, although future work will
>> + make it possible to integrate these two concepts.
>> </p>
>> <p>
>> The top-level <code>domainsnapshot</code> element may contain
>
> I'd slightly prefer if the snapshot-interlock stuff is in a separate
> patch.
Okay, I can split.
>> + <define name='domaincheckpoint'>
>> + <element name='domaincheckpoint'>
>> + <interleave>
>> + <optional>
>> + <element name='name'>
>> + <text/>
>
> This should be something more sensible than '<text/>' we have plenty of
> types for names which exclude newlines and '/'.
Copying from <domainsnapshot> but yes, that's not a good excuse, and
it's always easier to relax later than it is to restrict later. And
since we DO store the checkpoint name as a file (at least for qemu),
restricting the RNG up front makes it easier to prevent the use of '../'
as a name, since we're strictly validating RNG compliance.
>
>> + </element>
>> + </optional>
>> + <optional>
>> + <element name='description'>
>> + <text/>
>> + </element>
>> + </optional>
>> + <optional>
>> + <element name='creationTime'>
>> + <text/>
>
> This is supposed to be a number.
Copying from <domainsnapshot>, but yes this can be improved.
>
>> + </element>
>> + </optional>
>> + <optional>
>> + <element name='current'>
>> + <choice>
>> + <value>0</value>
>> + <value>1</value>
>
> This looks weird. I see 3 options to make it better:
> 1) put it as an attribute of 'domaincheckpoint' (and use 'yes'/'no' as
> values)
> 2) encode it as presence of the element
> 3) at least use 'yes'/'no' as values.
or 4) don't have it, after all (at least, not until we work out whether
integration with snapshot revert will require it).
>
> ACK if you sanitize the schema and avoid documenting internals in the
> public docs. I think that also the bitmap name field should be
> documented as internal.
Makes sense - the field will still be in the RNG schema (for two
reasons: REDEFINE must document everything it will parse, and when not
using REDEFINE, it's still often easier to copy-and-paste an existing
checkpoint XML to form a new checkpoint XML, without having to worry
about scrubbing out the readonly fields that will be ignored in the new
checkpoint), but I can document it as ignored during creation and
read-only in output for which bitmap got created.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.