[PATCH v4 0/5] domain snapshot documents

[PATCH v4 0/5] domain snapshot documents

[Bamvor Jian Zhang]

here is the fourth version about domain snapshot documents, the previous version
is here[1][2][3].

there are lots of potential feature about snapshots. for now, i focus on
providing the api for libvirt libxl driver in order to support the same
functionality compare with libvirt qemu driver.
i plan to work on other "advanced feature" after my first stage patch ack.

there are two types of snapshots supported by libxl: disk snapshot and domain
snapshot and four types of operations: create, delete, list and revert.

Disk snapshot will only be crash-consistent if the domain is running. Disk
snapshots can also be internal (qcow2) or external (snapshot in one file, delta
in another).

Domain snapshots include disk snapshots and domain state, allowing to resume
the domain from the same state when the snapshot was created. This type of
snapshot is also referred to as a domain checkpoint or system checkpoint.

In libvirt, there is a something like resource manager for domain snapshot
managements. So, in libxl, all these information is transfered through
libxl_domain_snapshot struct. xl will manage the snapshot by itself based on
Liu Wei libxl-json apis.

Domain snapshot create means save domain state and do disk snapshots.
At the beginning of domain snapshot create, it will check whether it is
snapshotable.

Domain snapshot revert means rollback the current snapshot state.

the domain snapshot information will store in the follow path:
/var/lib/xen/snapshots/<domain_uuid>/snapshotdata-<snapshot_name>.xl
reference patch 0004 for details.

here is some features i do not want to implement in my first version code:

about non-qdisk backend, after some discussion with Jim, it seems that the
snapshot of these device could be done by difference utils through libxl__exec.
for tap/tap2, td-util or vhd-util.
for lvm, snapshot create, delete, revert could be done by lvcreate, lvremove,
lvconvert respectively.

live snapshot.
i do not know how it should work exactly. if we talk about live memory save,
i guess it is simialr to live migration. but i am not sure how to sync with
live memory save and disk transaction.

changes since v3:
split one document into difference document, hope it will be easier to review and
discuss.

changes since v2:
1), reorgnized the whole docments.
2), do not export the dedicated the disk snapshot commands.
3), others changes according to Ian and Jim's comment.

[1] http://lists.xen.org/archives/html/xen-devel/2014-04/msg00414.html
    http://lists.xen.org/archives/html/xen-devel/2014-04/msg00244.html
[2] http://lists.xen.org/archives/html/xen-devel/2014-04/msg02549.html
[3] http://lists.xen.org/archives/html/xen-devel/2014-05/msg01977.html

Bamvor Jian Zhang (5):
  define snapshot API
  add xl snapshot command in xl manpage
  add xl snapshot configuration syntax file
  examples for xl internal snapshot
  examples for xl external snapshot

 docs/man/xl.pod.1                  | 94 ++++++++++++++++++++++++++++++++++++++
 docs/man/xl.snapshot.conf.pod.5    | 58 +++++++++++++++++++++++
 tools/examples/xlsnapshot.external |  7 +++
 tools/examples/xlsnapshot.internal |  6 +++
 tools/libxl/libxl.h                | 37 +++++++++++++++
 5 files changed, 202 insertions(+)
 create mode 100644 docs/man/xl.snapshot.conf.pod.5
 create mode 100644 tools/examples/xlsnapshot.external
 create mode 100644 tools/examples/xlsnapshot.internal

--
1.8.1.4

[PATCH v4 1/5] define snapshot API

[Bamvor Jian Zhang]

it includes two parts APIs: domain snapshot configuration file operation
(load, store, delete, it base on Wei Liu's libxl-json api) and disk
snapshot operation(create, delete, revert, including implementation
details: choose qmp or qemu-img command).

about xl and libvirt cooperation. currently, libvirt use xml for description
domain snapshot for both user interface and store the snapshot information
on disks. if libvirt libxl driver could use libxl-json format in load/store
domain snapshot configuration, it would be easier for the user who may be
switch xl and libvirt. this will not affect the libvirt user experience.

Signed-off-by: Bamvor Jian Zhang <bjzhang@suse.com>
---
 tools/libxl/libxl.h | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index be722b6..8106f4b 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1261,6 +1261,43 @@ int libxl_load_domain_configuration(libxl_ctx *ctx, uint32_t domid,
 int libxl_store_domain_configuration(libxl_ctx *ctx, uint32_t domid,
                                      libxl_domain_config *d_config);
 
+/* snapshot relative APIs */
+
+/* management functions for domain snapshot configuration */
+
+/* Load, save, delete domain snapshot configuration file. */
+int libxl_load_dom_snapshot_conf(libxl_ctx *ctx, uint32_t domid,
+                                    libxl_domain_snapshot *snapshot);
+int libxl_store_dom_snapshot_conf(libxl_ctx *ctx, uint32_t domid,
+                                     libxl_domain_snapshot *snapshot);
+int libxl_delete_dom_snapshot_conf(libxl_ctx *ctx, uint32_t domid,
+                                   libxl_domain_snapshot *snapshot);
+
+/* retrieve all the snapshot information from disk, put the number of it to num.
+ * caller is responsible for free the libxl_domain_snapshot array.
+ */
+libxl_domain_snapshot *libxl_domain_snapshot_list(libxl_ctx *ctx,
+                                                  uint32_t domid, int *num);
+
+/* functions for disk snapshot operations */
+/* create disk snapshot through qmp transaction */
+int libxl_disk_snapshot_create(libxl_ctx *ctx, int domid,
+                               libxl_disk_snapshot *snapshot, int nb);
+
+/* delete disk snapshot through qmp delete */
+int libxl_disk_snapshot_delete(libxl_ctx *ctx, int domid,
+                               libxl_disk_snapshot *snapshot, int nb);
+
+/* revert disk snapshot through qemu-img snapshot apply command */
+int libxl_disk_snapshot_revert(libxl_ctx *ctx, uint32_t domid,
+                               libxl_disk_snapshot *snapshot, int nb);
+
+/* create libxl_disk_snapshot from libxl_device_disk
+ * will alloc disks if disks empty
+ */
+int libxl_disk_to_snapshot(libxl_ctx *ctx, uint32_t domid,
+                           libxl_domain_snapshot *snapshot);
+
 #include <libxl_event.h>
 
 #endif /* LIBXL_H */
-- 
1.8.1.4

[PATCH v4 2/5] add xl snapshot command in xl manpage

[Bamvor Jian Zhang]

Some discussion on xl snapshot-create, snapshot-revert implementation

xl snapshot-create

snapshot-create will save domain memory through save_domain if it is not a disk
only snapshot. and save disk snapshot by libxl_disk_snapshot_create. after this
successful, it will save snapshot configuration.

xl snapshot-revert
flow: destroy domain -> revert disk snapshot -> restore domain

idealy, i should use libxl__xc_domain_restore for domain memory restore. but i
will be a issue when the domain configuration is different between current vm
and snapshot. so, in my current implementation, firstly, i destroy the current
domain. and after revert disk snapshot, i restore the domain through
create_domain with proper dom_info. so, the revert is not pull the domain
status back. it is different from qemu snapshot implementation, qemu is pull
the domain back to the snapshot point with loadvm hmp.

Signed-off-by: Bamvor Jian Zhang <bjzhang@suse.com>
---
 docs/man/xl.pod.1 | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 94 insertions(+)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 30bd4bf..93badc2 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1368,6 +1368,100 @@ policy. Loading new security policy will reset runtime changes to device labels.
 
 =back
 
+=head1 SNAPSHOT
+
+there are two types of snapshots supported by libxl: disk snapshot and domain
+snapshot and four types of operations: create, delete, list and revert.
+
+Disk snapshot will only be crash-consistent if the domain is running. Disk
+snapshots can also be internal (qcow2) or external (snapshot in one file, delta
+in another).
+
+Domain snapshots include disk snapshots and domain state, allowing to resume
+the domain from the same state when the snapshot was created. This type of
+snapshot is also referred to as a domain checkpoint or system checkpoint.
+
+=over 4
+
+=item B<snapshot-create> [I<configfile>] [I<OPTIONS>] I<domain-id>
+
+create domain/disk snapshot.
+
+The create subcommand takes a config file as first argument: see
+L<xl.snapshot.cfg(5)> for full details of that file format and possible options.
+If I<configfile> is missing B<XL> creates the snapshot of domain from options.
+I<configfile> has to be an absolute path to a file.
+it is allowed if user use I<configfile> combine with I<OPTIONS>, if there is
+some conflict in it, it will report and exit. at the point, it is highly
+recommend that use I<configfile> or I<OPTIONS> alone.
+
+B<OPTIONS>
+
+=over 4
+
+=item B<-n>
+
+domain/disk snapshot name. if ignored, it will be the epoch second from 1, Jan
+1970.
+
+=item B<-D>, B<--disk-only>
+
+take the internal disk snapshot for all disk except the read-only disk
+(e.g. cdrom). for external disk snapshot, user should give the proper options
+(such as external file and format) in I<configfile>.
+
+=back
+
+=item B<snapshot-delete> [I<OPTIONS>] I<domain-id>
+
+delete domain/disk snapshot according to the snapshot configuration file, which
+may include memory check point file, disk snapshot file and configuration file.
+
+B<OPTIONS>
+
+=over 4
+
+=item B<-n>
+
+snapshot name.
+
+=back
+
+=item B<snapshot-list> [I<OPTIONS>] I<domain-id> I<snapshot-name>
+
+Prints information about one or all snapshots for <domain-id>. If no snapshots
+are specified it prints out information about all snapshots.
+
+B<OPTIONS>
+
+=over 4
+
+=item B<-n>
+
+snapshot name.
+
+=item B<-l>, B<--long>
+
+list the details for specific snapshot or all snapshots of such domain.
+
+=back
+
+=item B<snapshot-revert> [I<OPTIONS>] I<domain-id> I<snapshot-name>
+
+revert domain snapshot according to domain snapshot configuration file.
+
+B<OPTIONS>
+
+=over 4
+
+=item B<-n>
+
+snapshot name.
+
+=back
+
+=back
+
 =head1 TO BE DOCUMENTED
 
 We need better documentation for:
-- 
1.8.1.4

[PATCH v4 3/5] add xl snapshot configuration syntax file

[Bamvor Jian Zhang]

Signed-off-by: Bamvor Jian Zhang <bjzhang@suse.com>
---
 docs/man/xl.snapshot.conf.pod.5 | 58 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 docs/man/xl.snapshot.conf.pod.5

diff --git a/docs/man/xl.snapshot.conf.pod.5 b/docs/man/xl.snapshot.conf.pod.5
new file mode 100644
index 0000000..9904fa8
--- /dev/null
+++ b/docs/man/xl.snapshot.conf.pod.5
@@ -0,0 +1,58 @@
+=head1 NAME
+
+xl.snapshot.cfg - XL Domain Snapshot Configuration File Syntax
+
+=head1 DESCRIPTION
+
+without snapshot configuration file, xl snapshot-create could create internal
+domain snapshot or internal disk snapshot. To create a user-defined domain
+snapshot, xl requires a domain snapshot config file. Typically these live in
+`/var/lib/xen/snapshots/UUID/snapshotdata-d.NAME.libxl-json` where
+UUID is the uuid of domain and the NAME is name of snapshot.
+it is highly recommend that the external file snapshot should locate the same
+directory with snapshot configuration file and memory save image.
+here is two examples for internal domain snapshot and external domain snapshot:
+"/etc/xen/examples/xlsnapshot.internal", "/etx/xen/examples/xlsnapshot.external".
+
+=head1 SYNTAX
+
+A domain config file consists of a series of C<KEY=VALUE> pairs. it share the
+same rules with xl.cfg
+
+=head1 OPTIONS
+
+=over 4
+
+=item B<name="NAME">
+
+Specifies the name of the domain snapshot. Names of snapshots existing on a
+single domain must be unique. if ignored, it will be the epoch second from 1,
+Jan 1970.
+
+=item B<description="DESCRIPTION">
+
+the snapshot description.
+
+=item B<creationtime="CREATIONTIME">
+
+the creation time of this snapshot, it will be the epoch second from 1, Jan
+1970. this field could not be specified by user. it will be ignored in domain
+snapshot creation.
+
+=item B<memory="PATHNAME">
+
+describe the location of memory save image, this image is as same as the image
+in "xl save". the value could be the full directory of the location of memory
+save image or "yes" which means `/var/lib/xen/snapshots/UUID/NAME.save`.
+if ignored, this snapshot will be a disk only snapshot.
+
+=item B<disk=[ "DISK_SPEC_STRING", "DISK_SPEC_STRING", ...]>
+
+the disk snapshot description. See F<docs/misc/xl-disk-configuration.txt> for
+full description. for internal snapshot, user should provide vdev. for external
+snapshot, user should provide vdev, format, target. format is the format of
+external snapshot file. only qcow2 is allowed. target is the external snapshot
+file including full path.
+
+=back
+
-- 
1.8.1.4

[PATCH v4 4/5] examples for xl internal snapshot

[Bamvor Jian Zhang]

Signed-off-by: Bamvor Jian Zhang <bjzhang@suse.com>
---
 tools/examples/xlsnapshot.internal | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100644 tools/examples/xlsnapshot.internal

diff --git a/tools/examples/xlsnapshot.internal b/tools/examples/xlsnapshot.internal
new file mode 100644
index 0000000..a9d7fca
--- /dev/null
+++ b/tools/examples/xlsnapshot.internal
@@ -0,0 +1,6 @@
+
+name = "internal_snapshot_after_installation"
+description="a snapshot after OS installation. internal snapshot example"
+creationtime="1397207580"
+save="/var/lib/xen/snapshots/<domain_uuid>/1397207580.save"
+
-- 
1.8.1.4

[PATCH v4 5/5] examples for xl external snapshot

[Bamvor Jian Zhang]

Signed-off-by: Bamvor Jian Zhang <bjzhang@suse.com>
---
 tools/examples/xlsnapshot.external | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 tools/examples/xlsnapshot.external

diff --git a/tools/examples/xlsnapshot.external b/tools/examples/xlsnapshot.external
new file mode 100644
index 0000000..fa24857
--- /dev/null
+++ b/tools/examples/xlsnapshot.external
@@ -0,0 +1,7 @@
+
+name = "1397207577"
+description="a snapshot after OS installation. external snapshot example"
+creationtime="1397207577"
+save="/var/lib/xen/snapshots/<domain_uuid>/1397207577.save"
+disk=[ 'hda=/var/lib/xen/snapshots/<domain_uuid>/disk_hda.qcow2,type=qcow2', 'hdc=/var/lib/xen/snapshots/<domain_uuid>/disk_hdc.qcow2,type=qcow2']
+
-- 
1.8.1.4

Re: [PATCH v4 1/5] define snapshot API

[Chun Yan Liu]

>>> On 6/23/2014 at 07:25 PM, in message
<1403522755-6894-2-git-send-email-bjzhang@suse.com>, Bamvor Jian Zhang
<bjzhang@suse.com> wrote: 
> it includes two parts APIs: domain snapshot configuration file operation 
> (load, store, delete, it base on Wei Liu's libxl-json api) and disk 
> snapshot operation(create, delete, revert, including implementation 
> details: choose qmp or qemu-img command). 
>  
> about xl and libvirt cooperation. currently, libvirt use xml for description 
> domain snapshot for both user interface and store the snapshot information 
> on disks. if libvirt libxl driver could use libxl-json format in load/store 
> domain snapshot configuration, it would be easier for the user who may be 
> switch xl and libvirt. this will not affect the libvirt user experience. 
>  
> Signed-off-by: Bamvor Jian Zhang <bjzhang@suse.com> 
> --- 
>  tools/libxl/libxl.h | 37 +++++++++++++++++++++++++++++++++++++ 
>  1 file changed, 37 insertions(+) 
>  
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h 
> index be722b6..8106f4b 100644 
> --- a/tools/libxl/libxl.h 
> +++ b/tools/libxl/libxl.h 
> @@ -1261,6 +1261,43 @@ int libxl_load_domain_configuration(libxl_ctx *ctx,  
> uint32_t domid, 
>  int libxl_store_domain_configuration(libxl_ctx *ctx, uint32_t domid, 
>                                       libxl_domain_config *d_config); 
>   
> +/* snapshot relative APIs */ 
> + 
> +/* management functions for domain snapshot configuration */ 
> + 
> +/* Load, save, delete domain snapshot configuration file. */ 
> +int libxl_load_dom_snapshot_conf(libxl_ctx *ctx, uint32_t domid, 
> +                                    libxl_domain_snapshot *snapshot); 
> +int libxl_store_dom_snapshot_conf(libxl_ctx *ctx, uint32_t domid, 
> +                                     libxl_domain_snapshot *snapshot); 
> +int libxl_delete_dom_snapshot_conf(libxl_ctx *ctx, uint32_t domid, 
> +                                   libxl_domain_snapshot *snapshot); 

Do we need libxl_domain_snapshot as parameter here? Why not
'ctx, domid, snapshot-name'?

> + 
> +/* retrieve all the snapshot information from disk, put the number of it to  
> num. 

Typo  here. Is it to retrieve the snapshot info of the domain?
If  domain has 2 snapshots, return the 2 snapshots' info and set num=2.

> + * caller is responsible for free the libxl_domain_snapshot array. 
> + */ 
> +libxl_domain_snapshot *libxl_domain_snapshot_list(libxl_ctx *ctx, 
> +                                                  uint32_t domid, int  
> *num); 
> + 
> +/* functions for disk snapshot operations */ 
> +/* create disk snapshot through qmp transaction */ 
> +int libxl_disk_snapshot_create(libxl_ctx *ctx, int domid, 
> +                               libxl_disk_snapshot *snapshot, int nb); 
> + 
> +/* delete disk snapshot through qmp delete */ 
> +int libxl_disk_snapshot_delete(libxl_ctx *ctx, int domid, 
> +                               libxl_disk_snapshot *snapshot, int nb); 
> + 
> +/* revert disk snapshot through qemu-img snapshot apply command */ 
> +int libxl_disk_snapshot_revert(libxl_ctx *ctx, uint32_t domid, 
> +                               libxl_disk_snapshot *snapshot, int nb); 
> + 
> +/* create libxl_disk_snapshot from libxl_device_disk 
> + * will alloc disks if disks empty 
> + */ 
> +int libxl_disk_to_snapshot(libxl_ctx *ctx, uint32_t domid, 
> +                           libxl_domain_snapshot *snapshot); 

What is this function for? And how would we use it? It's better to have some
explanation to the functionality and the parameters.

Regards,
Chunyan

> + 
>  #include <libxl_event.h> 
>   
>  #endif /* LIBXL_H */ 

Re: [PATCH v4 1/5] define snapshot API

[Chun Yan Liu]

>>> On 6/23/2014 at 07:25 PM, in message
<1403522755-6894-2-git-send-email-bjzhang@suse.com>, Bamvor Jian Zhang
<bjzhang@suse.com> wrote: 
> it includes two parts APIs: domain snapshot configuration file operation 
> (load, store, delete, it base on Wei Liu's libxl-json api) and disk 
> snapshot operation(create, delete, revert, including implementation 
> details: choose qmp or qemu-img command). 
>  
> about xl and libvirt cooperation. currently, libvirt use xml for description 
> domain snapshot for both user interface and store the snapshot information 
> on disks. if libvirt libxl driver could use libxl-json format in load/store 
> domain snapshot configuration, it would be easier for the user who may be 
> switch xl and libvirt. this will not affect the libvirt user experience. 
>  
> Signed-off-by: Bamvor Jian Zhang <bjzhang@suse.com> 
> --- 
>  tools/libxl/libxl.h | 37 +++++++++++++++++++++++++++++++++++++ 
>  1 file changed, 37 insertions(+) 
>  
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h 
> index be722b6..8106f4b 100644 
> --- a/tools/libxl/libxl.h 
> +++ b/tools/libxl/libxl.h 
> @@ -1261,6 +1261,43 @@ int libxl_load_domain_configuration(libxl_ctx *ctx,  
> uint32_t domid, 
>  int libxl_store_domain_configuration(libxl_ctx *ctx, uint32_t domid, 
>                                       libxl_domain_config *d_config); 
>   
> +/* snapshot relative APIs */ 
> + 
> +/* management functions for domain snapshot configuration */ 
> + 
> +/* Load, save, delete domain snapshot configuration file. */ 
> +int libxl_load_dom_snapshot_conf(libxl_ctx *ctx, uint32_t domid, 
> +                                    libxl_domain_snapshot *snapshot); 
> +int libxl_store_dom_snapshot_conf(libxl_ctx *ctx, uint32_t domid, 
> +                                     libxl_domain_snapshot *snapshot); 
> +int libxl_delete_dom_snapshot_conf(libxl_ctx *ctx, uint32_t domid, 
> +                                   libxl_domain_snapshot *snapshot); 
> + 
> +/* retrieve all the snapshot information from disk, put the number of it to  
> num. 
> + * caller is responsible for free the libxl_domain_snapshot array. 
> + */ 
> +libxl_domain_snapshot *libxl_domain_snapshot_list(libxl_ctx *ctx, 
> +                                                  uint32_t domid, int  
> *num); 
> + 
> +/* functions for disk snapshot operations */ 
> +/* create disk snapshot through qmp transaction */ 
> +int libxl_disk_snapshot_create(libxl_ctx *ctx, int domid, 
> +                               libxl_disk_snapshot *snapshot, int nb); 
> + 
> +/* delete disk snapshot through qmp delete */ 
> +int libxl_disk_snapshot_delete(libxl_ctx *ctx, int domid, 
> +                               libxl_disk_snapshot *snapshot, int nb); 
> + 
> +/* revert disk snapshot through qemu-img snapshot apply command */ 
> +int libxl_disk_snapshot_revert(libxl_ctx *ctx, uint32_t domid, 
> +                               libxl_disk_snapshot *snapshot, int nb); 
> + 
> +/* create libxl_disk_snapshot from libxl_device_disk 
> + * will alloc disks if disks empty 
> + */ 
> +int libxl_disk_to_snapshot(libxl_ctx *ctx, uint32_t domid, 
> +                           libxl_domain_snapshot *snapshot); 
> + 
>  #include <libxl_event.h> 
>   
>  #endif /* LIBXL_H */ 

I didn't see the details of structure libxl_disk_snapshot. Miss it?

And generally, I think it would be helpful to roughly introduce your ideas in
this patch, like:
a) User interface, how would you expect user to use vm snapshot functionality?
     xl snapshot-create xxx xxx xxx
     xl snapshot-delete xxx xxx xxx
b) To each operation, rough ideas on how you would implement. That may help
    to understand why you introduce new structures and new APIs.
c) New structures
d) New APIs (would be helpful if there is description on function, parameters,
     return value.)

Thanks,
Chunyan

Re: [PATCH v4 1/5] define snapshot API

[Bamvor Jian Zhang]

Hi, Chunyan
> >>> On 6/23/2014 at 07:25 PM, in message
> <1403522755-6894-2-git-send-email-bjzhang@suse.com>, Bamvor Jian Zhang
> <bjzhang@suse.com> wrote:
> > it includes two parts APIs: domain snapshot configuration file operation
> > (load, store, delete, it base on Wei Liu's libxl-json api) and disk
> > snapshot operation(create, delete, revert, including implementation
> > details: choose qmp or qemu-img command).
> >
> > about xl and libvirt cooperation. currently, libvirt use xml for description
> > domain snapshot for both user interface and store the snapshot information
> > on disks. if libvirt libxl driver could use libxl-json format in load/store
> > domain snapshot configuration, it would be easier for the user who may be
> > switch xl and libvirt. this will not affect the libvirt user experience.
> >
> > Signed-off-by: Bamvor Jian Zhang <bjzhang@suse.com>
> > ---
> >  tools/libxl/libxl.h | 37 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> >
> > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> > index be722b6..8106f4b 100644
> > --- a/tools/libxl/libxl.h
> > +++ b/tools/libxl/libxl.h
> > @@ -1261,6 +1261,43 @@ int libxl_load_domain_configuration(libxl_ctx *ctx,
> > uint32_t domid,
> >  int libxl_store_domain_configuration(libxl_ctx *ctx, uint32_t domid,
> >                                       libxl_domain_config *d_config);
> >
> > +/* snapshot relative APIs */
> > +
> > +/* management functions for domain snapshot configuration */
> > +
> > +/* Load, save, delete domain snapshot configuration file. */
> > +int libxl_load_dom_snapshot_conf(libxl_ctx *ctx, uint32_t domid,
> > +                                    libxl_domain_snapshot *snapshot);
> > +int libxl_store_dom_snapshot_conf(libxl_ctx *ctx, uint32_t domid,
> > +                                     libxl_domain_snapshot *snapshot);
> > +int libxl_delete_dom_snapshot_conf(libxl_ctx *ctx, uint32_t domid,
> > +                                   libxl_domain_snapshot *snapshot);
>
> Do we need libxl_domain_snapshot as parameter here? Why not
> 'ctx, domid, snapshot-name'?
yes, i only need the name of snapshot for deleting. i put libxl_domain_snapshot
here in order to consistent with load and store functions above.
>
> > +
> > +/* retrieve all the snapshot information from disk, put the number of it to
> > num.
>
> Typo  here. Is it to retrieve the snapshot info of the domain?
> If  domain has 2 snapshots, return the 2 snapshots' info and set num=2.
yes. thanks.
>
> > + * caller is responsible for free the libxl_domain_snapshot array.
> > + */
> > +libxl_domain_snapshot *libxl_domain_snapshot_list(libxl_ctx *ctx,
> > +                                                  uint32_t domid, int
> > *num);
> > +
> > +/* functions for disk snapshot operations */
> > +/* create disk snapshot through qmp transaction */
> > +int libxl_disk_snapshot_create(libxl_ctx *ctx, int domid,
> > +                               libxl_disk_snapshot *snapshot, int nb);
> > +
> > +/* delete disk snapshot through qmp delete */
> > +int libxl_disk_snapshot_delete(libxl_ctx *ctx, int domid,
> > +                               libxl_disk_snapshot *snapshot, int nb);
> > +
> > +/* revert disk snapshot through qemu-img snapshot apply command */
> > +int libxl_disk_snapshot_revert(libxl_ctx *ctx, uint32_t domid,
> > +                               libxl_disk_snapshot *snapshot, int nb);
> > +
> > +/* create libxl_disk_snapshot from libxl_device_disk
> > + * will alloc disks if disks empty
> > + */
> > +int libxl_disk_to_snapshot(libxl_ctx *ctx, uint32_t domid,
> > +                           libxl_domain_snapshot *snapshot);
>
> What is this function for? And how would we use it? It's better to have some
> explanation to the functionality and the parameters.
sorry for confuse. this function will save disk parameter to libxl_disk_snapshot
from libxl_diskinfo. reference my next reply for structure definition.
libxl_disk_to_snapshot will get disk list through libxl_device_disk_list, and
get libxl_diskinfo through libxl_device_disk_getinfo. and it will only process
the disk support snapshot in current implementaiton. e.g.
for internal snapshot: only qcow2 is supported.
for external snapshot: raw and qcow2 is supported.

regards

bamvor

Re: [PATCH v4 1/5] define snapshot API

[Bamvor Jian Zhang]

Hi, Chunyan

> >>> On 6/23/2014 at 07:25 PM, in message
> <1403522755-6894-2-git-send-email-bjzhang@suse.com>, Bamvor Jian Zhang
> <bjzhang@suse.com> wrote:
> > it includes two parts APIs: domain snapshot configuration file operation
> > (load, store, delete, it base on Wei Liu's libxl-json api) and disk
> > snapshot operation(create, delete, revert, including implementation
> > details: choose qmp or qemu-img command).
> >
> > about xl and libvirt cooperation. currently, libvirt use xml for description
> > domain snapshot for both user interface and store the snapshot information
> > on disks. if libvirt libxl driver could use libxl-json format in load/store
> > domain snapshot configuration, it would be easier for the user who may be
> > switch xl and libvirt. this will not affect the libvirt user experience.
> >
> > Signed-off-by: Bamvor Jian Zhang <bjzhang@suse.com>
> > ---
> >  tools/libxl/libxl.h | 37 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> >
> > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> > index be722b6..8106f4b 100644
> > --- a/tools/libxl/libxl.h
> > +++ b/tools/libxl/libxl.h
> > @@ -1261,6 +1261,43 @@ int libxl_load_domain_configuration(libxl_ctx *ctx,
> > uint32_t domid,
> >  int libxl_store_domain_configuration(libxl_ctx *ctx, uint32_t domid,
> >                                       libxl_domain_config *d_config);
> >
> > +/* snapshot relative APIs */
> > +
> > +/* management functions for domain snapshot configuration */
> > +
> > +/* Load, save, delete domain snapshot configuration file. */
> > +int libxl_load_dom_snapshot_conf(libxl_ctx *ctx, uint32_t domid,
> > +                                    libxl_domain_snapshot *snapshot);
> > +int libxl_store_dom_snapshot_conf(libxl_ctx *ctx, uint32_t domid,
> > +                                     libxl_domain_snapshot *snapshot);
> > +int libxl_delete_dom_snapshot_conf(libxl_ctx *ctx, uint32_t domid,
> > +                                   libxl_domain_snapshot *snapshot);
> > +
> > +/* retrieve all the snapshot information from disk, put the number of it to
> > num.
> > + * caller is responsible for free the libxl_domain_snapshot array.
> > + */
> > +libxl_domain_snapshot *libxl_domain_snapshot_list(libxl_ctx *ctx,
> > +                                                  uint32_t domid, int
> > *num);
> > +
> > +/* functions for disk snapshot operations */
> > +/* create disk snapshot through qmp transaction */
> > +int libxl_disk_snapshot_create(libxl_ctx *ctx, int domid,
> > +                               libxl_disk_snapshot *snapshot, int nb);
> > +
> > +/* delete disk snapshot through qmp delete */
> > +int libxl_disk_snapshot_delete(libxl_ctx *ctx, int domid,
> > +                               libxl_disk_snapshot *snapshot, int nb);
> > +
> > +/* revert disk snapshot through qemu-img snapshot apply command */
> > +int libxl_disk_snapshot_revert(libxl_ctx *ctx, uint32_t domid,
> > +                               libxl_disk_snapshot *snapshot, int nb);
> > +
> > +/* create libxl_disk_snapshot from libxl_device_disk
> > + * will alloc disks if disks empty
> > + */
> > +int libxl_disk_to_snapshot(libxl_ctx *ctx, uint32_t domid,
> > +                           libxl_domain_snapshot *snapshot);
> > +
> >  #include <libxl_event.h>
> >
> >  #endif /* LIBXL_H */
>
> I didn't see the details of structure libxl_disk_snapshot. Miss it?
sorry, here is the struct, it is defined in libxl_types.idl. this get the benefit
from Liu wei's libxl-json, e.g. libxl_domain_snapshot_to_json and
libxl_domain_snapshot_from_json will be used by libxl_load_dom_snapshot_conf
and libxl_store_dom_snapshot_conf respectively.

the definition and usecase of libxl_disk_snapshot and libxl_domain_snapshot
is coherent with patch 0003-add-xl-snapshot-configuration-syntax-file.patch

libxl_disk_snapshot = Struct("disk_snapshot",[
    ("device",        string),              //name of disk: hda, hdc
    ("name",          string),              //name of this disk snapshot,
                                            //usually it is inherit from
                                            //libxl_domain_snapshot
    ("file",          string),              //the external snapshot file.
                                            //empty in internal snapshot.
    ("format",        libxl_disk_format),   //the external snapshot format
                                            //for internal snapshot, it will be
                                            //ignored and it should be
                                            //LIBXL_DISK_FORMAT_UNKNOWN
    ("path",          string),              //path for disk backend. it always
                                            //get from libxl_device_disk_getinfo.
                                            //it will be empty string when store
                                            //domain snapshot configuration in
                                            //order to hide this from users.
    ])

libxl_domain_snapshot = Struct("domain_snapshot",[
    ("name",          string),              //name of domain snapshot
    ("description",   string),              //description of domain snapshot
    ("creation_time", uint64),              //the creation time of domain
                                            //snapshot which is the epoch
                                            //second from 1, Jan 1970.
    ("memory",        string),              //path for domain memory save.
                                            //empty mean it is a disk-only
                                            //snapshot.
    //the following state get from libxl_domain_info.
    ("running",       bool),
    ("blocked",       bool),
    ("paused",        bool),
    ("shutdown",      bool),
    ("dying",         bool),
    ("disks", Array(libxl_disk_snapshot, "num_disks")),//each disk snapshot
                                            //information belong to this
                                            //domain snapshot.
    ])

regards

bamvor
>
> And generally, I think it would be helpful to roughly introduce your ideas in
> this patch, like:
> a) User interface, how would you expect user to use vm snapshot functionality?
>      xl snapshot-create xxx xxx xxx
>      xl snapshot-delete xxx xxx xxx
> b) To each operation, rough ideas on how you would implement. That may help
>     to understand why you introduce new structures and new APIs.
> c) New structures
> d) New APIs (would be helpful if there is description on function, parameters,
>      return value.)
>
> Thanks,
> Chunyan

Re: [PATCH v4 1/5] define snapshot API

[Jim Fehlig]

Chun Yan Liu wrote:
>   
>>>> On 6/23/2014 at 07:25 PM, in message
>>>>         
> <1403522755-6894-2-git-send-email-bjzhang@suse.com>, Bamvor Jian Zhang
> <bjzhang@suse.com> wrote: 
>   
>> it includes two parts APIs: domain snapshot configuration file operation 
>> (load, store, delete, it base on Wei Liu's libxl-json api) and disk 
>> snapshot operation(create, delete, revert, including implementation 
>> details: choose qmp or qemu-img command). 
>>  
>> about xl and libvirt cooperation. currently, libvirt use xml for description 
>> domain snapshot for both user interface and store the snapshot information 
>> on disks. if libvirt libxl driver could use libxl-json format in load/store 
>> domain snapshot configuration, it would be easier for the user who may be 
>> switch xl and libvirt. this will not affect the libvirt user experience. 
>>  
>> Signed-off-by: Bamvor Jian Zhang <bjzhang@suse.com> 
>> --- 
>>  tools/libxl/libxl.h | 37 +++++++++++++++++++++++++++++++++++++ 
>>  1 file changed, 37 insertions(+) 
>>  
>> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h 
>> index be722b6..8106f4b 100644 
>> --- a/tools/libxl/libxl.h 
>> +++ b/tools/libxl/libxl.h 
>> @@ -1261,6 +1261,43 @@ int libxl_load_domain_configuration(libxl_ctx *ctx,  
>> uint32_t domid, 
>>  int libxl_store_domain_configuration(libxl_ctx *ctx, uint32_t domid, 
>>                                       libxl_domain_config *d_config); 
>>   
>> +/* snapshot relative APIs */ 
>> + 
>> +/* management functions for domain snapshot configuration */ 
>> + 
>> +/* Load, save, delete domain snapshot configuration file. */ 
>> +int libxl_load_dom_snapshot_conf(libxl_ctx *ctx, uint32_t domid, 
>> +                                    libxl_domain_snapshot *snapshot); 
>> +int libxl_store_dom_snapshot_conf(libxl_ctx *ctx, uint32_t domid, 
>> +                                     libxl_domain_snapshot *snapshot); 
>> +int libxl_delete_dom_snapshot_conf(libxl_ctx *ctx, uint32_t domid, 
>> +                                   libxl_domain_snapshot *snapshot); 
>> + 
>> +/* retrieve all the snapshot information from disk, put the number of it to  
>> num. 
>> + * caller is responsible for free the libxl_domain_snapshot array. 
>> + */ 
>> +libxl_domain_snapshot *libxl_domain_snapshot_list(libxl_ctx *ctx, 
>> +                                                  uint32_t domid, int  
>> *num); 
>> + 
>> +/* functions for disk snapshot operations */ 
>> +/* create disk snapshot through qmp transaction */ 
>> +int libxl_disk_snapshot_create(libxl_ctx *ctx, int domid, 
>> +                               libxl_disk_snapshot *snapshot, int nb); 
>> + 
>> +/* delete disk snapshot through qmp delete */ 
>> +int libxl_disk_snapshot_delete(libxl_ctx *ctx, int domid, 
>> +                               libxl_disk_snapshot *snapshot, int nb); 
>> + 
>> +/* revert disk snapshot through qemu-img snapshot apply command */ 
>> +int libxl_disk_snapshot_revert(libxl_ctx *ctx, uint32_t domid, 
>> +                               libxl_disk_snapshot *snapshot, int nb); 
>> + 
>> +/* create libxl_disk_snapshot from libxl_device_disk 
>> + * will alloc disks if disks empty 
>> + */ 
>> +int libxl_disk_to_snapshot(libxl_ctx *ctx, uint32_t domid, 
>> +                           libxl_domain_snapshot *snapshot); 
>> + 
>>  #include <libxl_event.h> 
>>   
>>  #endif /* LIBXL_H */ 
>>     
>
> I didn't see the details of structure libxl_disk_snapshot. Miss it?
>
> And generally, I think it would be helpful to roughly introduce your ideas in
> this patch, like:
> a) User interface, how would you expect user to use vm snapshot functionality?
>      xl snapshot-create xxx xxx xxx
>      xl snapshot-delete xxx xxx xxx
>   

Agreed.  Here you could also provide some user stories.  E.g. "As an
admin, I want to snapshot the system before applying the latest kernel
updates, so I can revert to the known working state if the update
introduces regressions."  And then describe how this is done with the
user interface.  IMO, all of this, along with the information in the
cover letter, should be in a new 'docs/misc/snapshot-HOTOW.txt' or such
file.

> b) To each operation, rough ideas on how you would implement. That may help
>     to understand why you introduce new structures and new APIs.
> c) New structures
> d) New APIs (would be helpful if there is description on function, parameters,
>      return value.)
>   

Yep, this sounds like a reasonable approach to me.

Regards,
Jim

Re: [PATCH v4 4/5] examples for xl internal snapshot

[Jim Fehlig]

Bamvor Jian Zhang wrote:
> Signed-off-by: Bamvor Jian Zhang <bjzhang@suse.com>
> ---
>  tools/examples/xlsnapshot.internal | 6 ++++++
>  1 file changed, 6 insertions(+)
>  create mode 100644 tools/examples/xlsnapshot.internal
>
> diff --git a/tools/examples/xlsnapshot.internal b/tools/examples/xlsnapshot.internal
> new file mode 100644
> index 0000000..a9d7fca
> --- /dev/null
> +++ b/tools/examples/xlsnapshot.internal
> @@ -0,0 +1,6 @@
> +
> +name = "internal_snapshot_after_installation"
> +description="a snapshot after OS installation. internal snapshot example"
> +creationtime="1397207580"
> +save="/var/lib/xen/snapshots/<domain_uuid>/1397207580.save"
> +
>   

IMO, if this example (and the one in 5/5) is needed, it should be in the
aforementioned docs/misc/snapshot-HOWTO.txt.

Regards,
Jim