Re: [Xen-devel] [RFC V6] Libxl Domain Snapshot API Design

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-08-27 at 15:22 +0800, Chunyan Liu wrote:
> Since Bamvor left SUSE and turns to work on ARM server, I'd like to continue
> this work and make progress. Following the discussion about V5, which mainly
> focused on the API design, here post the updated API design. Thanks for any
> of your further suggestions!
> 
> Main changes to V5:
> * libxl_disk_snapshot: reuse libxl_device_disk rather than specify path,
>     format separately in the structure. Including two libxl_device_disk
>     components, one is to indicate the original disk info, one is to
>     indicate the external snapshot info if it is 'external snapshot'.
> * define common APIs for domain snapshot creating/deleting/reverting,
>   rather than a group of functions for disk snapshot operations.
> * remove those APIs for loading/storing/deleting snapshot config.

Please could you say a few words on what a snapshot actually is (i.e.
what are its component parts), I think I know but it would be good to
make sure we are all on the same page.

This might include a description of what "internal" vs "external"
snapshots are (see below).

> 
> V5 is here:
> http://lists.xenproject.org/archives/html/xen-devel/2014-07/msg00893.html
> V5 about API Design is here:
> http://lists.xenproject.org/archives/html/xen-devel/2014-07/msg00897.html
> 
> ===========================================================================
> Libxl Domain Snapshot API
> 
> libxl_domain_snapshot = Struct("domain_snapshot",[

Please can you say which of these needs to be filled in by the caller of
libxl_domain_snapshot_create and which are filled in by that function
(i.e. which fields are inputs and which are outputs).

My guess is that most of these are inputs.

>     ("name",          string),              /* snapshot name */
>     ("description",   string),              /* snapshot description */
>     ("creation_time", uint64),              /* creation time, in seconds */

Are these necessary at the libxl level? They seem like the sort of thing
the toolstack ought to be keeping as part of its overall snapshot
tracking.

>     /* save memory or not. "false" means disk-only snapshot */
>     ("memory",        bool),
> 
>     /* memory state file when snapshot is external */

Under what circumstances is the snapshot external? What is it external
to?

>     ("memory_file",   string),
> 
>     /* Array to store disk snapshot info. */
>     ("disks", Array(libxl_disk_snapshot, "num_disks")),
>     ])
> 
> libxl_disk_snapshot = Struct("disk_snapshot",[
>     ("disk",           libxl_device_disk),      /* orignal disk */

"original"

>     ("name",           string),                 /* snapshot name */
>     ("external",       bool),                   /* external snapshot or not */
> 
>     /* external snapshot info, including file path and format, etc.
>      * if "external" is false, this will be "NULL".
>      */
>     ("external_sn",    libxl_device_disk),

What does the "sn" suffix stand for?

> int libxl_domain_snapshot_create(libxl_ctx *ctx, const char *domname,

Should take a domid not a name, for consistency with all the other libxl
functions.

>                                  libxl_domain_snapshot *snapshot,
>                                  unsigned int flags);
> 
>     Creates a new snapshot of a domain based on the snapshot config contained
>     in @snapshot.
> 
>     If @flags includes LIBXL_DOMAIN_SNAPSHOT_CREATE_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. Only support
>     this flag during external checkpoints.
> 
>     If @flags includes LIBXL_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY, then the 
> snapshot
>     will be limited to the disks described in @snapshot, and no VM state will
>     be saved. For an active guest, this is not supported.

Isn't this redundant with the ->memory  field of the snapshot object?

>     ctx: context
>     domname:  domain name
>     snapshot: configuration of domain snapshot
>     flags: bitwise-OR of libxlDomainSnapshotCreateFlags
>     Returns: 0 on success, -1 on failure
> 
> 
> int libxl_domain_snapshot_delete(libxl_ctx *ctx, const char *domname,

Hrm, this suggests that libxl will have some mechanism for managing
snapshots, is that right?

I don't think libxl should have that functionality since that is the
toolstack's responsibility to manage the snapshots once they are
created, using whatever means it likes.

libxl should be providing the mechanisms ("take a snapshot and put it
here") but not the policies ("snapshots live in this directory, have
this lifecycle and this format").

Similar to how libxl provides a way to say "take these bits and present
them as a disk to the guest", but it leaves the management of image
files to the toolstack (or in the case of xl the actual user).

>                                  const char *snapshot_name,

If this were a libxl_domain_snapshot object I could just about imagine
that this would be useful helper which just iterated over the files
referenced by the snapshot and removed them. I'm not sure how useful
that helper would be in practice though (depends on the toolstack's
actual requirements).

>                                  unsigned int flags);
> 
>     Delete a snapshot.
> 
>     If @flags is 0, then just this snapshot is deleted, and changes from this
>     snapshot are automatically merged into children snapshots.
> 
>     If @flags includes LIBXL_DOMAIN_SNAPSHOT_DELETE_CHILDREN, then this 
> snapshot
>     and any descendant snapshots are deleted.

This definitely sounds like toolstack level functionality to me.

>     ctx: context
>     domname: domain name
>     snapshot_name: snapshot name
>     flags: bitwise-OR of supported libxlDomainSnapshotDeleteFlags
>     Returns: 0 on success, -1 on error.
> 
> int libxl_disk_snapshot_revert(libxl_ctx *ctx, const char *domname,

Should take a domid.

>                                const char *snapshot_name,

The input here should be a libxl_domain_snapshot object I think.
(Otherwise libxl would have to track/manage snapshot names) 

>                                unsigned int flags);
> 
>     Revert the domain to a given snapshot.
> 
>     Normally, the domain will revert to the same state the domain was in while
>     the snapshot was taken (whether inactive, running, or paused).

What is the distinction between inactive and paused?

> 
>     If @flags includes LIBXL_DOMAIN_SNAPSHOT_REVERT_RUNNING, then overrides 
> the
>     snapshot state to guarantee a running domain after the revert.
> 
>     If @flags includes LIBXL_DOMAIN_SNAPSHOT_REVERT_PAUSED, then guarantees a
>     paused domain after the revert.
> 
>     ctx: context
>     domname: domain name
>     snapshot_name: snapshot name
>     flags: bitwise-OR of supported libxlDomainSnapshotRevertFlags
>     Returns: 0 on success, -1 on error.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel