[V11,09/17] qmp: add interface query-snapshots

This interface returns info of valid internal snapshots for whole vm.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qapi.c     |   17 ++++++++++++++++
 qapi-schema.json |   14 +++++++++++++
 qmp-commands.hx  |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+), 0 deletions(-)

On Tue, Apr 02, 2013 at 07:47:22PM +0800, Wenchao Xia wrote:
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 1e0e11e..6b20684 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -1765,6 +1765,61 @@ EQMP
>      },
>  
>  SQMP
> +query-snapshots
> +---------------
> +
> +Show the internal consistent snapshot information
> +
> +Each snapshot is represented by a json-object. The returned value
> +is a json-array of all snapshots

I think the "consistent snapshot" term is not defined.  Please add
something like:

"Consistent snapshots are snapshots that exist in all writeable block
devices.  When 'savevm' takes a snapshot it uses the same ID across all
writeable block devices.  If a snapshot ID only exists in one block
device then it is not a consistent snapshot."

Stefan

Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:

>   This interface returns info of valid internal snapshots for whole vm.
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/qapi.c     |   17 ++++++++++++++++
>  qapi-schema.json |   14 +++++++++++++
>  qmp-commands.hx  |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 86 insertions(+), 0 deletions(-)
>
> diff --git a/block/qapi.c b/block/qapi.c
> index 6e0c7c3..5e91ab8 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -258,6 +258,23 @@ BlockInfo *bdrv_query_info(BlockDriverState *bs)
>      return info;
>  }
>  
> +SnapshotInfoList *qmp_query_snapshots(Error **errp)
> +{
> +    BlockDriverState *bs;
> +    SnapshotInfoList *list = NULL;
> +
> +    /* internal snapshot for whole vm */
> +    bs = bdrv_snapshots();
> +    if (!bs) {
> +        error_setg(errp, "No available block device supports snapshots\n");
> +        return NULL;
> +    }
> +
> +    bdrv_query_snapshot_info_list(bs, &list, true, errp);
> +
> +    return list;
> +}
> +
>  BlockInfoList *qmp_query_block(Error **errp)
>  {
>      BlockInfoList *head = NULL, **p_next = &head;
> diff --git a/qapi-schema.json b/qapi-schema.json
> index f629a24..225afef 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -841,6 +841,20 @@
>  { 'command': 'query-block', 'returns': ['BlockInfo'] }
>  
>  ##
> +# @query-snapshots:
> +#
> +# Get a list of internal snapshots for the whole virtual machine. Only valid
> +# internal snapshots will be returned, inconsistent ones will be ignored
> +#
> +# Returns: a list of @SnapshotInfo describing all consistent virtual machine
> +#          snapshots
> +#
> +# Since: 1.5
> +##
> +{ 'command': 'query-snapshots',
> +  'returns': ['SnapshotInfo'] }
> +
> +##
>  # @BlockDeviceStats:
>  #
>  # Statistics of a virtual block device or a block backing device.
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 1e0e11e..6b20684 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -1765,6 +1765,61 @@ EQMP
>      },
>  
>  SQMP
> +query-snapshots
> +---------------
> +
> +Show the internal consistent snapshot information
> +
> +Each snapshot is represented by a json-object. The returned value
> +is a json-array of all snapshots
> +
> +Each json-object contain the following:
> +
> +- "id": unique snapshot id (json-string)
> +- "name": internal snapshot name (json-string)
> +- "vm-state-size": size of the VM state in bytes (json-int)
> +- "date-sec": UTC date of the snapshot in seconds (json-int)
> +- "date-nsec": fractional part in nanoseconds to be used with
> +               date-sec(json-int)
> +- "vm-clock-sec": VM clock relative to boot in seconds (json-int)
> +- "vm-clock-nsec": fractional part in nanoseconds to be used with
> +                   vm-clock-sec (json-int)
> +
> +Example:
> +
> +-> { "execute": "query-snapshots" }
> +<- {
> +      "return":[
> +         {
> +            "id": "1",
> +            "name": "snapshot1",
> +            "vm-state-size": 0,
> +            "date-sec": 10000200,
> +            "date-nsec": 12,
> +            "vm-clock-sec": 206,
> +            "vm-clock-nsec": 30

Not your patch's fault, but here goes anyway: I dislike this
representation of time.

QMP has time in seconds, milliseconds, nanoseconds, (seconds,
milliseconds) and (seconds, nanoseconds).  There has been no adult
supervision, obviously (I may say that, because it's as much my fault as
it's anybody else's).

The sanest one by far is nanoseconds.  Good for 2^63 of them.  Since pi
seconds is a nanocentury, good for 2^63 / (pi * 1e9) centuries, which
should be safely beyond your retirement age.

> +         },
> +         {
> +            "id": "2",
> +            "name": "snapshot2",
> +            "vm-state-size": 34000000,
> +            "date-sec": 13000200,
> +            "date-nsec": 32,
> +            "vm-clock-sec": 406,
> +            "vm-clock-nsec": 31
> +         }
> +      ]
> +   }
> +
> +EQMP
> +
> +    {
> +        .name       = "query-snapshots",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_input_query_snapshots,
> +    },
> +
> +SQMP
>  query-blockstats
>  ----------------

