On 11/13/2012 05:16 PM, Peter Krempa wrote:
On 11/13/12 20:09, Eric Blake wrote:
Now that we can filter on this information, we should also make it easy to get at.
* tools/virsh-snapshot.c (cmdSnapshotInfo): Add another output row. --- tools/virsh-snapshot.c | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-)
- tmp = strstr(doc, "<state>"); - if (!tmp) { + state = strstr(doc, "<state>"); + if (!state) { vshError(ctl, "%s", _("unexpected problem reading snapshot xml")); goto cleanup; } - tmp += strlen("<state>"); + state += strlen("<state>"); vshPrint(ctl, "%-15s %.*s\n", _("State:"), - (int) (strchr(tmp, '<') - tmp), tmp); + (int) (strchr(state, '<') - 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");
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. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org