[libvirt] [PATCH v4 06/20] backup: Document new XML for backups

John Ferlan jferlan at redhat.com
Sat Feb 9 15:56:11 UTC 2019



On 2/6/19 2:18 PM, Eric Blake wrote:
> Prepare for new checkpoint and backup APIs by describing the XML
> that will represent a checkpoint and backup.  The checkpoint XML
> is modeled heavily after virDomainSnapshotPtr, since both represent
> a point in time of the guest (however, a snapshot exists with the
> intent to roll back to that point, while a checkpoint exists to
> facilitate later incremental backups). Meanwhile, the backup XML
> has enough information to represent both push model (the hypervisor
> writes the backup file to a location of the user's choice) and the
> pull model (the hypervisor needs local temporary storage, and also
> creates an NBD server that the user can use to read the backup via
> a third-party client)..  But while a snapshot exists with the

s/../.

> intent of rolling back to that state, a checkpoint instead makes it
> possible to create an incremental backup at a later time.
> 

I think most of the description beyond the first sentence or so is
covered in the docs and probably doesn't need to be in the commit.
Perhaps just " Prepare for new checkpoint and backup APIs by describing
the XML that will represent a checkpoint and backup.  The checkpoint XML
is modeled heavily after virDomainSnapshotPtr, while the backup XML is
provides newer capabilities for both push and pull models.


> Add testsuite coverage for some minimal uses of both XML.
> 
> Ultimately, I'd love for push model backups to target a network
> driver rather than just a local file or block device; but doing
> that got hairy (while <domain> uses <source> as the description
> of a host or network resource, I picked <target> as the description
> of a push model backup target [defaults to qcow2 but can also be
> raw or any other format], and <scratch> as the description
> of a pull model backup scratch space [must be qcow2]). The ideal
> refactoring would be a way to parameterize RNG to accept
> <disk type='FOO'>...</disk> so that the name of the subelement
> can be <source> for domain, or <target> or <scratch> as needed for
> backups. Future patches may improve this area of code.

Hmmm... Should this paragraph be in the commit message or under the ---?

BTW: Only after reading through all this does this partially make sense.

> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> 
> ---
> v2: apply (some) wording changes from review
> ---
>  docs/docs.html.in                            |   3 +-
>  docs/domainstatecapture.html.in              |   4 +-
>  docs/format.html.in                          |   1 +
>  docs/formatcheckpoint.html.in                | 291 +++++++++++++++++++
>  docs/index.html.in                           |   3 +-
>  docs/schemas/domainbackup.rng                | 185 ++++++++++++
>  docs/schemas/domaincheckpoint.rng            |  94 ++++++
>  libvirt.spec.in                              |   2 +
>  mingw-libvirt.spec.in                        |   4 +
>  tests/Makefile.am                            |   6 +-
>  tests/domainbackupxml2xmlin/backup-pull.xml  |   9 +
>  tests/domainbackupxml2xmlin/backup-push.xml  |   9 +
>  tests/domainbackupxml2xmlin/empty.xml        |   1 +
>  tests/domainbackupxml2xmlout/backup-pull.xml |   9 +
>  tests/domainbackupxml2xmlout/backup-push.xml |   9 +
>  tests/domainbackupxml2xmlout/empty.xml       |   7 +
>  tests/domaincheckpointxml2xmlin/empty.xml    |   1 +
>  tests/domaincheckpointxml2xmlin/sample.xml   |   7 +
>  tests/domaincheckpointxml2xmlout/empty.xml   |  10 +
>  tests/domaincheckpointxml2xmlout/sample.xml  |  16 +
>  tests/virschematest.c                        |   4 +
>  21 files changed, 670 insertions(+), 5 deletions(-)
>  create mode 100644 docs/formatcheckpoint.html.in
>  create mode 100644 docs/schemas/domainbackup.rng
>  create mode 100644 docs/schemas/domaincheckpoint.rng
>  create mode 100644 tests/domainbackupxml2xmlin/backup-pull.xml
>  create mode 100644 tests/domainbackupxml2xmlin/backup-push.xml
>  create mode 100644 tests/domainbackupxml2xmlin/empty.xml
>  create mode 100644 tests/domainbackupxml2xmlout/backup-pull.xml
>  create mode 100644 tests/domainbackupxml2xmlout/backup-push.xml
>  create mode 100644 tests/domainbackupxml2xmlout/empty.xml
>  create mode 100644 tests/domaincheckpointxml2xmlin/empty.xml
>  create mode 100644 tests/domaincheckpointxml2xmlin/sample.xml
>  create mode 100644 tests/domaincheckpointxml2xmlout/empty.xml
>  create mode 100644 tests/domaincheckpointxml2xmlout/sample.xml
> 

Lots of detail and files... I know they're related, but if there's a way
to split it may be nicer for review processing ;-)

> diff --git a/docs/docs.html.in b/docs/docs.html.in
> index 4c46b74980..4914e7dbed 100644
> --- a/docs/docs.html.in
> +++ b/docs/docs.html.in
> @@ -79,7 +79,8 @@
>            <a href="formatdomaincaps.html">domain 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">backups and 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/domainstatecapture.html.in b/docs/domainstatecapture.html.in
> index f7f2fe0b98..9b890b4c0c 100644
> --- a/docs/domainstatecapture.html.in
> +++ b/docs/domainstatecapture.html.in
> @@ -259,9 +259,9 @@
>          a checkpoint as a side-effect of starting a new incremental
>          backup with <code>virDomainBackupBegin()</code>, since a
>          second incremental backup is most useful when using the
> -        checkpoint created during the first.  <!--See also
> +        checkpoint created during the first.  See also
>          the <a href="formatcheckpoint.html">XML details</a> used with
> -        this command.--></dd>
> +        this command.</dd>
> 
>        <dt>virDomainBackupBegin(), virDomainBackupEnd()</dt>
>        <dd>This API wraps approaches for capturing the state of disks
> diff --git a/docs/format.html.in b/docs/format.html.in
> index 22b23e3fc7..8c4e15e079 100644
> --- a/docs/format.html.in
> +++ b/docs/format.html.in
> @@ -24,6 +24,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">Backups and 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..6d66bd0511
> --- /dev/null
> +++ b/docs/formatcheckpoint.html.in
> @@ -0,0 +1,291 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<!DOCTYPE html>
> +<html xmlns="http://www.w3.org/1999/xhtml">
> +  <body>
> +    <h1>Checkpoint and Backup 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

QEMU

> +      active layer; if other disk formats are in use, capturing disk
> +      backups requires different libvirt APIs
> +      (see <a href="domainstatecapture.html">domain state capture</a>
> +      for a comparison between 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.  Most disk
> +      checkpoints are created in concert with a backup

s/concert/conjunction

(although I understood concert, I think conjunction is more apropos)

> +      via <code>virDomainBackupBegin()</code>; however, libvirt also
> +      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.
> +    </p>
> +    <p>
> +      Checkpoints are maintained in a hierarchy.  A domain can have a
> +      current checkpoint, which is the most recent checkpoint compared to
> +      the current state of the domain (although a domain might have
> +      checkpoints without a current checkpoint, if checkpoints have been
> +      deleted in the meantime).  Creating or reverting to a checkpoint
> +      sets that checkpoint as current, and the prior current checkpoint is

s/,//

> +      the parent of the new checkpoint.  Branches in the hierarchy can
> +      be formed by reverting to a checkpoint with a child, then creating
> +      another checkpoint.
> +    </p>
> +    <p>
> +      The top-level <code>domaincheckpoint</code> element may contain
> +      the following elements:
> +    </p>
> +    <dl>
> +      <dt><code>name</code></dt>
> +      <dd>The name for this checkpoint.  If the name is specified when

The optional name

> +        initially creating the checkpoint, then the checkpoint will have
> +        that particular name.  If the name is omitted when initially
> +        creating the checkpoint, then libvirt will make up a name for
> +        the checkpoint, based on the time when it was created.

I would think the description of what happens when a name is specified
would be obvious.

> +      </dd>
> +      <dt><code>description</code></dt>
> +      <dd>A human-readable description of the checkpoint.  If the

An optional human-...

> +        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

s/domain (/domain. In/

QEMU

> +        disks, this element may be needed on input for excluding guest
> +        disks that are not in qcow2 format); if the entire element was

s/); if/. If/

> +        omitted on input, then all disks participate in the
> +        checkpoint, but if individual disks were omitted from the
> +        element, they will not be part of the checkpoint.  On output,
> +        this is fully populated to show the state of each disk in the

