Re: [Xen-devel] [RFC v3] domain snapshot documents

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

Hi, Jim
> Bamvor Jian Zhang wrote:
> > Hi, David
> >
> >> On Fri, May 16, 2014 at 6:00 PM, Bamvor Jian Zhang <bjzhang@xxxxxxxx> 
> >> wrote:
> >>
> >>
> >>> Hi, david
> >>>
> >>>
> >>>> On Thu, May 15, 2014 at 4:58 PM, Bamvor Jian Zhang <bjzhang@xxxxxxxx>
> >>>>
> >>> wrote:
> >>>
> >>>>> Hi,
> >>>>>
> >>>>> here is the third version about domain snapshot documents, the second
> >>>>> version
> >>>>> and the first version is here[1][2].
> >>>>>
> >>> ...
> >>>
> >>>>> 2, new functions
> >>>>> there is no common api like libxl_snapshot_xxx. the reason is that
> >>>>> different
> >>>>> toolstack may need to different event handling machanism(synchronize or
> >>>>> asynchronize). and obviously, domain snapshot create need async
> >>>>>
> >>> handler.
> >>>
> >>>>> so i
> >>>>> decide to only provide the sub api for xl and other toolstack(e.g.
> >>>>> libvirt).
> >>>>> it make eailer for toolstack to handle the event by themselves.
> >>>>>
> >>>>> 1), in libxl/libxl.h
> >>>>> the implementation will be located in libxl_snapshot.c
> >>>>> /* disk snapshot api
> >>>>>  * support create for external and internal disks, support delete for
> >>>>> internal
> >>>>>  * snapshot of disks.
> >>>>>  */
> >>>>> /* 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_disk_snapshot *snapshot, int nb,
> >>>>>                                const libxl_asyncop_how *ao_how);
> >>>>> /* delete number of nb disk snapshot describe in snapshot array
> >>>>>  */
> >>>>> int libxl_disk_snapshot_delete(libxl_ctx *ctx, int domid,
> >>>>>                                libxl_disk_snapshot *snapshot, int nb);
> >>>>>
> >> Are this the only two functions you are exposing?I mean the API?or am I
> >> getting something wrong?
> >>
> >>
> > there are functions libxl_qmp_loadvm, libxl_qmp_savevm which is called by
> > domain internal snapshot create/revert. currently, they are internal
> > functions.
> > (libxl_intenal.h). but i should expose them, e.g. look like:
> > int libxl_loadvm(libxl_ctx *ctx, int domid, libxl_domain_snapshot *snapshot,
> >                  const libxl_asyncop_how *ao_how);
> > int libxl_savevm(libxl_ctx *ctx, int domid, libxl_domain_snapshot *snapshot,
> >                  const libxl_asyncop_how *ao_how);
> >
>
> IMO, would be better to have libxl_disk_snapshot_create() and
> libxl_disk_snapshot_delete() exposed, and keep these as internal.
i am not sure if i could follow you. i guess you mean we should expose unique
api for dedicate operations. this is what i want.
but the issue is there is no single revert disk snapshot qmp or hmp in qemu.
so, in libvirt qemu driver, internal or external disk snapshot call
loadvm/savevm/delvm and transaction/blockdev-snapshot-sync/
blockdev-snapshot-internal-sync respectively.
IMO, it will be better if qemu support blockdev snapshot revert qmp which expose
bdrv_snapshot_goto to qemu user(e.g. libxl).
i do not know how qemu upstream think about it.  how about discuss it later
when basic snapshot function accept in libxl?

regards

bamvor

>
> Regards,
> Jim
>



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