Re: [Xen-devel] [RFC v2] vm snapshot documents

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

Hi, david

 >>>David kiarie <davidkiarie4@xxxxxxxxx> wrote: 
> On Wed, Apr 23, 2014 at 1:18 PM, Bamvor Jian Zhang <bjzhang@xxxxxxxx> wrote: 
> > Hi, 
> > 
> >  >>>Jim Fehlig <jfehlig@xxxxxxxx> 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? 
> > 
> >> 


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