each disk participating in the checkpoint ?

Or perhaps "On output, this is the state of the domain's list of disks
relative to how the checkpoint is used for each." [  - or something like
that]

> +        checkpoint.  This element has a list of <code>disk</code>
> +        sub-elements, describing anywhere from one to all of the disks
> +        associated with the domain.
> +        <dl>
> +          <dt><code>disk</code></dt>
> +          <dd>This sub-element describes the checkpoint properties of
> +            a specific disk.  The attribute <code>name</code> is
> +            mandatory, and 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.  The attribute <code>checkpoint</code> is
> +            optional on input; 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, in
> +            which case another attribute <code>bitmap</code> will be
> +            the name of the tracking bitmap (defaulting to the
> +            checkpoint name).  On output, an additional
> +            attribute <code>size</code> may be 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.

This is tough to read as one fairly long paragraph especially when it
comes to picking out the attributes, consider:

    <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>The attribute <code>name</code> is mandatory and 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>The attribute <code>checkpoint</code> is optional on
          input. 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>Optional attribute to define the name of the tracking
          bitmap (defaulting to the checkpoint name) when the disk
          is participating in the checkpoint processing.</dd>
        <dt><code>size</code></dt>
        <dd>On output, this attribute may be 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>The time this checkpoint was created.  The time is specified

A readonly representation of the time this checkpoint was created.

> +        in seconds since the Epoch, UTC (i.e. Unix time).  Readonly.
> +      </dd>
> +      <dt><code>parent</code></dt>
> +      <dd>The parent of this checkpoint.  If present, this element

An optional readonly representation of the parent of this checkpoint.

> +        contains exactly one child element, name.  This specifies the

s/name/<code>name</code>

> +        name of the parent checkpoint of this one, and is used to
> +        represent trees of checkpoints.  Readonly.
> +      </dd>
> +      <dt><code>domain</code></dt>
> +      <dd>The inactive <a href="formatdomain.html">domain

A readonly representation of the inactive ...

> +        configuration</a> at the time the checkpoint was created.
> +        Readonly.
> +      </dd>

And then just remove the Readonly single word.

> +    </dl>
> +
> +    <h2><a id="BackupAttributes">Backup XML</a></h2>
> +
> +    <p>
> +      Creating a backup, whether full or incremental, is done
> +      via <code>virDomainBackupBegin()</code>, which takes an XML
> +      description of the actions to perform.  There are two general
> +      modes for backups: a push mode (where the hypervisor writes out
> +      the data to the destination file, which may be local or remote),
> +      and a pull mode (where the hypervisor creates an NBD server that
> +      a third-party client can then read as needed, and which requires
> +      the use of temporary storage, typically local, until the backup
> +      is complete).
> +    </p>
> +    <p>
> +      The instructions for beginning a backup job are provided as
> +      attributes and elements of the
> +      top-level <code>domainbackup</code> element.  This element
> +      includes an optional attribute <code>mode</code> which can be
> +      either "push" or "pull" (default push).  Where elements are
> +      optional on creation, <code>virDomainBackupGetXMLDesc()</code>

s/,/. The/

> +      can be used to see the actual values selected (for example,
> +      learning which port the NBD server is using in the pull model,

s/model,/model/

> +      or what file names libvirt generated when none were supplied).
> +      The following child elements are supported:
> +    </p>
> +    <dl>
> +      <dt><code>incremental</code></dt>
> +      <dd>Optional. If this element is present, it must name an

An optional element to describe the name of an existing checkpoint...

[one would hope we wouldn't have to give a link to the above named
checkpoints, but I suppose it would be possible - your call on that]

> +        existing checkpoint of the domain, which will be used to make
> +        this backup an incremental one (in the push model, only

s/one (in the/one. In the/

> +        changes since the checkpoint are written to the destination;

s/;/./

> +        in the pull model, the NBD server uses the

s/in/In/

> +        NBD_OPT_SET_META_CONTEXT extension to advertise to the client
> +        which portions of the export contain changes since the
> +        checkpoint).  If omitted, a full backup is performed.

s/)./.

> +      </dd>
> +      <dt><code>server</code></dt>
> +      <dd>Present only for a pull mode backup.  Contains the same
> +        attributes as the <code>protocol</code> element of a disk

