From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Bamvor Jian Zhang" Subject: Re: [RFC v2] vm snapshot documents Date: Wed, 07 May 2014 03:10:32 -0600 Message-ID: <536A690802000030000320F7@soto.provo.novell.com> References: <53516A92020000300002DA52@soto.provo.novell.com> <53573407.3090609@suse.com> <535803DE020000300002E87B@soto.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: David kiarie Cc: Zhiqiang Zhou , Ian Campbell , Ian Jackson , Chun Yan Liu , xen-devel@lists.xen.org, Jim Fehlig , Anthony.perard@citrix.com List-Id: xen-devel@lists.xenproject.org Hi, david >>>David kiarie wrote: > On Wed, Apr 23, 2014 at 1:18 PM, Bamvor Jian Zhang wrote: > > Hi, > > > > >>>Jim Fehlig 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. ... > >> > 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. > > Yes, I also think so. > I was also thinking that one struct could be used to represent all > snapshot(vm and disk) information. > We should have some flags in the struct indicating the contents of the > structure whether disk_snapshot,vm_snapshot, or system checkpoint.This > can be implemented using enums or a bit mask. > > That done,we can libxl_snapshot_*() functions controlled by a flags > argument for creation,deletion,listing,reverting and deleting except the id/name in vm, disk, there is no common information for vm and disk. i am not sure how do you want to do, maybe you could give a example? regardsa bamvor > > 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? > > > >>