[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

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.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]