Re: [Xen-devel] [RFC V9 4/4] domain snapshot design: libxl/libxlu

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

>>> On 12/18/2014 at 11:27 PM, in message 
>>> <1418916436.11882.101.camel@xxxxxxxxxx>,
Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote: 
> On Tue, 2014-12-16 at 14:32 +0800, Chunyan Liu wrote: 
> > Changes to V8: 
> >   * remove libxl_domain_snapshot_create/delete/revert API 
> >   * export disk snapshot functionality for both xl and libvirt usage 
> >  
> > =========================================================================== 
> > Libxl/libxlu Design 
> >  
> > 1. New Structures 
> >  
> > libxl_disk_snapshot = Struct("disk_snapshot",[ 
> >     # target disk 
> >     ("disk",            libxl_device_disk), 
> >  
> >     # disk snapshot name 
> >     ("name",            string), 
> >  
> >     # internal/external disk snapshot? 
> >     ("external",        bool), 
> >  
> >     # for external disk snapshot, specify following two field 
> >     ("external_format", string), 
> >     ("external_path",   string), 
>  
> Should this be a KeyedUnion over a new LIBXL_DISK_SNAPSHOT_KIND enum 
> (with values INTERNAL and EXTERNAL)?
The KeyedUnion seems to be unnecessary. Only EXTERNAL has data items,
INTERNAL doesn't, and no third types.

> This would automatically make the 
> binding between external==true and the fields which depend on that. 
>  
> external_format should be of type libxl_disk_format, unless it is 
> referring to something else? 

Yes. That's right. I'll update.

>  
> Is it possible for format to differ from the format of the underlying 
> disk? Perhaps taking a snapshot of a raw disk as a qcow?

This is related to implementation details. As I understand qemu's
implementation, taking an external disk snapshot is actually a way:
origin domain disk: a raw disk
external= true, external_format: qcow2, external_path: test
a), create a qcow2 file (test.qcow2) with  backing file (the raw disk)
b), replace domain disk, now domain uses test.qcow2 (the raw disk
     is actually to be the snapshot)

So, I think the external_format can only be those supporting backing file.

> In any case 
> passing in UNKNOWN and letting libxl choose (probably by picking the 
> same as the underlying disk) should be supported.

If external_format is not passed (NULL), by default, we will use qcow2.

>  
> > /*  This API might not be used by xl, since xl won't take care of deleting 
> >  *  snapshots. But for libvirt, since libvirt manages snapshots and will 
> >  *  delete snapshot, this API will be used. 
> >  */ 
> > int libxl_disk_snapshot_delete(libxl_ctx *ctx, uint32_t domid, 
> >                                libxl_disk_snapshot *snapshot, int nb); 
>  
> The three usecases I mentioned in the previous mail are important here, 
> because depending on which usecases you are considering there maybe a 
> many to one relationship between domains and a given snapshot (gold 
> image case). This interface cannot support that I think.

I'm not quite clear about the three usecases, especially the 3rd usercase,
so really not sure what's the requirement towards deleting disk snapshot.
 
>  
> When we discussed this in previous iterations I suggested a libxl 
> command to tell a VM that it needed to reexamine its disks to see if any 
> of the chains had changed. I'm sure that's not the only potential answer 
> though.
 
About delete disk snapshot in a snapshot chain, whether we need to do
extra work to avoid data break, it can be discussed:
a). For external snapshots, usually it's based on backing file chain, qemu
does this, vhd-util does this. In this case, to delete a domain snapshot,
one doesn't need to do anything to disk (no need to delete disk snapshot
at all). Downside is, there might be a long backing chain.
b). For internal snapshot, like qcow2, lvm too. For lvm, it doesn't support
snapshot of snapshot, so out of scope. For qcow2, delete any disk snapshot
won't affect others.

>  
> > int libxl_disk_to_snapshot(libxl_ctx *ctx, uint32_t domid, 
> >                            libxl_disk_snapshot **snapshot, int *num); 
> >  
> >     This is for domain snapshot create. If user doesn't specify disks, 
> >     then by default it will take internal disk snapshot to each domain 
> >     disk. This function will fill libxl_disk_snapshot according to domain 
> >     disks info. 
>  
> Is this just a helper to produce an array to pass to 
> libxl_disk_snapshot_create? Or does it actually do stuff? 
>  
> I think it's the former, but it could be clarified.

Yes, the former.

> I *think* this is 
> just a special case of libxl_device_disk_list which returns plausible 
> snapshot objects instead of the disks themselves. 

So we prefer adding codes to libxl_device_disk_list rather than adding
a new API, right?

> >  
> > For disk snapshot revert, no qmp command for that, it always calls 
> > external commands to finish the work, so put in libxlu (?): 
>  
> I think rather than "no qmp" the issue is that "revert" is (at least as 
> far as libxl knows) essentially, destroy, rollback disks, restore from 
> RAM snapshot. So there is no qemu to speak to during the rollback. Is 
> that right? 

Yes, that's right.

>  
> Ian. 
>  
>  
>  


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