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

Re: [libvirt] [PATCH 1/4] snapshot: add two more filter sets to API



On 11/13/12 20:09, Eric Blake wrote:
As we enable more modes of snapshot creation, it becomes more important
to be able to quickly filter based on snapshot properties.  This patch
introduces new filter flags; subsequent patches will introduce virsh
back-compat filtering, as well as actual libvirt filtering.

* include/libvirt/libvirt.h.in (virDomainSnapshotListFlags): Add
five new flags in two new groups.
* src/libvirt.c (virDomainSnapshotNum, virDomainSnapshotListNames)
(virDomainListAllSnapshots, virDomainSnapshotNumChildren)
(virDomainSnapshotListChildrenNames)
(virDomainSnapshotListAllChildren): Document them.
* src/conf/snapshot_conf.h (VIR_DOMAIN_SNAPSHOT_FILTERS_*): Add
new convenience filter collection macros.
* tools/virsh-snapshot.c (cmdSnapshotList): Add 5 new flags.
* tools/virsh.pod (snapshot-list): Document them.
---
  include/libvirt/libvirt.h.in | 19 ++++++++++++++
  src/conf/snapshot_conf.h     |  9 +++++++
  src/libvirt.c                | 60 ++++++++++++++++++++++++++++++++++++++++++++
  tools/virsh-snapshot.c       | 44 +++++++++++++++++++-------------
  tools/virsh.pod              | 17 ++++++++++++-
  5 files changed, 130 insertions(+), 19 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 49a361a..75c7c11 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -3818,6 +3818,25 @@ typedef enum {
                                                          which have metadata */
      VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA = (1 << 4), /* Filter by snapshots
                                                          with no metadata */
+
+    VIR_DOMAIN_SNAPSHOT_LIST_OFFLINE     = (1 << 5), /* Filter by snapshots
+                                                        taken while guest was
+                                                        offline */
+    VIR_DOMAIN_SNAPSHOT_LIST_ONLINE      = (1 << 6), /* Filter by snapshots
+                                                        taken while guest was
+                                                        online, and with
+                                                        memory state */

I'm wondering if we shouldn't go for ACTIVE/INACTIVE here?

+    VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY   = (1 << 7), /* Filter by snapshots
+                                                        taken while guest was
+                                                        online, but without
+                                                        memory state */
+
+    VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL    = (1 << 8), /* Filter by snapshots
+                                                        stored internal to
+                                                        disk images */
+    VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL    = (1 << 9), /* Filter by snapshots
+                                                        that use files external
+                                                        to disk images */
  } virDomainSnapshotListFlags;

  /* Return the number of snapshots for this domain */

Otherwise looks okay so I'm comfortable with ACKing this when the naming question gets settled.

Peter


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