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

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.