[could provide a link to the domain disk section if you feel it's
appropriate or necessary]

> +        attached via NBD in the domain (such as transport, socket,
> +        name, port, or tls), necessary to set up an NBD server that
> +        exposes the content of each disk at the time the backup
> +        started.

s/started/is started/

> +      </dd>
> +      <dt><code>disks</code></dt>
> +      <dd>This is an optional listing of instructions for disks

An optional listing

> +        participating in the backup (if omitted, all disks
> +        participate, and libvirt attempts to generate filenames by

s/, and/ and/

> +        appending the current timestamp as a suffix). When provided on
> +        input, disks omitted from the list do not participate in the
> +        backup.  On output, the list is present but contains only the
> +        disks participating in the backup job.  This element has a

s/ job././

[job is an implementation detail]

> +        list of <code>disk</code> sub-elements, describing anywhere
> +        from one to all of the disks associated with the domain.
> +        <dl>
> +          <dt><code>disk</code></dt>
> +          <dd>This sub-element describes the backup properties of
> +            a specific disk.  The attribute <code>name</code> is
> +            mandatory, and 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.  The optional attribute <code>type</code> can
> +            be <code>file</code>, <code>block</code>,
> +            or <code>network</code>, similar to a disk declaration for
> +            a domain, controls what additional sub-elements are needed
> +            to describe the destination (such as <code>protocol</code>
> +            for a network destination).  In push mode backups, the
> +            primary sub-element is <code>target</code>; in pull mode,
> +            the primary sub-element is <code>scratch</code>; but
> +            either way, the primary sub-element describes the file
> +            name to be used during the backup operation, similar to
> +            the <code>source</code> sub-element of a domain disk. In
> +            push mode, an optional sub-element <code>driver</code> can
> +            also be used, with an attribute <code>type</code> to
> +            specify a destination format different from
> +            qcow2. Additionally, if a push backup is not
> +            incremental, <code>target</code> may contain an optional
> +            attribute <code>shallow="on"</code> so that the
> +            destination file copies only the top-most source file in a
> +            backing chain, rather than collapsing the entire chain
> +            into the copy.

This one's harder to read than the other one...Consider:

    <dt><code>disk</code></dt>
    <dd>This sub-element describes the backup properties of
      a specific disk with the following attributes:
      <dl>
        <dt><code>name</code></dt>
        <dd>A mandatory attribute and 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>type</code></dt>
        <dd>An optional attribute to describe the type of disk.
          Valid values can be <code>file</code>, <code>block</code>,
          or <code>network</code>. Similar to a disk declaration for
          a domain, controls what additional sub-elements are needed
          to describe the destination (such as <code>protocol</code>
          for a network destination).</dd>
        <dt><code>target</code></dt>
        <dd>For push mode backups, this is the primary sub-element
          to describe the file name to be used during the backup
          operation similar to the <code>source</code> sub-element
          of a domain disk. An optional sub-element <code>driver</code>
          can also be used, with an attribute <code>type</code> to
          specify a destination format different from qcow2.
          Additionally, if the push backup is not incremental,
          <code>target</code> may contain an optional attribute
          <code>shallow="on"</code> so that the destination file
          copies only the top-most source file in a backing chain,
          rather than collapsing the entire chain into the copy.</dd>
        <dt><code>scratch</code></dt>
        <dd>For pull mode backups, this is the primary sub-element
          to describe the file name to be used during the backup
          operation similar to the <code>source</code> sub-element
          of a domain disk.</dd>
      </dl>
    </dd>

> +          </dd>
> +        </dl>
> +      </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>
> +  <creationTime>1525889631</creationTime>
> +  <parent>
> +    <name>1525111885</name>
> +  </parent>
> +  <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>.  Now, we
> +      can make a subsequent call
> +      to <code>virDomainBackupBegin()</code> to perform an incremental
> +      backup of just this data, using the following XML to start a
> +      pull model NBD export of the vda disk:
> +    </p>
> +    <pre>
> +<domainbackup mode="pull">
> +  <incremental>1525889631</incremental>
> +  <server transport="unix" socket="/path/to/server"/>
> +  <disks/>
> +    <disk name='vda' type='file'>
> +      <scratch file='/path/to/file1.scratch'/>
> +    </disk>
> +  </disks/>
> +</domainbackup>
> +    </pre>
> +  </body>

