Re: [RFC v2] vm snapshot documents

["Bamvor Jian Zhang" <bjzhang@suse.com>]

Hi,

 >>>Jim Fehlig <jfehlig@suse.com> wrote: 
> Bamvor Jian Zhang wrote:
> > Hi,
> > here is the second version about vm snapshot documents, the first version is
> > here[1]
> Hi Bamvor,
>
> Thanks for being adventurous and starting this work :-).
thanks for your reply.
>
> > thanks Ian Jackson suggestion, i write this doc about vm snapshot including
> > new
> > command, struct and api. currently, the disk snapshot(create, delete, list)
> > worked(patch is already sent[2]), and vm snapshot(create, delete, list) work.
> > but i have some questions about snapshot apply(see the last section).
> >
> > feel free to comment it. thanks.
> >
> > 1, new command
> > =head1 SNAPSHOT
> >
> > there are two types of snapshots supported by libxl: disk snapshot and vm
> > snapshot.
>
> Here and throughout, you should be consistent with the use of 'vm'
> verses 'domain'. For the most part, the existing docs use 'domain'.
>
> It would be good to provide more detail about these types of snapshots
> too. E.g. a 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.
thanks, i will add more description in next version.
>
> >  The following subcommands management the snapshot of a domain,
> > including create, delete, list and apply.
> >
>
> I struggle over apply vs revert, but lean toward the latter.
so as to me.
>
> > a domain snapshot(or a vm snapshot) means save the domain config, memory and
> > disk snapshot. currently, only support qcow2 and internal disk snapshot.
> >
> > the vm snapshot information will store in the follow path:
> > /var/lib/xen/snapshots/<domain_uuid>/snapshotdata-<snapshot_name>.xl
> >
> > here is an example for snapshot information file:
> > snapshot_name="1397207577"
> > snapshot_creationtime="1397207577"
> > snapshot_save="/var/lib/xen/snapshots/5c84adcc-bd59-788a-96d2-195f9b599cfe/1397207577.save"
> >
>
> The snapshot prefix seems redundant. A 'description' entry would be
> useful too.
ok.
>
> > the user could give a snapshot name when vm snapshot created. if not, the
> > epoch
> > seconds will set as name as the above examples.
> >
> > =over 4
> >
> > =item B<vm-snapshot-create> [I<OPTIONS>] I<domain-id>
> >
> > create vm snapshot.
> > it will call the qmp transaction for creating the disk snapshot in order to
> > ensure the disk snapshot is coherence.
> > vm is paused during snapshot create, and is unpause after take snapshot
> > finished.
> >
>
> I think the user should be able to choose whether to live snapshot vs
> pausing/unpausing, via an option. E.g. 'xl snapshot-create <dom> ...
> --live'.
i agree that live snapshot may be useful. meanwhile i am not sure how could i
implement the live snapshot. it might be hard to ensure the consistent between
memory and disk.
>
> > B<OPTIONS>
> >
> > =over 4
> >
> > =item B<-n>
> >
> > vm snapshot name
> >
> > =back
> >
> > =item B<vm-snapshot-delete> [I<OPTIONS>] I<domain-id>
> >
>
> This should take a snapshot name, since a domain could have several.
yes, see the option "-n" below.
>
> > delete vm snapshot.
> > delete the saved memory file and snapshot information file created by
> > vm-snapshot-create.
> >
> > B<OPTIONS>
> >
> > =over 4
> >
> > =item B<-n>
> >
> > vm snapshot name
> >
> > =back
> >
> > =item B<vm-snapshot-list> I<domain-id>
> >
> > list vm snapshot for the dedicated domain including snapshot name and creation
> > time which stored in the vm snapshot information file.
> >
>
> This could behave like domain listing in xl. E.g. 'xl snapshot-list
> <dom>' will list all snapshots. 'xl snapshot-list --long <dom>' will
> list detailed info about all snapshots. 'xl snapshot-list --long <dom>
> <snapshot-name>' will list detailed info on the requested snapshot.
yeah, it it usefull. i will add this feature.
>
> > =item B<vm-snapshot-apply> [I<OPTIONS>] I<domain-id>
> >
> > apply vm snapshot for the dedicated domain.
> >
> > B<OPTIONS>
> >
> > =over 4
> >
> > =item B<-n>
> >
> > vm snapshot name
> >
> > =back
> >
> > =item B<disk-snapshot-create> [I<OPTIONS>] I<domain-id>
> >
> > create disk snapshot.
> >
> > B<OPTIONS>
> >
> > =over 4
> >
> > =item B<-n>
> >
> > disk snapshot name
> >
> > =back
> >
> > =item B<disk-snapshot-delete> [I<OPTIONS>] I<domain-id>
> >
> > delete disk snapshot by snapshot name or snapshot id.
> >
> > B<OPTIONS>
> >
> > =over 4
> >
> > =item B<-i>
> >
> > disk snapshot id
> >
> > =item B<-n>
> >
> > disk snapshot name
> >
> > =back
> >
> > =item B<disk-snapshot-list> I<domain-id>
> >
> > list disk snapshot including snapshot id, tag, vm size, date, vm clock.
> >
> > =back
> >
>
> IMO, the vm-snapshot-* and disk-snapshot-* subcommands can be combined
> into one snapshot-*, and passing an option for disk only snapshots. E.g.
> 'xl snapshot-create <dom> ... --disk-only'.
sure.
>
> > 2, new struct and new api
> > 1), new struct
> > (1), libxl_snapshot struct store a disk snapshot information, which get from
> > qcow2 image through "query-block" qmp command.
> >
> > libxl_snapshot = Struct("snapshot",[
> >     ("device",        string),
> >
>
> Should this be libxl_device_disk instead of a string? And should
> multiple be supported, allowing control over which of a domain's disks
> are included in the snapshot?
one libxl_snapshot represent one disk. see my comment below.
>
> "description" would be helpful.
>
> >     ("id",            string),
> >     ("vm_state_size", uint64),
> >     ("date_sec",      uint64),
> >     ("date_nsec",     uint64),
> >     ("vm_clock_sec",  uint64),
> >     ("vm_clock_nsec", uint64),
> >
>
> A lot of these are read-only and wouldn't be provided when calling
> libxl_snapshot_create(). I think adding the domain state at time of
> snapshot would be useful.
do you mean the stopped or running state for a domain?
> There are probably other items I'm not
> considering atm, which makes me nervous about future extensibility.
so, how about this?
 libxl_vm_snapshot = Struct("vm_snapshot",[
     ("name",          string),
     ("creation_time", uint64),
     ("save",          string),
+    ("state",         string),
+    ("disks", Array(libxl_snapshot, "num_disks")),
     ])

BTW: maybe i should rename libxl_snapshot to libxl_disk_snapshot?

>
> > (2), libxl_vm_snapshot store vm snapshot information which store in the path
> > shown above. i add some api for create, delete and list these information.
> > at first, i want to add these information to xenstore, but it will lose when
> > xenstore reboot or dom0 reboot.
> >
> > libxl_vm_snapshot = Struct("vm_snapshot",[
> >     ("name",          string),
> >     ("creation_time", uint64),
> >     ("save",          string),
> >     ])
> >
> > 2), new api
> > (1), in libxl/libxl.h
> > /* disk snapshot api
> >  * support create, delete and list for internal snapshot of a single disk
> >  * only support internal snapshot rigt now.
> >  */
> > /* create disk snapshot according to the device name in snapshot array. nb is
> >  * the number of snapshot array.
> >  * use the qmp transaction to ensure all snapshot of disk is coherence.
> >  */
> > int libxl_disk_snapshot_create(libxl_ctx *ctx, int domid,
> >                                libxl_snapshot *snapshot, int nb);
> > /* delete number of nb disk snapshot describe in snapshot array
> >  */
> > int libxl_disk_snapshot_delete(libxl_ctx *ctx, int domid,
> >                                libxl_snapshot *snapshot, int nb);
> > int libxl__disk_snapshot_delete(libxl_ctx *ctx, int domid,
> >                                 libxl_snapshot *snapshot);
> > /* apply the disk snapshot by qemu-img command
> >  */
> > int libxl_disk_snapshot_apply(libxl_ctx *ctx, int domid,
> >                               libxl_snapshot *snapshot, int nb);
> > /* list disk snapshot by qmp query-block command
> >  */
> > int libxl__disk_snapshot_list(libxl_ctx *ctx, int domid,
> >                               libxl_snapshot **snapshotp);
> >
> > /* vm snapshot api
> >  */
> > /* write vm snapshot information to file
> >  */
> > int libxl_vm_snapshot_add(libxl_ctx *ctx, uint32_t domid,
> >                           libxl_vm_snapshot *snapshot,
> >                           const libxl_asyncop_how *ao_how)
> >                           LIBXL_EXTERNAL_CALLERS_ONLY;
> > /* delete vm snapshot information file
> >  */
> > int libxl_vm_snapshot_remove(libxl_ctx *ctx, uint32_t domid,
> >                              libxl_vm_snapshot *snapshot,
> >                              const libxl_asyncop_how *ao_how)
> >                              LIBXL_EXTERNAL_CALLERS_ONLY;
> > /* list vm snapshot from file
> >  */
> > libxl_vm_snapshot *libxl_vm_snapshot_list(libxl_ctx *ctx, uint32_t domid,
> >                                           int *num)
> >                                           LIBXL_EXTERNAL_CALLERS_ONLY;
> > /* read vm snapshot information from dedicated file
> >  */
> > int libxl_vm_snapshot_getinfo(libxl_ctx *ctx, uint32_t domid,
> >                               libxl_vm_snapshot *snapshot);
> > /* delete vm save image */
> > int libxl_vm_snapshot_delete_save_image(libxl_ctx *ctx,
> >                                         libxl_vm_snapshot *snapshot);
> >
>
> IMO, these too should be combined into libxl_snapshot_*(), with disk vs
> domain snapshot controlled via a flags argument.
maybe the function name is not clear. i will call the above function in the
libxl_snapshot_create(), like this:
libxl_snapshot_create()
{
    //save memory
    save_domain
    //get disk list
    get_disk
    //create disk snapshot according to disk list
    libxl__disk_snapshot_create
    //store snapshot information to file, maybe libxl_snapshot_store is more
    //clear?
    libxl_snapshot_add
    //unpause
    libxl_domain_unpause
}

but i have some problems while i write libxl_snapshot_* apis.
firstly, there are different routines about vm save/restore because in libvirt
the driver need to interact between libvirtd.
secondly, i want to reuse the save_domain, create_domain in xl_cmdimpl.c.
so, it is hard to me to provided the domain snapshot api directly. if i only
provide some sub-command for domain snapshot, it just like
libxl_domain_suspend in save_doman or libxl_domain_new in create_domain, it
might be easier for libvirt driver to implement snapshot.
is it make sense? or is there a better way to provide the domain snapshot api?

>
> > (2), libxl/xl_cmdimpl.c
> > /* get disk device name (hda, hdc..) and image path through
> >  * libxl_device_disk_list and libxl_device_disk_getinfo
> >  */
> > static int get_disk(uint32_t domid, libxl_snapshot **snapshotp, char *name);
> >
> > 3, question
> > 1), how to do disk snapshot apply?
> > when i apply the vm snapshot, i need to revert the disk snapshot. in qemu, it
> > is
> > done by bdrv_snapshot_goto api. this api is only called by loadvm hmp or
> > qemu-img commands.
> > there is no hmp api in libxl. so the only choice is using qemu-img command.
> > i know usually i should call qmp for qemu operation. but there is not qmp at
> > the moment, could i call qemu-img in libxl for disk snapshot apply?
> >
>
> Similar to the libvirt qemu driver, is it possible to use both qmp and
> hmp monitors in libxl?
after check the qemu code, i found that there is a qmp command
(human-monitor-command) could send hmp through qmp. so i could use loadvm and
savevm in xen.
loadvm will restore vm state and disk snapshot. i could call savevm for domain
snapshot in order to provide the qemu save file for loadvm.
for the snapshot with external snapshot, i will call save_domain for memory
save and qmp transaction for disk snapshot.
as a result, similar to libvirt, create support external disk snapshot. but
revert do not support.
and i will call different vm memory save function: qemu_save_device_state and
qemu_savevm_state depends on.

                                create        revert      mem save api
without external disk snasphot  savevm        loadvm      qemu_save_device_state

with external disk snasphot     save_domain   no support  qemu_savevm_state
                                +transaction

is it make sense?


regards

bamvor
>
> Regards,
> Jim