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@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list