[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: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [PATCH V11 09/17] qmp: add interface query-snapshots |
Date: |
Thu, 11 Apr 2013 09:39:14 -0400 |
On Thu, 11 Apr 2013 07:08:41 -0600
Eric Blake <address@hidden> 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.
- Re: [Qemu-devel] [PATCH V11 06/17] block: add check for VM snapshot in bdrv_query_snapshot_info_list(), (continued)
- [Qemu-devel] [PATCH V11 08/17] block: move qmp_query_block() and bdrv_query_info() to block/qapi.c, Wenchao Xia, 2013/04/02
- [Qemu-devel] [PATCH V11 09/17] qmp: add interface query-snapshots, Wenchao Xia, 2013/04/02
- Re: [Qemu-devel] [PATCH V11 09/17] qmp: add interface query-snapshots, Stefan Hajnoczi, 2013/04/08
- Re: [Qemu-devel] [PATCH V11 09/17] qmp: add interface query-snapshots, Markus Armbruster, 2013/04/10
- Re: [Qemu-devel] [PATCH V11 09/17] qmp: add interface query-snapshots, Wenchao Xia, 2013/04/11
- Re: [Qemu-devel] [PATCH V11 09/17] qmp: add interface query-snapshots, Markus Armbruster, 2013/04/11
- Re: [Qemu-devel] [PATCH V11 09/17] qmp: add interface query-snapshots, Luiz Capitulino, 2013/04/11
- Re: [Qemu-devel] [PATCH V11 09/17] qmp: add interface query-snapshots, Eric Blake, 2013/04/11
- Re: [Qemu-devel] [PATCH V11 09/17] qmp: add interface query-snapshots,
Luiz Capitulino <=
- Re: [Qemu-devel] [PATCH V11 09/17] qmp: add interface query-snapshots, Wenchao Xia, 2013/04/11
- Re: [Qemu-devel] [PATCH V11 09/17] qmp: add interface query-snapshots, Eric Blake, 2013/04/11
- Re: [Qemu-devel] [PATCH V11 09/17] qmp: add interface query-snapshots, Wenchao Xia, 2013/04/12
- Re: [Qemu-devel] [PATCH V11 09/17] qmp: add interface query-snapshots, Markus Armbruster, 2013/04/12
[Qemu-devel] [PATCH V11 10/17] qmp: add recursive member in ImageInfo, Wenchao Xia, 2013/04/02
[Qemu-devel] [PATCH V11 07/17] block: add image info query function bdrv_query_image_info(), Wenchao Xia, 2013/04/02