[libvirt] [PATCH 1/5] snapshot: define new API virDomainSnapshotDeleteByName

[Guannan Ren]

On 06/21/2013 05:55 PM, Ján Tomko wrote:
> On 06/13/2013 02:55 PM, Guannan Ren wrote:
>> This is to delete a snapshot object atomically, the API accepts
>> argments: domain object, snapshot name and flags.
>> Add a new flag VIR_DOMAIN_SNAPSHOT_DELETE_CURRENT, others are same.
>>
>> include/libvirt/libvirt.h.in: Declare virDomainSnapshotDeleteByName
>>                                Add VIR_DOMAIN_SNAPSHOT_DELETE_CURRENT
>> src/driver.h: (virDrvDomainSnapshotDeleteByName)
>> src/libvirt.c: Implement the public API
>> src/libvirt_public.syms: Export the symbol to public
>> ---
>>   include/libvirt/libvirt.h.in |  5 +++
>>   src/driver.h                 |  6 ++++
>>   src/libvirt.c                | 73 ++++++++++++++++++++++++++++++++++++++++++++
>>   src/libvirt_public.syms      |  5 +++
>>   4 files changed, 89 insertions(+)
>>
>> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
>> index 574b970..fa90197 100644
>> --- a/include/libvirt/libvirt.h.in
>> +++ b/include/libvirt/libvirt.h.in
>> @@ -4339,11 +4339,16 @@ typedef enum {
>>       VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN      = (1 << 0), /* Also delete children */
>>       VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY = (1 << 1), /* Delete just metadata */
>>       VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY = (1 << 2), /* Delete just children */
>> +    VIR_DOMAIN_SNAPSHOT_DELETE_CURRENT       = (1 << 3), /* Delete current snapshot */
>>   } virDomainSnapshotDeleteFlags;
> This flag doesn't specify how to delete a snapshot, it says which one to
> delete. These flags are shared with virDomainSnapshotDelete, but both Delete
> and DeleteByName would do the same thing with this flag (if you implemented it
> in the first one too).
>
> I think it would be nicer to assume SnapshotDeleteByName with a NULL name
> means current snapshot.

     Sorry, it is so late to respond.
     I agree what you pointed out above, the DELETE_CURRENT didn't make 
itself clear.
     For SnapshotDeleteByNam(), it accepts NULL as name, so it could be 
simpler to represent current snapshot object.
     I will change it in v2.

     Thanks.
     Guannan