On 11/15/12 00:36, Eric Blake wrote:
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"));
Hm, reporting of libvirt's error isn't really needed here. When the
filtering fails everything should fail gracefully and the error would be
printed by vshCmdRun at the end when the command fails. The only benefit
of this is that the error from libvirt is printed before the more
specific error. In other commands we just print a local error and then
let the libvirt error to be printed by the command handler.
+ } else {
+ vshResetLibvirtError();
+ }
+ VIR_FREE(state);
+ xmlXPathFreeContext(ctxt);
+ xmlFreeDoc(xmldoc);
+ VIR_FREE(xml);
+ return ret;
+}
+
ACK when you remove the error message printing or provide a explanation
why you chose that approach.
Peter
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
