Re: [Qemu-devel] [PATCH V11 09/17] qmp: add interface query-snapshots

[Eric Blake]

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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature