[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH V11 09/17] qmp: add interface query-snapshots
Date: Thu, 11 Apr 2013 07:08:41 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130402 Thunderbird/17.0.5

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
# 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

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]