Re: [Xen-devel] [RFC V6] Libxl Domain Snapshot API Design

["Chun Yan Liu" <cyliu@xxxxxxxx>]

>>> On 9/5/2014 at 08:44 PM, in message
<1409921073.10156.82.camel@xxxxxxxxxxxxxxxxxxxxxx>, Ian Campbell
<Ian.Campbell@xxxxxxxxxx> wrote: 
> On Wed, 2014-08-27 at 15:22 +0800, Chunyan Liu wrote: 
> > Since Bamvor left SUSE and turns to work on ARM server, I'd like to  
> continue 
> > this work and make progress. Following the discussion about V5, which  
> mainly 
> > focused on the API design, here post the updated API design. Thanks for any 
> > of your further suggestions! 
> >  
> > Main changes to V5: 
> > * libxl_disk_snapshot: reuse libxl_device_disk rather than specify path, 
> >     format separately in the structure. Including two libxl_device_disk 
> >     components, one is to indicate the original disk info, one is to 
> >     indicate the external snapshot info if it is 'external snapshot'. 
> > * define common APIs for domain snapshot creating/deleting/reverting, 
> >   rather than a group of functions for disk snapshot operations. 
> > * remove those APIs for loading/storing/deleting snapshot config. 
>  
> Please could you say a few words on what a snapshot actually is (i.e. 
> what are its component parts), I think I know but it would be good to 
> make sure we are all on the same page. 
>  
> This might include a description of what "internal" vs "external" 
> snapshots are (see below). 

Thanks, I'll include the description part in next version.