Perhaps an example of mode="push" would be useful as that <target>
sub-element has a lot of "nuances" (kindly said ;-))

> +</html>
> diff --git a/docs/index.html.in b/docs/index.html.in
> index 1f9f448399..6c5d3a6dc3 100644
> --- a/docs/index.html.in
> +++ b/docs/index.html.in
> @@ -68,7 +68,8 @@
>            <a href="formatdomaincaps.html">domain 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">backups and 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/domainbackup.rng b/docs/schemas/domainbackup.rng
> new file mode 100644
> index 0000000000..edc68a37cf
> --- /dev/null
> +++ b/docs/schemas/domainbackup.rng
> @@ -0,0 +1,185 @@
> +<?xml version="1.0"?>
> +<!-- A Relax NG schema for the libvirt domain backup properties XML format -->
> +<grammar xmlns="http://relaxng.org/ns/structure/1.0">
> +  <start>
> +    <ref name='domainbackup'/>
> +  </start>
> +
> +  <include href='domaincommon.rng'/>
> +
> +  <define name='domainbackup'>
> +    <element name='domainbackup'>
> +      <optional>
> +        <attribute name='id'>
> +          <ref name="unsignedInt"/>
> +        </attribute>
> +      </optional>

The 'id' is not described in docs nor do I see it in any of your output.
Remnant or future?

> +      <interleave>
> +        <optional>
> +          <element name='incremental'>
> +            <text/>
> +          </element>
> +        </optional>
> +        <choice>
> +          <group>
> +            <optional>
> +              <attribute name='mode'>
> +                <value>push</value>
> +              </attribute>
> +            </optional>
> +            <ref name='backupDisksPush'/>
> +          </group>
> +          <group>
> +            <attribute name='mode'>
> +              <value>pull</value>
> +            </attribute>
> +            <interleave>
> +              <element name='server'>
> +                <choice>
> +                  <group>
> +                    <optional>
> +                      <attribute name='transport'>
> +                        <value>tcp</value>
> +                      </attribute>
> +                    </optional>
> +                    <attribute name='name'>
> +                      <choice>
> +                        <ref name='dnsName'/>
> +                        <ref name='ipAddr'/>
> +                      </choice>
> +                    </attribute>
> +                    <optional>
> +                      <attribute name='port'>
> +                        <ref name='unsignedInt'/>
> +                      </attribute>
> +                    </optional>
> +                    <!-- add tls? -->

More work?  leaving a golden egg like this keeps me interested ;-)

> +                  </group>
> +                  <group>
> +                    <attribute name='transport'>
> +                      <value>unix</value>
> +                    </attribute>
> +                    <attribute name='socket'>
> +                      <ref name='absFilePath'/>
> +                    </attribute>
> +                  </group>
> +                </choice>
> +              </element>

Could something be done to create some "common" include file that would
allow for sharing w/ diskSourceNetworkHost so that when/if updating
domaincommon someone doesn't forget or neglect to update the
domainbackup?  One difference I see would be rdma transport which would
need to be handled specially in domainbackup XML processing since I
assume it wouldn't be supported.

> +              <ref name='backupDisksPull'/>
> +            </interleave>
> +          </group>
> +        </choice>
> +      </interleave>
> +    </element>
> +  </define>
> +
> +  <define name='backupPushDriver'>
> +    <optional>
> +      <element name='driver'>
> +        <attribute name='type'>
> +          <ref name='storageFormat'/>
> +        </attribute>
> +      </element>
> +    </optional>
> +  </define>
> +
> +  <define name='backupDisksPush'>
> +    <optional>
> +      <element name='disks'>
> +        <oneOrMore>
> +          <element name='disk'>
> +            <attribute name='name'>
> +              <choice>
> +                <ref name='diskTarget'/>
> +                <ref name='absFilePath'/>
> +              </choice>
> +            </attribute>
> +            <choice>
> +              <!-- FIXME allow push to a network location, by
> +                   refactoring 'diskSource' to take element name by a
> +                   per-grammar ref -->

Is there still work to be done here!