>>   # Statistics of a virtual block device or a block backing device.
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index 1e0e11e..6b20684 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -1765,6 +1765,61 @@ EQMP
>>       },
>>   
>>   SQMP
>> +query-snapshots
>> +---------------
>> +
>> +Show the internal consistent snapshot information
>> +
>> +Each snapshot is represented by a json-object. The returned value
>> +is a json-array of all snapshots
>> +
>> +Each json-object contain the following:
>> +
>> +- "id": unique snapshot id (json-string)
>> +- "name": internal snapshot name (json-string)
>> +- "vm-state-size": size of the VM state in bytes (json-int)
>> +- "date-sec": UTC date of the snapshot in seconds (json-int)
>> +- "date-nsec": fractional part in nanoseconds to be used with
>> +               date-sec(json-int)
>> +- "vm-clock-sec": VM clock relative to boot in seconds (json-int)
>> +- "vm-clock-nsec": fractional part in nanoseconds to be used with
>> +                   vm-clock-sec (json-int)
>> +
>> +Example:
>> +
>> +-> { "execute": "query-snapshots" }
>> +<- {
>> +      "return":[
>> +         {
>> +            "id": "1",
>> +            "name": "snapshot1",
>> +            "vm-state-size": 0,
>> +            "date-sec": 10000200,
>> +            "date-nsec": 12,
>> +            "vm-clock-sec": 206,
>> +            "vm-clock-nsec": 30
> 
> Not your patch's fault, but here goes anyway: I dislike this
> representation of time.
> 
> QMP has time in seconds, milliseconds, nanoseconds, (seconds,
> milliseconds) and (seconds, nanoseconds).  There has been no adult
> supervision, obviously (I may say that, because it's as much my fault as
> it's anybody else's).
> 
> The sanest one by far is nanoseconds.  Good for 2^63 of them.  Since pi
> seconds is a nanocentury, good for 2^63 / (pi * 1e9) centuries, which
> should be safely beyond your retirement age.
> 
  OK, will insert a patch before removing "vm-clock-sec". Have one
question: how to declare uint64_t in qmp-schema.json?


>> +         },
>> +         {
>> +            "id": "2",
>> +            "name": "snapshot2",
>> +            "vm-state-size": 34000000,
>> +            "date-sec": 13000200,
>> +            "date-nsec": 32,
>> +            "vm-clock-sec": 406,
>> +            "vm-clock-nsec": 31
>> +         }
>> +      ]
>> +   }
>> +
>> +EQMP
>> +
>> +    {
>> +        .name       = "query-snapshots",
>> +        .args_type  = "",
>> +        .mhandler.cmd_new = qmp_marshal_input_query_snapshots,
>> +    },
>> +
>> +SQMP
>>   query-blockstats
>>   ----------------
>

Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:

