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

["Bamvor Jian Zhang" <bjzhang@xxxxxxxx>]

Hi, Chunyan

> >>> On 6/23/2014 at 07:25 PM, in message
> <1403522755-6894-2-git-send-email-bjzhang@xxxxxxxx>, Bamvor Jian Zhang
> <bjzhang@xxxxxxxx> 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@xxxxxxxx>
> > ---
> >  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



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