> +              <group>
> +                <optional>
> +                  <attribute name='type'>
> +                    <value>file</value>
> +                  </attribute>
> +                </optional>
> +                <interleave>
> +                  <optional>
> +                    <element name='target'>
> +                      <attribute name='file'>
> +                        <ref name='absFilePath'/>
> +                      </attribute>
> +                    </element>
> +                  </optional>
> +                  <ref name='backupPushDriver'/>
> +                </interleave>
> +              </group>
> +              <group>
> +                <attribute name='type'>
> +                  <value>disk</value>

So the docs describe attribute type having "file", "block", or "network"
and this is different. Things should match I would think.

> +                </attribute>
> +                <interleave>
> +                  <optional>
> +                    <element name='target'>
> +                      <attribute name='dev'>
> +                        <ref name='absFilePath'/>
> +                      </attribute>

Missing optional attribute "shallow" w/ OnOff as describe in docs.

> +                    </element>
> +                  </optional>
> +                  <ref name='backupPushDriver'/>
> +                </interleave>
> +              </group>
> +            </choice>
> +          </element>
> +        </oneOrMore>
> +      </element>
> +    </optional>
> +  </define>
> +
> +  <define name='backupDisksPull'>
> +    <optional>
> +      <element name='disks'>
> +        <oneOrMore>
> +          <element name='disk'>
> +            <attribute name='name'>
> +              <choice>
> +                <ref name='diskTarget'/>
> +                <ref name='absFilePath'/>
> +              </choice>
> +            </attribute>
> +            <choice>
> +              <group>
> +                <optional>
> +                  <attribute name='type'>
> +                    <value>file</value>
> +                  </attribute>
> +                </optional>
> +                <optional>
> +                  <element name='scratch'>
> +                    <attribute name='file'>
> +                      <ref name='absFilePath'/>
> +                    </attribute>
> +                  </element>
> +                </optional>
> +              </group>
> +              <group>
> +                <attribute name='type'>
> +                  <value>disk</value>

Same w/r/t to valid 'type' values.

> +                </attribute>
> +                <optional>
> +                  <element name='scratch'>
> +                    <attribute name='dev'>
> +                      <ref name='absFilePath'/>
> +                    </attribute>
> +                  </element>
> +                </optional>
> +              </group>
> +            </choice>
> +          </element>
> +        </oneOrMore>
> +      </element>
> +    </optional>
> +  </define>
> +
> +</grammar>
> diff --git a/docs/schemas/domaincheckpoint.rng b/docs/schemas/domaincheckpoint.rng
> new file mode 100644
> index 0000000000..d8dfda9f1c
> --- /dev/null
> +++ b/docs/schemas/domaincheckpoint.rng
> @@ -0,0 +1,94 @@
> +<?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='disks'>
> +            <oneOrMore>
> +              <ref name='diskcheckpoint'/>
> +            </oneOrMore>
> +          </element>
> +        </optional>
> +        <optional>
> +          <choice>
> +            <element name='domain'>
> +              <element name='uuid'>
> +                <ref name="UUID"/>
> +              </element>
> +            </element>
> +            <!-- 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>
> +          </choice>
> +        </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"/>

I haven't looked ahead to conf processing, but I assume this is the same
type as whatever this gets read into.  Whenever I see size in reference
to disks I'm thinking ULL's.

