5:20 p.m.
On 11/14/2012 11:43 AM, Eric Blake wrote:
> Bleah. Raw XML parsing. Wouldn't it be easier in and cleaner
convert
> this piece code to use the XML parser and xpath?
Not the first time we've done this. I agree that using the XML parser
and xpath is probably nicer, but it actually takes more code than a
simple strstr.
> The code looks OK in what it should be doing, but I don't like the raw
> XML parse-hacking stuff. The only reason to put this in as-is would be
> if it would be really complicated/overheading to use xpath here.
I'll post an interdiff that shows what it would take to use xpath, and
we can decide based on how nice or ugly it looks.
Here's the diff; any decisions on whether to go with xpath?
tools/virsh-snapshot.c | 61
+++++++++++++++++++++++++++++++++-----------------
1 file changed, 40 insertions(+), 21 deletions(-)
diff --git i/tools/virsh-snapshot.c w/tools/virsh-snapshot.c
index e38ce84..36f5b46 100644
--- i/tools/virsh-snapshot.c
+++ w/tools/virsh-snapshot.c
@@ -797,8 +797,10 @@ cmdSnapshotInfo(vshControl *ctl, const vshCmd *cmd)
virDomainSnapshotPtr snapshot = NULL;
const char *name;
char *doc = NULL;
- char *state;
- bool internal;
+ xmlDocPtr xmldoc = NULL;
+ xmlXPathContextPtr ctxt = NULL;
+ char *state = NULL;
+ int external;
char *parent = NULL;
bool ret = false;
int count;
@@ -840,39 +842,48 @@ cmdSnapshotInfo(vshControl *ctl, const vshCmd *cmd)
if (!doc)
goto cleanup;
- state = strstr(doc, "<state>");
+ xmldoc = virXMLParseStringCtxt(doc, _("(domain_snapshot)"), &ctxt);
+ if (!xmldoc)
+ goto cleanup;
+
+ state = virXPathString("string(/domainsnapshot/state)", ctxt);
if (!state) {
vshError(ctl, "%s",
_("unexpected problem reading snapshot xml"));
goto cleanup;
}
- state += strlen("<state>");
- vshPrint(ctl, "%-15s %.*s\n", _("State:"),
- (int) (strchr(state, '<') - state), state);
+ vshPrint(ctl, "%-15s %s\n", _("State:"), state);
/* In addition to state, location is useful. If the snapshot has
* a <memory> element, then the existence of snapshot='external'
* prior to <domain> is the deciding factor; for snapshots
* created prior to 1.0.1, a state of disk-only is the only
* external snapshot. */
- if (strstr(state, "<memory")) {
- char *domain = strstr(state, "<domain");
+ switch (virXPathBoolean("boolean(/domainsnapshot/memory)", ctxt)) {
+ case 1:
+ external =
virXPathBoolean("boolean(/domainsnapshot/memory/@snapshot=external "
+ "|
/domainsnapshot/disks/disk/@snapshot=external)",
+ ctxt);
+ break;
+ case 0:
+ external = STREQ(state, "disk-snapshot");
+ break;
+ default:
+ external = -1;
+ break;
- if (!domain) {
- vshError(ctl, "%s",
- _("unexpected problem reading snapshot xml"));
- goto cleanup;
- }
- internal = !memmem(state, domain - state,
"snapshot='external'",
- strlen("snapshot='external'"));
- } else {
- internal = !STRPREFIX(state, "disk-snapshot");
+ }
+ if (external < 0) {
+ vshError(ctl, "%s",
+ _("unexpected problem reading snapshot xml"));
+ goto cleanup;
}
vshPrint(ctl, "%-15s %s\n", _("Location:"),
- internal ? _("internal") : _("external"));
+ external ? _("external") : _("internal"));
- if (vshGetSnapshotParent(ctl, snapshot, &parent) < 0)
- goto cleanup;
+ /* Since we already have the XML, there's no need to call
+ * virDomainSnapshotGetParent */
+ parent = virXPathString("string(/domainsnapshot/parent/name)", ctxt);
vshPrint(ctl, "%-15s %s\n", _("Parent:"), parent ? parent :
"-");
/* Children, Descendants. After this point, the fallback to
@@ -884,8 +895,13 @@ cmdSnapshotInfo(vshControl *ctl, const vshCmd *cmd)
}
flags = 0;
count = virDomainSnapshotNumChildren(snapshot, flags);
- if (count < 0)
+ if (count < 0) {
+ if (last_error->code == VIR_ERR_NO_SUPPORT) {
+ vshResetLibvirtError();
+ ret = true;
+ }
goto cleanup;
+ }
vshPrint(ctl, "%-15s %d\n", _("Children:"), count);
flags = VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
count = virDomainSnapshotNumChildren(snapshot, flags);
@@ -908,6 +924,9 @@ cmdSnapshotInfo(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
+ VIR_FREE(state);
+ xmlXPathFreeContext(ctxt);
+ xmlFreeDoc(xmldoc);
VIR_FREE(doc);
VIR_FREE(parent);
if (snapshot)
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org