>>>   # Statistics of a virtual block device or a block backing device.
>>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>>> index 1e0e11e..6b20684 100644
>>> --- a/qmp-commands.hx
>>> +++ b/qmp-commands.hx
>>> @@ -1765,6 +1765,61 @@ EQMP
>>>       },
>>>   
>>>   SQMP
>>> +query-snapshots
>>> +---------------
>>> +
>>> +Show the internal consistent snapshot information
>>> +
>>> +Each snapshot is represented by a json-object. The returned value
>>> +is a json-array of all snapshots
>>> +
>>> +Each json-object contain the following:
>>> +
>>> +- "id": unique snapshot id (json-string)
>>> +- "name": internal snapshot name (json-string)
>>> +- "vm-state-size": size of the VM state in bytes (json-int)
>>> +- "date-sec": UTC date of the snapshot in seconds (json-int)
>>> +- "date-nsec": fractional part in nanoseconds to be used with
>>> +               date-sec(json-int)
>>> +- "vm-clock-sec": VM clock relative to boot in seconds (json-int)
>>> +- "vm-clock-nsec": fractional part in nanoseconds to be used with
>>> +                   vm-clock-sec (json-int)
>>> +
>>> +Example:
>>> +
>>> +-> { "execute": "query-snapshots" }
>>> +<- {
>>> +      "return":[
>>> +         {
>>> +            "id": "1",
>>> +            "name": "snapshot1",
>>> +            "vm-state-size": 0,
>>> +            "date-sec": 10000200,
>>> +            "date-nsec": 12,
>>> +            "vm-clock-sec": 206,
>>> +            "vm-clock-nsec": 30
>> 
>> Not your patch's fault, but here goes anyway: I dislike this
>> representation of time.
>> 
>> QMP has time in seconds, milliseconds, nanoseconds, (seconds,
>> milliseconds) and (seconds, nanoseconds).  There has been no adult
>> supervision, obviously (I may say that, because it's as much my fault as
>> it's anybody else's).
>> 
>> The sanest one by far is nanoseconds.  Good for 2^63 of them.  Since pi
>> seconds is a nanocentury, good for 2^63 / (pi * 1e9) centuries, which
>> should be safely beyond your retirement age.
>> 
>   OK, will insert a patch before removing "vm-clock-sec".

Before you do that, let's get Luiz's blessing.

>                                                           Have one
> question: how to declare uint64_t in qmp-schema.json?

You can't.  All we got is 'int'.  The implementation restricts 'int'
values to 64 bits signed.  Plenty of range for this purpose.

[...]

On Thu, 11 Apr 2013 14:11:23 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:
> 
> >>>   # Statistics of a virtual block device or a block backing device.
> >>> diff --git a/qmp-commands.hx b/qmp-commands.hx
> >>> index 1e0e11e..6b20684 100644
> >>> --- a/qmp-commands.hx
> >>> +++ b/qmp-commands.hx
> >>> @@ -1765,6 +1765,61 @@ EQMP
> >>>       },
> >>>   
> >>>   SQMP
> >>> +query-snapshots
> >>> +---------------
> >>> +
> >>> +Show the internal consistent snapshot information
> >>> +
> >>> +Each snapshot is represented by a json-object. The returned value
> >>> +is a json-array of all snapshots
> >>> +
> >>> +Each json-object contain the following:
> >>> +
> >>> +- "id": unique snapshot id (json-string)
> >>> +- "name": internal snapshot name (json-string)
> >>> +- "vm-state-size": size of the VM state in bytes (json-int)
> >>> +- "date-sec": UTC date of the snapshot in seconds (json-int)
> >>> +- "date-nsec": fractional part in nanoseconds to be used with
> >>> +               date-sec(json-int)
> >>> +- "vm-clock-sec": VM clock relative to boot in seconds (json-int)
> >>> +- "vm-clock-nsec": fractional part in nanoseconds to be used with
> >>> +                   vm-clock-sec (json-int)
> >>> +
> >>> +Example:
> >>> +
> >>> +-> { "execute": "query-snapshots" }
> >>> +<- {
> >>> +      "return":[
> >>> +         {
> >>> +            "id": "1",
> >>> +            "name": "snapshot1",
> >>> +            "vm-state-size": 0,
> >>> +            "date-sec": 10000200,
> >>> +            "date-nsec": 12,
> >>> +            "vm-clock-sec": 206,
> >>> +            "vm-clock-nsec": 30
> >> 
> >> Not your patch's fault, but here goes anyway: I dislike this
> >> representation of time.
> >> 
> >> QMP has time in seconds, milliseconds, nanoseconds, (seconds,
> >> milliseconds) and (seconds, nanoseconds).  There has been no adult
> >> supervision, obviously (I may say that, because it's as much my fault as
> >> it's anybody else's).
> >> 
> >> The sanest one by far is nanoseconds.  Good for 2^63 of them.  Since pi
> >> seconds is a nanocentury, good for 2^63 / (pi * 1e9) centuries, which
> >> should be safely beyond your retirement age.
> >> 
> >   OK, will insert a patch before removing "vm-clock-sec".

Are you saying you're going to drop vm-clock-sec?

> Before you do that, let's get Luiz's blessing.

Fine with me if I got it right.

On 04/11/2013 06:44 AM, Luiz Capitulino wrote:

>>>>> +-> { "execute": "query-snapshots" }
>>>>> +<- {
>>>>> +      "return":[
>>>>> +         {
>>>>> +            "id": "1",
>>>>> +            "name": "snapshot1",
>>>>> +            "vm-state-size": 0,
>>>>> +            "date-sec": 10000200,
>>>>> +            "date-nsec": 12,
>>>>> +            "vm-clock-sec": 206,
>>>>> +            "vm-clock-nsec": 30
>>>>
>>>> Not your patch's fault, but here goes anyway: I dislike this
>>>> representation of time.
>>>>
>>>> QMP has time in seconds, milliseconds, nanoseconds, (seconds,

> Are you saying you're going to drop vm-clock-sec?
> 
>> Before you do that, let's get Luiz's blessing.
> 
> Fine with me if I got it right.

Problem.  Let's go back to the original definition that we are talking
about modifying:

+#
+# Since: 1.5
+##
+{ 'command': 'query-snapshots',
+  'returns': ['SnapshotInfo'] }
+

Now let's look for that type:

# @vm-clock-sec: VM clock relative to boot in seconds
#
# @vm-clock-nsec: fractional part in nano seconds to be used with
vm-clock-sec
#
# Since: 1.3
#
##

{ 'type': 'SnapshotInfo',
  'data': { 'id': 'str', 'name': 'str', 'vm-state-size': 'int',
            'date-sec': 'int', 'date-nsec': 'int',
            'vm-clock-sec': 'int', 'vm-clock-nsec': 'int' } }


And that type is already used in 'ImageInfo'.

In other words, we're stuck.  We've already cemented the mistake.  You
_can't_ drop vm-clock-sec, without breaking the behavior of management
apps written against the 1.3 interface when dealing with ImageInfo.  If
you want to avoid a (sec/nsec) pair, you would have to invent a new type
instead of reusing the already-existing SnapshotInfo.

Hmm, as I typed that, I did another search of qemu-schema.json - we have
the type 'ImageInfo' defined, but none of the existing 'command's ever
call out the use of that type.  Is it a type we are only using
internally to date, and where this is the first QMP command that would
actually expose SnapshotInfo or ImageInfo to a management app?  Then
maybe we _CAN_ modify SnapshotInfo, clean up all the internal code, and
provide a saner type for the first time that QMP can actually use it.

On Thu, 11 Apr 2013 07:08:41 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 04/11/2013 06:44 AM, Luiz Capitulino wrote:
> 
> >>>>> +-> { "execute": "query-snapshots" }
> >>>>> +<- {
> >>>>> +      "return":[
> >>>>> +         {
> >>>>> +            "id": "1",
> >>>>> +            "name": "snapshot1",
> >>>>> +            "vm-state-size": 0,
> >>>>> +            "date-sec": 10000200,
> >>>>> +            "date-nsec": 12,
> >>>>> +            "vm-clock-sec": 206,
> >>>>> +            "vm-clock-nsec": 30
> >>>>
> >>>> Not your patch's fault, but here goes anyway: I dislike this
> >>>> representation of time.
> >>>>
> >>>> QMP has time in seconds, milliseconds, nanoseconds, (seconds,
> 
> > Are you saying you're going to drop vm-clock-sec?
> > 
> >> Before you do that, let's get Luiz's blessing.
> > 
> > Fine with me if I got it right.
> 
> Problem.  Let's go back to the original definition that we are talking
> about modifying:
> 
> +#
> +# Since: 1.5
> +##
> +{ 'command': 'query-snapshots',
> +  'returns': ['SnapshotInfo'] }
> +
> 
> Now let's look for that type:
> 
> # @vm-clock-sec: VM clock relative to boot in seconds
> #
> # @vm-clock-nsec: fractional part in nano seconds to be used with
> vm-clock-sec
> #
> # Since: 1.3
> #
> ##
> 
> { 'type': 'SnapshotInfo',
>   'data': { 'id': 'str', 'name': 'str', 'vm-state-size': 'int',
>             'date-sec': 'int', 'date-nsec': 'int',
>             'vm-clock-sec': 'int', 'vm-clock-nsec': 'int' } }
> 
> 
> And that type is already used in 'ImageInfo'.
> 
> In other words, we're stuck.  We've already cemented the mistake.  You
> _can't_ drop vm-clock-sec, without breaking the behavior of management
> apps written against the 1.3 interface when dealing with ImageInfo.  If
> you want to avoid a (sec/nsec) pair, you would have to invent a new type
> instead of reusing the already-existing SnapshotInfo.

You're right. I actually replied too quickly and thought that we were talking
about the types being introduced by this patch. Sorry for that.

> Hmm, as I typed that, I did another search of qemu-schema.json - we have
> the type 'ImageInfo' defined, but none of the existing 'command's ever
> call out the use of that type.  Is it a type we are only using
> internally to date, and where this is the first QMP command that would
> actually expose SnapshotInfo or ImageInfo to a management app?  Then
> maybe we _CAN_ modify SnapshotInfo, clean up all the internal code, and
> provide a saner type for the first time that QMP can actually use it.

IIRC, it's being used by qemu-img. As it's not trivial to determine if
the field has users and as what goes in the schema is to be considered stable,
it's better to keep the field. Instead, we could add a better field and
deprecate the current one.

于 2013-4-11 21:39, Luiz Capitulino 写道:
> On Thu, 11 Apr 2013 07:08:41 -0600
> Eric Blake <eblake@redhat.com> wrote:
>
>> On 04/11/2013 06:44 AM, Luiz Capitulino wrote:
>>
>>>>>>> +-> { "execute": "query-snapshots" }
>>>>>>> +<- {
>>>>>>> +      "return":[
>>>>>>> +         {
>>>>>>> +            "id": "1",
>>>>>>> +            "name": "snapshot1",
>>>>>>> +            "vm-state-size": 0,
>>>>>>> +            "date-sec": 10000200,
>>>>>>> +            "date-nsec": 12,
>>>>>>> +            "vm-clock-sec": 206,
>>>>>>> +            "vm-clock-nsec": 30
>>>>>>
>>>>>> Not your patch's fault, but here goes anyway: I dislike this
>>>>>> representation of time.
>>>>>>
>>>>>> QMP has time in seconds, milliseconds, nanoseconds, (seconds,
>>
>>> Are you saying you're going to drop vm-clock-sec?
>>>
>>>> Before you do that, let's get Luiz's blessing.
>>>
>>> Fine with me if I got it right.
>>
>> Problem.  Let's go back to the original definition that we are talking
>> about modifying:
>>
>> +#
>> +# Since: 1.5
>> +##
>> +{ 'command': 'query-snapshots',
>> +  'returns': ['SnapshotInfo'] }
>> +
>>
>> Now let's look for that type:
>>
>> # @vm-clock-sec: VM clock relative to boot in seconds
>> #
>> # @vm-clock-nsec: fractional part in nano seconds to be used with
>> vm-clock-sec
>> #
>> # Since: 1.3
>> #
>> ##
>>
>> { 'type': 'SnapshotInfo',
>>    'data': { 'id': 'str', 'name': 'str', 'vm-state-size': 'int',
>>              'date-sec': 'int', 'date-nsec': 'int',
>>              'vm-clock-sec': 'int', 'vm-clock-nsec': 'int' } }
>>
>>
>> And that type is already used in 'ImageInfo'.
>>
>> In other words, we're stuck.  We've already cemented the mistake.  You
>> _can't_ drop vm-clock-sec, without breaking the behavior of management
>> apps written against the 1.3 interface when dealing with ImageInfo.  If
>> you want to avoid a (sec/nsec) pair, you would have to invent a new type
>> instead of reusing the already-existing SnapshotInfo.
>
> You're right. I actually replied too quickly and thought that we were talking
> about the types being introduced by this patch. Sorry for that.
>
>> Hmm, as I typed that, I did another search of qemu-schema.json - we have
>> the type 'ImageInfo' defined, but none of the existing 'command's ever
>> call out the use of that type.  Is it a type we are only using
>> internally to date, and where this is the first QMP command that would
>> actually expose SnapshotInfo or ImageInfo to a management app?  Then
>> maybe we _CAN_ modify SnapshotInfo, clean up all the internal code, and
>> provide a saner type for the first time that QMP can actually use it.
>
> IIRC, it's being used by qemu-img. As it's not trivial to determine if
> the field has users and as what goes in the schema is to be considered stable,
> it's better to keep the field. Instead, we could add a better field and
> deprecate the current one.
>
   I remember there is a patch laster year made "qemu-img info" dump out
json strings, where vm-clock-sec is dumped out.

On 04/11/2013 09:17 PM, Wenchao Xia wrote:

>>
>>> Hmm, as I typed that, I did another search of qemu-schema.json - we have
>>> the type 'ImageInfo' defined, but none of the existing 'command's ever
>>> call out the use of that type.  Is it a type we are only using
>>> internally to date, and where this is the first QMP command that would
>>> actually expose SnapshotInfo or ImageInfo to a management app?  Then
>>> maybe we _CAN_ modify SnapshotInfo, clean up all the internal code, and
>>> provide a saner type for the first time that QMP can actually use it.
>>
>> IIRC, it's being used by qemu-img. As it's not trivial to determine if
>> the field has users and as what goes in the schema is to be considered
>> stable,
>> it's better to keep the field. Instead, we could add a better field and
>> deprecate the current one.
>>
>   I remember there is a patch laster year made "qemu-img info" dump out
> json strings, where vm-clock-sec is dumped out.

Ah, so it's not QMP exposing it, but qemu-info in JSON mode.  Still,
changing that output to delete a field might break existing tools;
adding a field might be safer, but then you'd have redundant information
in the old style for old tools, as well as the added field.

于 2013-4-12 11:22, Eric Blake 写道:
> On 04/11/2013 09:17 PM, Wenchao Xia wrote:
>
>>>
>>>> Hmm, as I typed that, I did another search of qemu-schema.json - we have
>>>> the type 'ImageInfo' defined, but none of the existing 'command's ever
>>>> call out the use of that type.  Is it a type we are only using
>>>> internally to date, and where this is the first QMP command that would
>>>> actually expose SnapshotInfo or ImageInfo to a management app?  Then
>>>> maybe we _CAN_ modify SnapshotInfo, clean up all the internal code, and
>>>> provide a saner type for the first time that QMP can actually use it.
>>>
>>> IIRC, it's being used by qemu-img. As it's not trivial to determine if
>>> the field has users and as what goes in the schema is to be considered
>>> stable,
>>> it's better to keep the field. Instead, we could add a better field and
>>> deprecate the current one.
>>>
>>    I remember there is a patch laster year made "qemu-img info" dump out
>> json strings, where vm-clock-sec is dumped out.
>
> Ah, so it's not QMP exposing it, but qemu-info in JSON mode.  Still,
> changing that output to delete a field might break existing tools;
> adding a field might be safer, but then you'd have redundant information
> in the old style for old tools, as well as the added field.
>
   It seems keeping it, is what best we can do now. Markus, are you OK
with it?

Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:

> 于 2013-4-12 11:22, Eric Blake 写道:
>> On 04/11/2013 09:17 PM, Wenchao Xia wrote:
>>
>>>>
>>>>> Hmm, as I typed that, I did another search of qemu-schema.json - we have
>>>>> the type 'ImageInfo' defined, but none of the existing 'command's ever
>>>>> call out the use of that type.  Is it a type we are only using
>>>>> internally to date, and where this is the first QMP command that would
>>>>> actually expose SnapshotInfo or ImageInfo to a management app?  Then
>>>>> maybe we _CAN_ modify SnapshotInfo, clean up all the internal code, and
>>>>> provide a saner type for the first time that QMP can actually use it.
>>>>
>>>> IIRC, it's being used by qemu-img. As it's not trivial to determine if
>>>> the field has users and as what goes in the schema is to be considered
>>>> stable,
>>>> it's better to keep the field. Instead, we could add a better field and
>>>> deprecate the current one.
>>>>
>>>    I remember there is a patch laster year made "qemu-img info" dump out
>>> json strings, where vm-clock-sec is dumped out.
>>
>> Ah, so it's not QMP exposing it, but qemu-info in JSON mode.  Still,
>> changing that output to delete a field might break existing tools;
>> adding a field might be safer, but then you'd have redundant information
>> in the old style for old tools, as well as the added field.
>>
>   It seems keeping it, is what best we can do now. Markus, are you OK
> with it?

Yes, because:

"qemu-img info --output=json" exists since commit c054b3f, v1.3.0.  I
agree we shouldn't change it incompatibly just because we hate the way
time is represented there.

Doesn't mean we *have* to duplicate the ugly time representation into
QMP.  But avoiding it there involves duplicating SnapshotInfo and
ImageInfo in the schema.  Not worth it, considering there's no
consistency in QMP's time representation anyway.