Re: [Qemu-devel] [PATCH 16/18] qapi: Convert info snapshots

[Pavel Hrdina <phrdina@redhat.com>]

On 08/16/2012 06:36 AM, Eric Blake wrote:
> On 08/15/2012 01:41 AM, Pavel Hrdina wrote:
>> Signed-off-by: Pavel Hrdina<phrdina@redhat.com>
>> ---
>>   hmp.c            | 33 +++++++++++++++++++++++++++++++++
>>   hmp.h            |  1 +
>>   monitor.c        |  2 +-
>>   qapi-schema.json | 34 ++++++++++++++++++++++++++++++++++
>>   qmp-commands.hx  | 30 ++++++++++++++++++++++++++++++
>>   savevm.c         | 51 ++++++++++++++++++++++++---------------------------
>>   sysemu.h         |  2 --
>>   7 files changed, 123 insertions(+), 30 deletions(-)
>>
>> @@ -1053,6 +1053,40 @@
>>   { 'command': 'query-block-jobs', 'returns': ['BlockJobInfo'] }
>>   
>>   ##
>> +# @SnapshotInfo:
>> +#
> We've got competition here:
> https://lists.gnu.org/archive/html/qemu-devel/2012-08/msg02961.html
>
> These two patches need to converge into a single design.
I can use his design.
>> +# Snapshot list.  This structure contains list of snapshots for virtual machine.
>> +#
>> +# @id: id of the snapshot.
>> +#
>> +# @tag: human readable tag of the snapshot.
>> +#
>> +# @vm_size: size of the snapshot in Bytes.
> As this is a new QMP command, you should use '-', not '_'.  Benoit's
> patch named this @vm-state-size.
>
>> +#
>> +# @date: date and time of the snapshot as unix timestamp.
>> +#
>> +# @vm_clock: time in the guest in nsecs.
> For online internal snapshots (those created by savevm), the vm clock
> offset makes sense. But for offline snapshots (those created by
> 'qemu-img snapshot', with no VM state), there is no vm clock offset.
> 'qemu-img info' lists 00:00:00.000 as a result, but I wonder if it would
> be better to leave this field (and vm-state-size) as #optional and omit
> them from the JSON for offline snapshots.
>
> And since qemu refuses to load offline snapshots, it might be worth an
> additional field in the JSON that says whether a snapshot is online or
> offline, to make it easier for the parsing application to determine
> whether it can be loaded or must go through qemu-img while the guest is
> offline (then again, keeping vm-state-size as non-optional, and checking
> for 0 size, somewhat covers this).
As you said, this is covered and I think that checking for 0 size is 
good enough.
>> +#
>> +# Since:  1.2
> 1.3 (entire series...)
>