> +            </attribute>
> +          </optional>
> +        </group>
> +      </choice>
> +    </element>
> +  </define>
> +
> +</grammar>
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index c0e538d92d..2e9213510d 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -1813,7 +1813,9 @@ exit 0
>  %{_datadir}/libvirt/schemas/capability.rng
>  %{_datadir}/libvirt/schemas/cputypes.rng
>  %{_datadir}/libvirt/schemas/domain.rng
> +%{_datadir}/libvirt/schemas/domainbackup.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 249abb8475..a7b697d7bd 100644
> --- a/mingw-libvirt.spec.in
> +++ b/mingw-libvirt.spec.in
> @@ -239,7 +239,9 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh
>  %{mingw32_datadir}/libvirt/schemas/capability.rng
>  %{mingw32_datadir}/libvirt/schemas/cputypes.rng
>  %{mingw32_datadir}/libvirt/schemas/domain.rng
> +%{mingw32_datadir}/libvirt/schemas/domainbackup.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
> @@ -326,7 +328,9 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh
>  %{mingw64_datadir}/libvirt/schemas/capability.rng
>  %{mingw64_datadir}/libvirt/schemas/cputypes.rng
>  %{mingw64_datadir}/libvirt/schemas/domain.rng
> +%{mingw64_datadir}/libvirt/schemas/domainbackup.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 bdf7154fd5..9b835fa369 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -1,6 +1,6 @@
>  ## Process this file with automake to produce Makefile.in
> 
> -## Copyright (C) 2005-2015 Red Hat, Inc.
> +## Copyright (C) 2005-2018 Red Hat, Inc.
>  ##
>  ## This library is free software; you can redistribute it and/or
>  ## modify it under the terms of the GNU Lesser General Public
> @@ -92,7 +92,11 @@ EXTRA_DIST = \
>  	capabilityschemadata \
>  	commanddata \
>  	cputestdata \
> +	domainbackupxml2xmlin \
> +	domainbackupxml2xmlout \
>  	domaincapsschemadata \
> +	domaincheckpointxml2xmlin \
> +	domaincheckpointxml2xmlout \
>  	domainconfdata \
>  	domainschemadata \
>  	domainsnapshotxml2xmlin \
> diff --git a/tests/domainbackupxml2xmlin/backup-pull.xml b/tests/domainbackupxml2xmlin/backup-pull.xml
> new file mode 100644
> index 0000000000..2ce5cd6711
> --- /dev/null
> +++ b/tests/domainbackupxml2xmlin/backup-pull.xml
> @@ -0,0 +1,9 @@
> +<domainbackup mode="pull">
> +  <incremental>1525889631</incremental>
> +  <server transport='tcp' name='localhost' port='10809'/>
> +  <disks>
> +    <disk name='vda' type='file'>
> +      <scratch file='/path/to/file'/>
> +    </disk>
> +  </disks>
> +</domainbackup>
> diff --git a/tests/domainbackupxml2xmlin/backup-push.xml b/tests/domainbackupxml2xmlin/backup-push.xml
> new file mode 100644
> index 0000000000..1b7d3061fd
> --- /dev/null
> +++ b/tests/domainbackupxml2xmlin/backup-push.xml
> @@ -0,0 +1,9 @@
> +<domainbackup mode="push">
> +  <incremental>1525889631</incremental>
> +  <disks>
> +    <disk name='vda' type='file'>
> +      <driver type='raw'/>
> +      <target file='/path/to/file'/>
> +    </disk>
> +  </disks>
> +</domainbackup>
> diff --git a/tests/domainbackupxml2xmlin/empty.xml b/tests/domainbackupxml2xmlin/empty.xml
> new file mode 100644
> index 0000000000..7ed511f97b
> --- /dev/null
> +++ b/tests/domainbackupxml2xmlin/empty.xml
> @@ -0,0 +1 @@
> +<domainbackup/>

Should add a 'block' and 'network' example too for completeness - of
course that depends on the matching between docs and rng.

Should add target w/ shallow=on for completeness.

Yes, I recall the commit message indicating essentially minimal.

> diff --git a/tests/domainbackupoml2xmlout/backup-pull.xml b/tests/domainbackupxml2xmlout/backup-pull.xml
> new file mode 100644
> index 0000000000..2ce5cd6711
> --- /dev/null
> +++ b/tests/domainbackupxml2xmlout/backup-pull.xml
> @@ -0,0 +1,9 @@
> +<domainbackup mode="pull">
> +  <incremental>1525889631</incremental>
> +  <server transport='tcp' name='localhost' port='10809'/>
> +  <disks>
> +    <disk name='vda' type='file'>
> +      <scratch file='/path/to/file'/>
> +    </disk>
> +  </disks>
> +</domainbackup>
> diff --git a/tests/domainbackupxml2xmlout/backup-push.xml b/tests/domainbackupxml2xmlout/backup-push.xml
> new file mode 100644
> index 0000000000..1b7d3061fd
> --- /dev/null
> +++ b/tests/domainbackupxml2xmlout/backup-push.xml
> @@ -0,0 +1,9 @@
> +<domainbackup mode="push">
> +  <incremental>1525889631</incremental>
> +  <disks>
> +    <disk name='vda' type='file'>
> +      <driver type='raw'/>
> +      <target file='/path/to/file'/>
> +    </disk>
> +  </disks>
> +</domainbackup>
> diff --git a/tests/domainbackupxml2xmlout/empty.xml b/tests/domainbackupxml2xmlout/empty.xml
> new file mode 100644
> index 0000000000..13600fbb1c
> --- /dev/null
> +++ b/tests/domainbackupxml2xmlout/empty.xml
> @@ -0,0 +1,7 @@
> +<domainbackup mode="push">
> +  <disks>
> +    <disk name="vda" type="file">
> +      <target file="/path/to/file1.copy"/>
> +    </disk>
> +  </disks>
> +</domainbackup>
> diff --git a/tests/domaincheckpointxml2xmlin/empty.xml b/tests/domaincheckpointxml2xmlin/empty.xml
> new file mode 100644
> index 0000000000..dc36449142
> --- /dev/null
> +++ b/tests/domaincheckpointxml2xmlin/empty.xml
> @@ -0,0 +1 @@
> +<domaincheckpoint/>
> diff --git a/tests/domaincheckpointxml2xmlin/sample.xml b/tests/domaincheckpointxml2xmlin/sample.xml
> new file mode 100644
> index 0000000000..70ed964e1e
> --- /dev/null
> +++ b/tests/domaincheckpointxml2xmlin/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/domaincheckpointxml2xmlout/empty.xml b/tests/domaincheckpointxml2xmlout/empty.xml
> new file mode 100644
> index 0000000000..a26c7caab0
> --- /dev/null
> +++ b/tests/domaincheckpointxml2xmlout/empty.xml
> @@ -0,0 +1,10 @@
> +<domaincheckpoint>
> +  <name>1525889631</name>
> +  <creationTime>1525889631</creationTime>
> +  <disks>
> +    <disk name='vda' checkpoint='bitmap' bitmap='1525889631'/>
> +  </disks>
> +  <domain>
> +    <uuid>9d37b878-a7cc-9f9a-b78f-49b3abad25a8</uuid>

This looks strange without the name... and the domain type like the
sample.xml output has

> +  </domain>
> +</domaincheckpoint>
> diff --git a/tests/domaincheckpointxml2xmlout/sample.xml b/tests/domaincheckpointxml2xmlout/sample.xml
> new file mode 100644
> index 0000000000..559b29c8c1
> --- /dev/null
> +++ b/tests/domaincheckpointxml2xmlout/sample.xml
> @@ -0,0 +1,16 @@
> +<domaincheckpoint>
> +  <name>1525889631</name>
> +  <description>Completion of updates after OS install</description>
> +  <creationTime>1525889631</creationTime>
> +  <parent>
> +    <name>1525111885</name>
> +  </parent>
> +  <disks>
> +    <disk name='vda' checkpoint='bitmap' bitmap='1525889631'/>

Maybe this one should show the size attribute too...

I need to stop here for the day, but will continue...

John

> +    <disk name='vdb' checkpoint='no'/>
> +  </disks>
> +  <domain type='qemu'>
> +    <name>fedora</name>
> +    <uuid>93a5c045-6457-2c09-e56c-927cdf34e178</uuid>
> +  </domain>
> +</domaincheckpoint>
> diff --git a/tests/virschematest.c b/tests/virschematest.c
> index d1bcdeac9c..c3b41d5bbc 100644
> --- a/tests/virschematest.c
> +++ b/tests/virschematest.c
> @@ -221,7 +221,11 @@ mymain(void)
>                  "lxcxml2xmloutdata", "bhyvexml2argvdata", "genericxml2xmlindata",
>                  "genericxml2xmloutdata", "xlconfigdata", "libxlxml2domconfigdata",
>                  "qemuhotplugtestdomains");
> +    DO_TEST_DIR("domainbackup.rng", "domainbackupxml2xmlin",
> +                "domainbackupxml2xmlout");
>      DO_TEST_DIR("domaincaps.rng", "domaincapsschemadata");
> +    DO_TEST_DIR("domaincheckpoint.rng", "domaincheckpointxml2xmlin",
> +                "domaincheckpointxml2xmlout");
>      DO_TEST_DIR("domainsnapshot.rng", "domainsnapshotxml2xmlin",
>                  "domainsnapshotxml2xmlout");
>      DO_TEST_DIR("interface.rng", "interfaceschemadata");
> 




More information about the libvir-list mailing list