Re: [RFC v3] domain snapshot documents

[Jim Fehlig <jfehlig@suse.com>]

Bamvor Jian Zhang wrote:
> Hi, David
>   
>> On Fri, May 16, 2014 at 6:00 PM, Bamvor Jian Zhang <bjzhang@suse.com> wrote:
>>
>>     
>>> Hi, david
>>>
>>>       
>>>> On Thu, May 15, 2014 at 4:58 PM, Bamvor Jian Zhang <bjzhang@suse.com>
>>>>         
>>> 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.

Regards,
Jim