Re: [RFC V9 3/4] domain snapshot design: xl

[Ian Campbell <Ian.Campbell@citrix.com>]

On Tue, 2014-12-16 at 14:32 +0800, Chunyan Liu wrote:
> Changes to V8:
>   * xl won't manage snapshots, that means it won't maintain json files,
>     won't maintain snapshot chain relationship, and then as a result
>     won't take care of deleting snapshot and listing snapshots.
>   * remove snapshot-delete and snapshot-list interface
>   * update snapshot-revert interface
>   * update snapshot-create/revert implementaion
> 
> ===========================================================================
> 
> XL Design
> 
> 1. User Interface
> 
> xl snapshot-create:
>   Create a snapshot (disk and RAM) of a domain.
> 
>   SYNOPSIS:
>     snapshot-create <domain> [<cfgfile>] [--name <string>] [--live]
> 
>   OPTIONS:
>     --name <string>  snapshot name
>     --live           take a live snapshot
> 
>     If option includes --live, then the domain is not paused while creating
>     the snapshot, like live migration. This increases size of the memory
>     dump file, but reducess downtime of the guest.
> 
>     If option doens't include --name, a default name will be generated
>     according to the creation time.
> 
>     If specify @cfgfile, use cfgfile. (e.g. if --name specifies a name,
>     meanwhile there is name specified in cfgfile, name in cfgfile will
>     be used.)

If you do not specify cfgfile then where do things go? Is --name perhaps
also serving as a basename for a path to save things to?

I wonder if we can simplify this a bit and therefore avoid the need cfg
file in common case. e.g.

----8<-------
  xl snapshot-create [--live] [--internal|--external] <domain> <path>

<path> is a path to a directory, which must not exist. This path will be
created and the memory snapshot stored in it using some well defined
name.

If the snapshot is --external then the snapshot disks are created in
<path> with some well defined name based on the virtual device and
backend format in use.

If the snapshot is --internal then the snapshot disks are internal.

The default is <TBD, based on backends in use?>

--live is as you've already got it.
----8<-------

I'm not sure what name and description actually are here, so I've
omitted them.

Any of that could be overridden with e.g. more specific disk
configuration stanzas etc via the cfg file, if necessary.

This is just a suggestion, feel free to disagree.

NB xl create can accept a series of cfg file lines on the command line
too, and treats them as appended to the cfgfile (if any was given). I
think xl snapshot-create should include this functionality too.
> xl snapshot-revert:
>   Revert domain to status of a snapshot.
> 
>   SYNOPSIS:
>       snapshot-revert <domain> <cfgfile> [--running] [--force]
> 
>   OPTIONS:
>     --running        after reverting, change state to running
>     --force          try harder on risky reverts
> 
>     Normally, the domain will revert to the same state the domain was in while
>     the snapshot was taken (whether running, or paused).
> 
>     If option includes --running, then overrides the snapshot state to
>     guarantee a running domain after the revert.
> 
> 
> 
> 2. cfgfile syntax
> 
> #snapshot name. If user doesn't provide a VM snapshot name, xl will generate
> #a name automatically by the creation time.
> name=""

What is name used for? (I guessed it might be output path above, but I'm
not sure).

> #snapshot description. Default is NULL.
> description=""

What is this used for? Is it embedded into e.g. qcow images or is it
just for the admin's own use (in which cases the existing support for
#comments ought to suffice).

> #memory location. This field should be filled when memory=1. Default is NULL.

The memory option isn't defined anywhere, and I think you've rules
disk-only snapshots out of scope for xl, so I think this is just a left
over from a previous revision.

> #e.g. to specify exernal disk snapshot, like this:
> #disks=['/tmp/hda_snapshot.qcow2,qcow2,hda',
>         '/tmp/hdb_snapshot.qcow2,qcow2,hdb',]
> 
> #e.g. to specify internal disk snapshot, like this:
> disks=[',,hda',',,hdb',]

Ideally one or the other of these behaviours would be possible without
needing to be quite so explicit.

Ian.