Re: [libvirt] [PATCHv2 2/4] snapshot: add virsh back-compat support for new filters

Snapshot filtering based on types is useful enough to add back-compat support into virsh. It is also rather easy - all versions of libvirt that don't understand the new filter flags already gave us sufficient information in a single XML field to reconstruct all the information we need (that is, it isn't until libvirt 1.0.1 that we have more interesting types of snapshots, such as offline external). * tools/virsh-snapshot.c (vshSnapshotFilter): New function. (vshSnapshotListCollect): Add fallback support. --- tools/virsh-snapshot.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 110 insertions(+), 1 deletion(-) diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 0363e19..7cd2966 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -39,6 +39,7 @@ #include "util.h" #include "virsh-domain.h" #include "xml.h" +#include "conf/snapshot_conf.h" /* Helper for snapshot-create and snapshot-create-as */ static bool @@ -712,6 +713,67 @@ cleanup: return ret; } +/* Helper function to filter snapshots according to status and + * location portion of flags. Returns 0 if filter excluded snapshot + * (or if snapshot is already NULL), 1 if snapshot is okay, and -1 on + * failure with error reported. */ +static int +vshSnapshotFilter(vshControl *ctl, virDomainSnapshotPtr snapshot, + unsigned int flags) +{ + char *xml = NULL; + xmlDocPtr xmldoc = NULL; + xmlXPathContextPtr ctxt = NULL; + int ret = -1; + char *state = NULL; + + if (!snapshot) + return 0; + + xml = virDomainSnapshotGetXMLDesc(snapshot, 0); + if (!xml) + goto cleanup; + + xmldoc = virXMLParseStringCtxt(xml, _("(domain_snapshot)"), &ctxt); + if (!xmldoc) + goto cleanup; + + /* Libvirt 1.0.1 and newer never call this function, because the + * filtering is already supported by the listing functions. Older + * libvirt lacked /domainsnapshot/memory, but was also limited in + * the types of snapshots it could create: if state was disk-only, + * the snapshot is external; all other snapshots are internal. */ + state = virXPathString("string(/domainsnapshot/state)", ctxt); + if (!state) + goto cleanup; + if (STREQ(state, "disk-snapshot")) { + ret = ((flags & (VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY | + VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL)) == + (VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY | + VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL)); + } else { + if (!(flags & VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL)) + ret = 0; + else if (STREQ(state, "shutoff")) + ret = !!(flags & VIR_DOMAIN_SNAPSHOT_LIST_INACTIVE); + else + ret = !!(flags & VIR_DOMAIN_SNAPSHOT_LIST_ACTIVE); + } + +cleanup: + if (ret < 0) { + vshReportError(ctl); + vshError(ctl, "%s", _("unable to perform snapshot filtering"));

+ } else { + vshResetLibvirtError(); + } + VIR_FREE(state); + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xmldoc); + VIR_FREE(xml); + return ret; +} +