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

[Peter Krempa]

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