>  
> >  
> > V5 is here: 
> > http://lists.xenproject.org/archives/html/xen-devel/2014-07/msg00893.html 
> > V5 about API Design is here: 
> > http://lists.xenproject.org/archives/html/xen-devel/2014-07/msg00897.html 
> >  
> > =========================================================================== 
> > Libxl Domain Snapshot API 
> >  
> > libxl_domain_snapshot = Struct("domain_snapshot",[ 
>  
> Please can you say which of these needs to be filled in by the caller of 
> libxl_domain_snapshot_create and which are filled in by that function 
> (i.e. which fields are inputs and which are outputs). 

Thanks, I'll update.

>  
> My guess is that most of these are inputs. 

Yes. This is quite like libxl_domain_config to libxl_domain_create.

Internally there may be another structure (could be
libxl_domain_snapshot_obj) to store domain snapshot info, including
these info, and parent, children info. 

>  
> >     ("name",          string),              /* snapshot name */
Input. non-NULL. Should be prepared by application.
 
> >     ("description",   string),              /* snapshot description */
Input. Could be NULL.

> >     ("creation_time", uint64),              /* creation time, in seconds */
Input. non-NULL.
 
>  
> Are these necessary at the libxl level? They seem like the sort of thing 
> the toolstack ought to be keeping as part of its overall snapshot 
> tracking. 

These are necessary information when showing snapshot info. Generally
it could be like creating domain, xl can list all domains created by xl or 
virsh,
xl snapshot-list could list domain snapshots created by xl or virsh too. To
show complete snapshot information, I think it's better to include these
at libxl level.

>  
> >     /* save memory or not. "false" means disk-only snapshot */ 
> >     ("memory",        bool);

Input. Should be filled.

> >  
> >     /* memory state file when snapshot is external */ 
>  
> Under what circumstances is the snapshot external? What is it external 
> to? 

To talk about internal snapshot and external snapshot, it's closely related
to disk snapshot:
Internal snapshot means disk snapshot info stored within the disk image
itself, it is possible to a qcow2 disk.
External snapshot means disk snapshot info stored in another file,
different from the original disk backend. After snapshot, it's like the concept
of backing file. To some disk backend type, like 'raw' format, only external
is supported.

Here, to memory state file, in design:
if memory state file is NULL, it means memory state is piggy-backed
with other internal disk state;
if memory state file is not NULL, it means 'external', there will be another
file holding the VM memory state.
I'm not sure if the former case is proper to xen, probably only 'external'
one is doable, like in 'xl save', it always stores the memory state in a
separate file.

>  
> >     ("memory_file",   string), 

Input. In design, could be NULL. But as mentioned above, in xen code,
to reuse current functions in libxl and libxc, probably always need a path.

> >  
> >     /* Array to store disk snapshot info. */ 
> >     ("disks", Array(libxl_disk_snapshot, "num_disks")), 
> >     ]) 
> >  
> > libxl_disk_snapshot = Struct("disk_snapshot",[ 
> >     ("disk",           libxl_device_disk),      /* orignal disk */ 
>  
> "original" 
>  
> >     ("name",           string),                 /* snapshot name */ 
> >     ("external",       bool),                   /* external snapshot or not 
> >  
> */ 
> >  
> >     /* external snapshot info, including file path and format, etc. 
> >      * if "external" is false, this will be "NULL". 
> >      */ 
> >     ("external_sn",    libxl_device_disk), 
>  
> What does the "sn" suffix stand for? 
'snapshot'.

This structure is talked about many times in last version. As input,
following information should be filled:
target device, like 'vda';  ('libxl_device_disk disk'should include that)
disk snapshot name;
external or internal;

if external, external type;
if external, external path;
(these two should be included in 'libxl_device_disk external_sn')

>  
> > int libxl_domain_snapshot_create(libxl_ctx *ctx, const char *domname, 
>  
> Should take a domid not a name, for consistency with all the other libxl 
> functions. 

There is one problem:
If domain is not active (not started), domain snapshot can also be done at
disk-only mode. But domid does not exist in this case.

>  
> >                                  libxl_domain_snapshot *snapshot, 
> >                                  unsigned int flags); 
> >  
> >     Creates a new snapshot of a domain based on the snapshot config  
> contained 
> >     in @snapshot. 
> >  
> >     If @flags includes LIBXL_DOMAIN_SNAPSHOT_CREATE_LIVE, then the domain  
> is not 
> >     paused while creating the snapshot, like live migration. This increases 
> >  
> size 
> >     of the memory dump file, but reducess downtime of the guest. Only  
> support 
> >     this flag during external checkpoints. 
> >  
> >     If @flags includes LIBXL_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY, then the  
> snapshot 
> >     will be limited to the disks described in @snapshot, and no VM state  
> will 
> >     be saved. For an active guest, this is not supported. 
>  
> Isn't this redundant with the ->memory  field of the snapshot object? 

Yes. Can be removed. Just to keep consistent with libvirt flags.

>  
> >     ctx: context 
> >     domname:  domain name 
> >     snapshot: configuration of domain snapshot 
> >     flags: bitwise-OR of libxlDomainSnapshotCreateFlags 
> >     Returns: 0 on success, -1 on failure 
> >  
> >  
> > int libxl_domain_snapshot_delete(libxl_ctx *ctx, const char *domname, 
>  
> Hrm, this suggests that libxl will have some mechanism for managing 
> snapshots, is that right? 
>  
> I don't think libxl should have that functionality since that is the 
> toolstack's responsibility to manage the snapshots once they are 
> created, using whatever means it likes. 

To delete internal domain snapshot, it needs to delete internal disk snapshot.
That should be done by calling qmp command. Libvirt libxl driver doesn't have
that ability. It needs to call libxl API to help doing that.

>  
> libxl should be providing the mechanisms ("take a snapshot and put it 
> here") but not the policies ("snapshots live in this directory, have 
> this lifecycle and this format"). 
>  
> Similar to how libxl provides a way to say "take these bits and present 
> them as a disk to the guest", but it leaves the management of image 
> files to the toolstack (or in the case of xl the actual user). 
>  
> >                                  const char *snapshot_name, 

Thanks for pointing out. 
This is not we expected. I realized the parameter here should not be
a snapshot-name, but libxl_domain_snapshot maybe. Application needs
to supply path info too as in libxl_domain_snapshot_create.

>  
> If this were a libxl_domain_snapshot object I could just about imagine 
> that this would be useful helper which just iterated over the files 
> referenced by the snapshot and removed them. I'm not sure how useful 
> that helper would be in practice though (depends on the toolstack's 
> actual requirements). 
>  
> >                                  unsigned int flags); 
> >  
> >     Delete a snapshot. 
> >  
> >     If @flags is 0, then just this snapshot is deleted, and changes from  
> this 
> >     snapshot are automatically merged into children snapshots. 
> >  
> >     If @flags includes LIBXL_DOMAIN_SNAPSHOT_DELETE_CHILDREN, then this  
> snapshot 
> >     and any descendant snapshots are deleted. 
>  
> This definitely sounds like toolstack level functionality to me. 
>
>  
> >     ctx: context 
> >     domname: domain name 
> >     snapshot_name: snapshot name 
> >     flags: bitwise-OR of supported libxlDomainSnapshotDeleteFlags 
> >     Returns: 0 on success, -1 on error. 
> >  
> > int libxl_disk_snapshot_revert(libxl_ctx *ctx, const char *domname, 
>  
> Should take a domid. 
>  
> >                                const char *snapshot_name, 
>  
> The input here should be a libxl_domain_snapshot object I think. 
> (Otherwise libxl would have to track/manage snapshot names)  

Yes. I realized that.

>  
> >                                unsigned int flags); 
> >  
> >     Revert the domain to a given snapshot. 
> >  
> >     Normally, the domain will revert to the same state the domain was in  
> while 
> >     the snapshot was taken (whether inactive, running, or paused). 
>  
> What is the distinction between inactive and paused? 

Inactive should mean not started at all.
Disk-only snapshot can still be created in this status .

>  
> >  
> >     If @flags includes LIBXL_DOMAIN_SNAPSHOT_REVERT_RUNNING, then overrides 
> >  
> the 
> >     snapshot state to guarantee a running domain after the revert. 
> >  
> >     If @flags includes LIBXL_DOMAIN_SNAPSHOT_REVERT_PAUSED, then guarantees 
> >  
> a 
> >     paused domain after the revert. 
> >  
> >     ctx: context 
> >     domname: domain name 
> >     snapshot_name: snapshot name 
> >     flags: bitwise-OR of supported libxlDomainSnapshotRevertFlags 
> >     Returns: 0 on success, -1 on error. 
>  
>  
>  
>  


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