[libvirt] [PATCH 3/4] snapshot: expose location through virsh snapshot-info

[Eric Blake]

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 at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 617 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20121114/b5e51973/attachment-0001.sig>