All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: qemu-devel@nongnu.org, Luiz Capitulino <lcapitulino@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 4/4] qobject: Output valid JSON for non-finite numbers
Date: Thu, 16 Jun 2016 18:17:41 +0200	[thread overview]
Message-ID: <87inx9w1ju.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <1465526889-8339-5-git-send-email-eblake@redhat.com> (Eric Blake's message of "Thu, 9 Jun 2016 20:48:09 -0600")

Eric Blake <eblake@redhat.com> writes:

> It's better to give downstream clients a valid JSON string,
> even if they are semantically expecting a number, than it is
> to give them a bare keyword extension that can cause a
> lexical error.

Incompatible change.  If all clients are choking on non-finite numbers,
then the incompatibility is an improvement.  If a client exists that
groks non-finite numbers, ...  Absence is always hard to show.

Moreover, it turns query-qmp-schema into a liar: the schema it returns
claims a certain member of the reply has "type": "number", and then we
go on to send a string anyway.

> Of course, as long as we don't recognize (certain) strings as valid
> numbers during a conversion to QObject,

That would be even crazier!

>                                         this means our extension
> of accepting bare keywords for non-finite numbers cannot undergo
> a round trip (once converted into a string, we never get back to
> a QFloat).  However, non-finite input is rare enough that it's
> not worth bothering with at the moment.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>

I'm afraid the only sane solution is to find all uses of number in QMP
output, audit the code producing them, then assert isfinite() in the
monitor.  For commands without a side effect, we could fail the command
instead of tripping an assertion.  We'd have to declare such commands.

Let's examine the occurences of "number" in output of query-qmp-schema,
or actually in the qmp-introspect.c that gets generated with -u:

* Object q_obj_migrate_set_downtime-arg member value: input

* Builtin number: d'uh!

* Object MigrationStats member mbps: in output of query-migrate

* Object XBZRLECacheStats member overflow: likewise

* Object KeyValue case number: not a type.

* Object BlockDeviceTimedStats members avg_rd_queue_depth,
  avg_wr_queue_depth: in output of query-blockstats

* Enum CommandLineParameterType member: not a type

* Enum JSONType member: not a type

* Enum KeyValueKind: not a type

* Object PciBusInfo member: not a type

So it's just query-migrate and query-blockstats.

  reply	other threads:[~2016-06-16 16:17 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-10  2:48 [Qemu-devel] [PATCH 0/4] Guarantee valid JSON in QMP, even for nonfinite numbers Eric Blake
2016-06-10  2:48 ` [Qemu-devel] [PATCH 1/4] qobject: Correct JSON lexer grammar comments Eric Blake
2016-06-16 16:19   ` Markus Armbruster
2016-06-16 17:41     ` Eric Blake
2016-06-17  7:54       ` Markus Armbruster
2016-06-21 13:53         ` Eric Blake
2016-06-10  2:48 ` [Qemu-devel] [PATCH 2/4] checkpatch: There is no qemu_strtod() Eric Blake
2016-06-16 16:20   ` Markus Armbruster
2016-06-16 16:31     ` Paolo Bonzini
2016-06-10  2:48 ` [Qemu-devel] [PATCH 3/4] qobject: Parse non-finite numbers, as an extension Eric Blake
2016-06-16 15:38   ` Markus Armbruster
2016-06-16 16:25     ` Markus Armbruster
2016-06-17  3:00       ` Eric Blake
2016-06-17  8:04         ` Markus Armbruster
2016-06-10  2:48 ` [Qemu-devel] [PATCH 4/4] qobject: Output valid JSON for non-finite numbers Eric Blake
2016-06-16 16:17   ` Markus Armbruster [this message]
2016-06-17  3:06     ` Eric Blake
2016-06-17  8:14       ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87inx9w1ju.fsf@dusky.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=eblake@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.