[Qemu-devel] RFC: libyajl for JSON

[Qemu-devel] RFC: libyajl for JSON

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1717 bytes --]

Loaded question in response to
https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg06988.html, but
posting as a new thread to call attention to it:

Libvirt uses libyajl to parse and format JSON. Would it be worth
dragging in yet another prerequisite library into qemu and reuse
libyajl's parser instead of our own hand-rolled one?

I know that a while ago, the answer was "as long as we support
out-of-the-box builds on RHEL 5, that platform lacks yajl therefore we
can't depend on it" (and libvirt's solution on RHEL 5 is "qemu doesn't
support QMP and thus doesn't use JSON and thus libvirt doesn't need yajl
there").

But now that we have just recently bumped the minimum glib and python
versions to something not available on RHEL 5, it may also be time to
start thinking about outsourcing to libyajl, because as far as I can
tell, every platform that currently supports qemu out of the box has a
version of libyajl. And since libvirt has already figured out the grunt
work of how to simultaneously code to both the 1.x and 2.x APIs, it's
not that much of a stretch to reuse that work in qemu.

On the other hand, one of the "features" of qemu's hand-rolled json
parser is the ability to do qobject_from_jsonf("{'foo':%s}", "bar")
(that is, we extended JSON with our notion of single-quoted strings, and
with our notion of %s and similar escape sequences for piecing together
multiple inputs into a single input stream without having to first
g_strdup_printf our pieces into a single string).  I don't know if
libyajl lets us add extensions to the language it parses.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] RFC: libyajl for JSON

[Peter Maydell]

On 30 October 2015 at 19:45, Eric Blake <eblake@redhat.com> wrote:
> Loaded question in response to
> https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg06988.html, but
> posting as a new thread to call attention to it:
>
> Libvirt uses libyajl to parse and format JSON. Would it be worth
> dragging in yet another prerequisite library into qemu and reuse
> libyajl's parser instead of our own hand-rolled one?
>
> I know that a while ago, the answer was "as long as we support
> out-of-the-box builds on RHEL 5, that platform lacks yajl therefore we
> can't depend on it" (and libvirt's solution on RHEL 5 is "qemu doesn't
> support QMP and thus doesn't use JSON and thus libvirt doesn't need yajl
> there").
>
> But now that we have just recently bumped the minimum glib and python
> versions to something not available on RHEL 5, it may also be time to
> start thinking about outsourcing to libyajl, because as far as I can
> tell, every platform that currently supports qemu out of the box has a
> version of libyajl. And since libvirt has already figured out the grunt
> work of how to simultaneously code to both the 1.x and 2.x APIs, it's
> not that much of a stretch to reuse that work in qemu.

Even my OSX box's fink installation has yajl1 ;-)
OTOH, extra hard dependencies are a bit painful, especially
for people doing cross-compiles. One option would be to have
a git module for people who don't have a system version, as
we do already for pixman and dtc.

thanks
-- PMM

Re: [Qemu-devel] RFC: libyajl for JSON

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> Loaded question in response to
> https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg06988.html, but

Discussion of our parser's enormous appetite for wasting RAM.  Fixable,
but it's work, and it's not its only defect.

> posting as a new thread to call attention to it:
>
> Libvirt uses libyajl to parse and format JSON. Would it be worth
> dragging in yet another prerequisite library into qemu and reuse
> libyajl's parser instead of our own hand-rolled one?
>
> I know that a while ago, the answer was "as long as we support
> out-of-the-box builds on RHEL 5, that platform lacks yajl therefore we
> can't depend on it" (and libvirt's solution on RHEL 5 is "qemu doesn't
> support QMP and thus doesn't use JSON and thus libvirt doesn't need yajl
> there").
>
> But now that we have just recently bumped the minimum glib and python
> versions to something not available on RHEL 5, it may also be time to
> start thinking about outsourcing to libyajl, because as far as I can
> tell, every platform that currently supports qemu out of the box has a
> version of libyajl. And since libvirt has already figured out the grunt
> work of how to simultaneously code to both the 1.x and 2.x APIs, it's
> not that much of a stretch to reuse that work in qemu.

I'm not particularly attached to having our very own JSON parser.
However:

> On the other hand, one of the "features" of qemu's hand-rolled json
> parser is the ability to do qobject_from_jsonf("{'foo':%s}", "bar")
> (that is, we extended JSON with our notion of single-quoted strings, and
> with our notion of %s and similar escape sequences for piecing together
> multiple inputs into a single input stream without having to first
> g_strdup_printf our pieces into a single string).  I don't know if
> libyajl lets us add extensions to the language it parses.

Actually two separate extensions:

* Single-quoted strings

  The extension's purpose is avoiding quotes in C.  Example:

      "{ 'execute': 'migrate_set_speed', 'arguments': { 'value': 10 } }"

  is easier to read than

      "{ \"execute\": \"migrate_set_speed\", \"arguments\": { \"value\": 10 } }"

  Most actual uses are in tests.

  JSON5, a proposed extension to JSON also supports single-quoted
  strings.  So does Python.

* Optional interpolation

  If you pass a va_list to the parser, it replaces certain escape
  sequences by values from that va_list.  The escape sequences are a
  subset of printf conversion specifiers, to enable compile-time
  checking with __attribute__((format...)).

  We used to rely on this for creating "rich" error objects, but those
  are long gone (commit df1e608).  Perhaps two dozen uses are left.

  We could convert them to use "normal" interpolation, i.e. first format
  the arguments as JSON, then interpolate with g_strdup_printf() or
  similar, then parse.  Formatting only to parse it right back is
  inelegant.  Also inefficient, but that probably doesn't matter here.

  The current interface could be retained as convenience function to
  interpolate and feed the result to the JSOn parser.

  Alternatively, we could build the QObject manually instead.  More
  efficient than either kind of interpolation, but a good deal less
  readable.

Got one more, actually a pitfall, not an extension:

* Representation of JSON numbers

  RFC 7159 doesn't specify how numbers are to be represented.

  Many JSON implementations represent any JSON number as double.  This
  can represent signed integers up to 53 bits exactly.

  Our parser represents numbers as int64_t when possible, else as
  double.

  Unlike the extensions above, this one is essential: any parser that
  can't do it is useless for us.  Can yajl do it?

More extensions or pitfalls might be lurking in our parser.  Therefore,
replacing our parser by a suitable library is not without risk.  I guess
we could do it over a full development cycle.  No way for 2.5.

If we decide to attempt replacing it in the next development cycle, we
might want to refrain from fixing its bugs except for true show
stoppers.

Re: [Qemu-devel] RFC: libyajl for JSON

[Paolo Bonzini]

On 02/11/2015 09:40, Markus Armbruster wrote:
> 
> * Optional interpolation
> 
>   If you pass a va_list to the parser, it replaces certain escape
>   sequences by values from that va_list.  The escape sequences are a
>   subset of printf conversion specifiers, to enable compile-time
>   checking with __attribute__((format...)).
> 
>   We used to rely on this for creating "rich" error objects, but those
>   are long gone (commit df1e608).  Perhaps two dozen uses are left.

Most of these are probably in the tests, where they are used to send QMP
commands.

>   We could convert them to use "normal" interpolation, i.e. first format
>   the arguments as JSON, then interpolate with g_strdup_printf() or
>   similar, then parse.  Formatting only to parse it right back is
>   inelegant.  Also inefficient, but that probably doesn't matter here.

I don't care about inefficiency or elegance, but I care about bugs.  One
difference between printf-ing \"%s\" and JSON-parsing %s is that the
latter performs backslash escaping.  The lack of backslash escaping has
been the source of bugs in the past, so it's important that the API
remains as convenient as it is now.

There is one alternative: rewriting tests in Python.  I'm only
half-kidding, probably less than half.  It would be great for example to
reuse the work on BITS (http://biosbits.org/) within qtest.

>   Unlike the extensions above, this one is essential: any parser that
>   can't do it is useless for us.  Can yajl do it?

I think it does.

Paolo

Re: [Qemu-devel] RFC: libyajl for JSON

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 02/11/2015 09:40, Markus Armbruster wrote:
>> 
>> * Optional interpolation
>> 
>>   If you pass a va_list to the parser, it replaces certain escape
>>   sequences by values from that va_list.  The escape sequences are a
>>   subset of printf conversion specifiers, to enable compile-time
>>   checking with __attribute__((format...)).
>> 
>>   We used to rely on this for creating "rich" error objects, but those
>>   are long gone (commit df1e608).  Perhaps two dozen uses are left.
>
> Most of these are probably in the tests, where they are used to send QMP
> commands.

Yes.  Not all of them, though.

>>   We could convert them to use "normal" interpolation, i.e. first format
>>   the arguments as JSON, then interpolate with g_strdup_printf() or
>>   similar, then parse.  Formatting only to parse it right back is
>>   inelegant.  Also inefficient, but that probably doesn't matter here.
>
> I don't care about inefficiency or elegance, but I care about bugs.  One
> difference between printf-ing \"%s\" and JSON-parsing %s is that the
> latter performs backslash escaping.  The lack of backslash escaping has
> been the source of bugs in the past, so it's important that the API
> remains as convenient as it is now.

That's why I wrote "first format as JSON, then interpolate".  Formatting
with printf() does not work precisely because it doesn't format as JSON.
We'd have to use our JSON formatter.  Any bugs it may have we need to
fix anyway, unless we also replace it wholesale.

> There is one alternative: rewriting tests in Python.  I'm only
> half-kidding, probably less than half.  It would be great for example to
> reuse the work on BITS (http://biosbits.org/) within qtest.
>
[On representing JSON numbers as int64_t when they fit:]
>>   Unlike the extensions above, this one is essential: any parser that
>>   can't do it is useless for us.  Can yajl do it?
>
> I think it does.

Good.

One more thing on our JSON parser.

A classical parser gets passed a source of characters (string, file
descriptor, whatever), parses until it reaches a terminating state, then
returns an abstract syntax tree (AST).  Basically a function mapping a
character source to an AST.

Our JSON parser has inverted control flow: it gets fed characters on at
a time, by the main loop via chardev IOReadHandler, and when it reaches
a terminating state, it passes the parse result to a callback.

Except this is an oversimplification.  We actually have two parsers, a
stupid one that can only count braces and brackets (json-streamer.c),
and the real one (json-parser.c).  The stupid one passes a list of
tokens to the callback, which runs the real parser to parse the tokens
into a QObject (this is our AST), then does whatever needs to be done
with the QObject.

But this is detail, the point remains that the current JSON parsing
machinery gets fed characters and the code consuming the parse lives in
a callback, and any replacement also needs to be fit into the main loop
somehow.

Re: [Qemu-devel] RFC: libyajl for JSON

[Luiz Capitulino]

[I thought I had replied to this thread, but it doesn't seem so. So,
 I'll try again]

On Fri, 30 Oct 2015 13:45:48 -0600
Eric Blake <eblake@redhat.com> wrote:

> Loaded question in response to
> https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg06988.html, but
> posting as a new thread to call attention to it:
> 
> Libvirt uses libyajl to parse and format JSON. Would it be worth
> dragging in yet another prerequisite library into qemu and reuse
> libyajl's parser instead of our own hand-rolled one?

I'd be in favor of that.

I don't exactly remember why we didn't use an external library like
libyajl. Maybe it was unknown to us, or maybe having an external dep
was too painful at the time. But I do remember we looked at having
something we could merge in QEMU source code, and everything we found
had license issues.

> 
> I know that a while ago, the answer was "as long as we support
> out-of-the-box builds on RHEL 5, that platform lacks yajl therefore we
> can't depend on it" (and libvirt's solution on RHEL 5 is "qemu doesn't
> support QMP and thus doesn't use JSON and thus libvirt doesn't need yajl
> there").
> 
> But now that we have just recently bumped the minimum glib and python
> versions to something not available on RHEL 5, it may also be time to
> start thinking about outsourcing to libyajl, because as far as I can
> tell, every platform that currently supports qemu out of the box has a
> version of libyajl. And since libvirt has already figured out the grunt
> work of how to simultaneously code to both the 1.x and 2.x APIs, it's
> not that much of a stretch to reuse that work in qemu.
> 
> On the other hand, one of the "features" of qemu's hand-rolled json
> parser is the ability to do qobject_from_jsonf("{'foo':%s}", "bar")
> (that is, we extended JSON with our notion of single-quoted strings, and
> with our notion of %s and similar escape sequences for piecing together
> multiple inputs into a single input stream without having to first
> g_strdup_printf our pieces into a single string).  I don't know if
> libyajl lets us add extensions to the language it parses.
> 

Re: [Qemu-devel] RFC: libyajl for JSON

[Paolo Bonzini]

On 02/11/2015 13:56, Markus Armbruster wrote:
> A classical parser gets passed a source of characters (string, file
> descriptor, whatever), parses until it reaches a terminating state, then
> returns an abstract syntax tree (AST).  Basically a function mapping a
> character source to an AST.
> 
> Our JSON parser has inverted control flow: it gets fed characters on at
> a time, by the main loop via chardev IOReadHandler, and when it reaches
> a terminating state, it passes the parse result to a callback.

Yes, ours is a so-called "push" parser or streaming parser, and it's
actually more and more popular because it's easy to go push->pull but
much harder to build a push parser on top of a pull engine.

yajl does the same as QEMU already does.  It's main API is:

   YAJL_API yajl_status yajl_parse(yajl_handle hand,
                                   const unsigned char * jsonText,
                                   size_t jsonTextLength);

   YAJL_API yajl_status yajl_complete_parse(yajl_handle hand);

and actually these days you can even ask Bison to produce a push parser.

An interesting part of yajl is that it doesn't have an AST.  Instead the
user provides callbacks for integers, booleans, "start of map", "end of
map", etc.  So the "emit QObject" part of our parser, and especially the
need to maintain a stack, would remain.  Though perhaps we could use our
own callback abstraction (visitors) and reuse it to build QObjects with
only a simple mapping layer.  In that case the lack of AST would
actually be an advantage, or at least neutral.

> Except this is an oversimplification.  We actually have two parsers, a
> stupid one that can only count braces and brackets (json-streamer.c),
> and the real one (json-parser.c).  The stupid one passes a list of
> tokens to the callback, which runs the real parser to parse the tokens
> into a QObject (this is our AST), then does whatever needs to be done
> with the QObject.
> 
> But this is detail, the point remains that the current JSON parsing
> machinery gets fed characters and the code consuming the parse lives in
> a callback, and any replacement also needs to be fit into the main loop
> somehow.

Yeah, the streamer vs. parser distinction is a hack that avoids writing
everything in continuation-passing style (where the continuation is
basically the parser's state).  It's weird, but luckily it's not visible
to the ourside world.

Paolo

Re: [Qemu-devel] RFC: libyajl for JSON

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 390 bytes --]

On Mon, Nov 02, 2015 at 09:40:39AM +0100, Markus Armbruster wrote:
> More extensions or pitfalls might be lurking in our parser.  Therefore,
> replacing our parser by a suitable library is not without risk.  I guess
> we could do it over a full development cycle.  No way for 2.5.

Good points.  I'm also concerned about bugs due to the slight variant of
JSON that QEMU implements.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] RFC: libyajl for JSON

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 5929 bytes --]

On 11/02/2015 01:40 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> Loaded question in response to
>> https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg06988.html, but
> 
> Discussion of our parser's enormous appetite for wasting RAM.  Fixable,
> but it's work, and it's not its only defect.
> 

>> On the other hand, one of the "features" of qemu's hand-rolled json
>> parser is the ability to do qobject_from_jsonf("{'foo':%s}", "bar")
>> (that is, we extended JSON with our notion of single-quoted strings, and
>> with our notion of %s and similar escape sequences for piecing together
>> multiple inputs into a single input stream without having to first
>> g_strdup_printf our pieces into a single string).  I don't know if
>> libyajl lets us add extensions to the language it parses.
> 
> Actually two separate extensions:
> 
> * Single-quoted strings
> 
>   The extension's purpose is avoiding quotes in C.  Example:
> 
>       "{ 'execute': 'migrate_set_speed', 'arguments': { 'value': 10 } }"
> 
>   is easier to read than
> 
>       "{ \"execute\": \"migrate_set_speed\", \"arguments\": { \"value\": 10 } }"
> 
>   Most actual uses are in tests.

Except that we have documented that QMP clients may use it; and indeed:

$ printf "{'execute':'qmp_capabilities'}\n{'execute':'quit'}" | \
   ./x86_64-softmmu/qemu-system-x86_64 -nographic -nodefaults -qmp stdio
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 4, "major": 2},
"package": ""}, "capabilities": []}}
{"return": {}}
{"return": {}}
{"timestamp": {"seconds": 1446478389, "microseconds": 265886}, "event":
"SHUTDOWN"}

So that is an absolute must for whatever parser we choose.  My first
glance at libyajl is that it does NOT currently allow single quotes (not
even with any of its existing options), so we'd have to pre-process
input before feeding it to yajl.

> 
>   JSON5, a proposed extension to JSON also supports single-quoted
>   strings.  So does Python.

It would be an interesting task to see if yajl would accept patches to
parse JSON5 as additional options (for example, yajl already has options
on whether to diagnose or ignore UTF8 errors, and on whether to allow /*
*/ javascript comments).  But then we'd be requiring an
as-yet-unreleased version of libyajl rather than being able to rely on
what distros already have, at least for a few years; or we'd have to
carry new-enough yajl as a git submodule within qemu.git.

I'll ask on the yajl mailing lists, to get a feel for community
receptiveness, before attempting to actually write patches on that front.

> 
> * Optional interpolation
> 
>   If you pass a va_list to the parser, it replaces certain escape
>   sequences by values from that va_list.  The escape sequences are a
>   subset of printf conversion specifiers, to enable compile-time
>   checking with __attribute__((format...)).
> 
>   We used to rely on this for creating "rich" error objects, but those
>   are long gone (commit df1e608).  Perhaps two dozen uses are left.

The testsuite is probably the biggest user, but we still have uses
throughout the code base.

Based on my look at libyajl, I think we CAN get away with stream
interpolation - yajl maintains state such that you do NOT have to feed
it the entire stream in one go.  So this one is not insurmountable; we
could rewrite our qobject_from_jsonf() to make multiple calls into yajl
without having to pre-format a single string.

> 
>   We could convert them to use "normal" interpolation, i.e. first format
>   the arguments as JSON, then interpolate with g_strdup_printf() or
>   similar, then parse.  Formatting only to parse it right back is
>   inelegant.  Also inefficient, but that probably doesn't matter here.

Or indeed, this is still an option.

> 
>   The current interface could be retained as convenience function to
>   interpolate and feed the result to the JSOn parser.
> 
>   Alternatively, we could build the QObject manually instead.  More
>   efficient than either kind of interpolation, but a good deal less
>   readable.

At any rate, it is certainly less of a show-stopper when compared to the
need for supporting single-quoted strings.

> 
> Got one more, actually a pitfall, not an extension:
> 
> * Representation of JSON numbers
> 
>   RFC 7159 doesn't specify how numbers are to be represented.
> 
>   Many JSON implementations represent any JSON number as double.  This
>   can represent signed integers up to 53 bits exactly.
> 
>   Our parser represents numbers as int64_t when possible, else as
>   double.
> 
>   Unlike the extensions above, this one is essential: any parser that
>   can't do it is useless for us.  Can yajl do it?

Yes; the yajl parser has 4 modes of parse operation based on which of
three callbacks you implement: double-only (yajl_double), long long-only
(yajl_integer), double-and-int (both yajl_double and yajl_integer, not
sure which one has precedence if input satisfies both formats), or
custom number (yajl_number, which is given a string, and you turn it
into the format of your choice).  Likewise, the yajl formatter has 2
functions: yajl_gen_double() (formats doubles) and yajl_gen_number()
(you provide literal strings formatted how you like).

> 
> More extensions or pitfalls might be lurking in our parser.  Therefore,
> replacing our parser by a suitable library is not without risk.  I guess
> we could do it over a full development cycle.  No way for 2.5.
> 

Absolutely not for 2.5; we've missed softfreeze, and this would count as
a new feature.

> If we decide to attempt replacing it in the next development cycle, we
> might want to refrain from fixing its bugs except for true show
> stoppers.
> 

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] RFC: libyajl for JSON

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 11/02/2015 01:40 AM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>> 
>>> Loaded question in response to
>>> https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg06988.html, but
>> 
>> Discussion of our parser's enormous appetite for wasting RAM.  Fixable,
>> but it's work, and it's not its only defect.
>> 
>
>>> On the other hand, one of the "features" of qemu's hand-rolled json
>>> parser is the ability to do qobject_from_jsonf("{'foo':%s}", "bar")
>>> (that is, we extended JSON with our notion of single-quoted strings, and
>>> with our notion of %s and similar escape sequences for piecing together
>>> multiple inputs into a single input stream without having to first
>>> g_strdup_printf our pieces into a single string).  I don't know if
>>> libyajl lets us add extensions to the language it parses.
>> 
>> Actually two separate extensions:
>> 
>> * Single-quoted strings
>> 
>>   The extension's purpose is avoiding quotes in C.  Example:
>> 
>>       "{ 'execute': 'migrate_set_speed', 'arguments': { 'value': 10 } }"
>> 
>>   is easier to read than
>> 
>>       "{ \"execute\": \"migrate_set_speed\", \"arguments\": { \"value\": 10 } }"
>> 
>>   Most actual uses are in tests.
>
> Except that we have documented that QMP clients may use it; and indeed:
>
> $ printf "{'execute':'qmp_capabilities'}\n{'execute':'quit'}" | \
>    ./x86_64-softmmu/qemu-system-x86_64 -nographic -nodefaults -qmp stdio
> {"QMP": {"version": {"qemu": {"micro": 50, "minor": 4, "major": 2},
> "package": ""}, "capabilities": []}}
> {"return": {}}
> {"return": {}}
> {"timestamp": {"seconds": 1446478389, "microseconds": 265886}, "event":
> "SHUTDOWN"}

Unnecessary mistake :(

> So that is an absolute must for whatever parser we choose.  My first
> glance at libyajl is that it does NOT currently allow single quotes (not
> even with any of its existing options), so we'd have to pre-process
> input before feeding it to yajl.

I'm not sure reusing an existing parser makes much sense if we need to
preprocess anyway.  After all, parsing JSON isn't exactly a moon shot,
at least if you know what you're doing.

>>   JSON5, a proposed extension to JSON also supports single-quoted
>>   strings.  So does Python.
>
> It would be an interesting task to see if yajl would accept patches to
> parse JSON5 as additional options (for example, yajl already has options
> on whether to diagnose or ignore UTF8 errors, and on whether to allow /*
> */ javascript comments).  But then we'd be requiring an
> as-yet-unreleased version of libyajl rather than being able to rely on
> what distros already have, at least for a few years; or we'd have to
> carry new-enough yajl as a git submodule within qemu.git.
>
> I'll ask on the yajl mailing lists, to get a feel for community
> receptiveness, before attempting to actually write patches on that front.

Makes sense.

>> * Optional interpolation
>> 
>>   If you pass a va_list to the parser, it replaces certain escape
>>   sequences by values from that va_list.  The escape sequences are a
>>   subset of printf conversion specifiers, to enable compile-time
>>   checking with __attribute__((format...)).
>> 
>>   We used to rely on this for creating "rich" error objects, but those
>>   are long gone (commit df1e608).  Perhaps two dozen uses are left.
>
> The testsuite is probably the biggest user, but we still have uses
> throughout the code base.
>
> Based on my look at libyajl, I think we CAN get away with stream
> interpolation - yajl maintains state such that you do NOT have to feed
> it the entire stream in one go.  So this one is not insurmountable; we
> could rewrite our qobject_from_jsonf() to make multiple calls into yajl
> without having to pre-format a single string.

Sounds complicated...

>>   We could convert them to use "normal" interpolation, i.e. first format
>>   the arguments as JSON, then interpolate with g_strdup_printf() or
>>   similar, then parse.  Formatting only to parse it right back is
>>   inelegant.  Also inefficient, but that probably doesn't matter here.
>
> Or indeed, this is still an option.
>
>> 
>>   The current interface could be retained as convenience function to
>>   interpolate and feed the result to the JSOn parser.
>> 
>>   Alternatively, we could build the QObject manually instead.  More
>>   efficient than either kind of interpolation, but a good deal less
>>   readable.
>
> At any rate, it is certainly less of a show-stopper when compared to the
> need for supporting single-quoted strings.
>
>> Got one more, actually a pitfall, not an extension:
>> 
>> * Representation of JSON numbers
>> 
>>   RFC 7159 doesn't specify how numbers are to be represented.
>> 
>>   Many JSON implementations represent any JSON number as double.  This
>>   can represent signed integers up to 53 bits exactly.
>> 
>>   Our parser represents numbers as int64_t when possible, else as
>>   double.
>> 
>>   Unlike the extensions above, this one is essential: any parser that
>>   can't do it is useless for us.  Can yajl do it?
>
> Yes; the yajl parser has 4 modes of parse operation based on which of
> three callbacks you implement: double-only (yajl_double), long long-only
> (yajl_integer), double-and-int (both yajl_double and yajl_integer, not
> sure which one has precedence if input satisfies both formats), or
> custom number (yajl_number, which is given a string, and you turn it
> into the format of your choice).  Likewise, the yajl formatter has 2
> functions: yajl_gen_double() (formats doubles) and yajl_gen_number()
> (you provide literal strings formatted how you like).

Good.

>> More extensions or pitfalls might be lurking in our parser.  Therefore,
>> replacing our parser by a suitable library is not without risk.  I guess
>> we could do it over a full development cycle.  No way for 2.5.
>> 
>
> Absolutely not for 2.5; we've missed softfreeze, and this would count as
> a new feature.
>
>> If we decide to attempt replacing it in the next development cycle, we
>> might want to refrain from fixing its bugs except for true show
>> stoppers.
>> 

Re: [Qemu-devel] RFC: libyajl for JSON

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 4074 bytes --]

On 11/02/2015 12:10 PM, Markus Armbruster wrote:

>>> * Single-quoted strings

> 
>> So that is an absolute must for whatever parser we choose.  My first
>> glance at libyajl is that it does NOT currently allow single quotes (not
>> even with any of its existing options), so we'd have to pre-process
>> input before feeding it to yajl.

>>
>> I'll ask on the yajl mailing lists, to get a feel for community
>> receptiveness, before attempting to actually write patches on that front.
> 
> Makes sense.

On IRC, I got pointed to a couple of forks that have already done this,
such as:

https://github.com/ludocode/yajl/commits/master

although the upstream author Lloyd was not on the channel at the time.
Meanwhile, there's 47 open issues on the upstream repo:

https://github.com/lloyd/yajl/issues

which implies slow response by the author, but at least he WAS asking if
anyone would like to help him with maintenance:

https://github.com/lloyd/yajl/issues/161

|  lloyd commented on Sep 24
| ok, give me a minute to hand you a patch on a branch to review.

|  lloyd commented on Sep 24
| ok, I'll merge down and roll a new release within a day.

[still hasn't happened yet...]

|  lamont-granquist commented on Sep 25
| hey @lloyd would you consider adding other maintainers?

|  lloyd commented on Oct 1
| I would absolutely consider additional maintainers. Who's interested?

so I don't know what the future holds for extending things upstream.

To date, I don't have a github account, by conscious personal choice; so
I can't really take him up on that offer directly.  So far, I've been
trying the mailing list to see if he answers that in addition to the
github PR stream, but the archives show it to be pretty full of spam:
http://librelist.com/browser/yajl/

>>
>> Yes; the yajl parser has 4 modes of parse operation based on which of
>> three callbacks you implement: double-only (yajl_double), long long-only
>> (yajl_integer), double-and-int (both yajl_double and yajl_integer, not
>> sure which one has precedence if input satisfies both formats), or
>> custom number (yajl_number, which is given a string, and you turn it
>> into the format of your choice).  Likewise, the yajl formatter has 2
>> functions: yajl_gen_double() (formats doubles) and yajl_gen_number()
>> (you provide literal strings formatted how you like).
> 
> Good.

And one of the open bugs on the tracker was the same one we've
encountered ourselves: yajl is locale-sensitive when using strtod() for
parsing and when using printf() for printing doubles:

https://github.com/lloyd/yajl/issues/79

[I would love for C/POSIX to add strtod_l and printf_l, but that's a
thread for another day]

> 
>>> More extensions or pitfalls might be lurking in our parser.  Therefore,
>>> replacing our parser by a suitable library is not without risk.  I guess
>>> we could do it over a full development cycle.  No way for 2.5.
>>>
>>
>> Absolutely not for 2.5; we've missed softfreeze, and this would count as
>> a new feature.

Another interesting bug report against yajl: one reporter made the point
[1] that yajl is already a superset of the canonical RFC 4627 definition
of JSON [2], because while the RFC states that a JSON document has this
terminal state:
      JSON-text = object / array
yajl will accept _any_ JSON value (not just objects/arrays) as the
overall document.

[1] https://github.com/lloyd/yajl/issues/154
[2] https://www.ietf.org/rfc/rfc4627.txt

So at this point, I want to see if lloyd makes any progress towards an
actual yajl release and/or adding a co-maintainer, before even trying to
get formal upstream support for single quoting.  We could always create
a git submodule with our own choice of fork (since there are already
forks that do single-quote parsing) - but the mantra of 'upstream first'
has a lot of merit (I'm reluctant to fork without good reason).

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] RFC: libyajl for JSON

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 11/02/2015 12:10 PM, Markus Armbruster wrote:
>
>>>> * Single-quoted strings
>
>> 
>>> So that is an absolute must for whatever parser we choose.  My first
>>> glance at libyajl is that it does NOT currently allow single quotes (not
>>> even with any of its existing options), so we'd have to pre-process
>>> input before feeding it to yajl.
>
>>>
>>> I'll ask on the yajl mailing lists, to get a feel for community
>>> receptiveness, before attempting to actually write patches on that front.
>> 
>> Makes sense.
>
> On IRC, I got pointed to a couple of forks that have already done this,
> such as:
>
> https://github.com/ludocode/yajl/commits/master
>
> although the upstream author Lloyd was not on the channel at the time.
> Meanwhile, there's 47 open issues on the upstream repo:
>
> https://github.com/lloyd/yajl/issues
>
> which implies slow response by the author, but at least he WAS asking if
> anyone would like to help him with maintenance:
>
> https://github.com/lloyd/yajl/issues/161
>
> |  lloyd commented on Sep 24
> | ok, give me a minute to hand you a patch on a branch to review.
>
> |  lloyd commented on Sep 24
> | ok, I'll merge down and roll a new release within a day.
>
> [still hasn't happened yet...]
>
> |  lamont-granquist commented on Sep 25
> | hey @lloyd would you consider adding other maintainers?
>
> |  lloyd commented on Oct 1
> | I would absolutely consider additional maintainers. Who's interested?
>
> so I don't know what the future holds for extending things upstream.
>
> To date, I don't have a github account, by conscious personal choice; so
> I can't really take him up on that offer directly.  So far, I've been
> trying the mailing list to see if he answers that in addition to the
> github PR stream, but the archives show it to be pretty full of spam:
> http://librelist.com/browser/yajl/

I don't do github, either.

>>> Yes; the yajl parser has 4 modes of parse operation based on which of
>>> three callbacks you implement: double-only (yajl_double), long long-only
>>> (yajl_integer), double-and-int (both yajl_double and yajl_integer, not
>>> sure which one has precedence if input satisfies both formats), or
>>> custom number (yajl_number, which is given a string, and you turn it
>>> into the format of your choice).  Likewise, the yajl formatter has 2
>>> functions: yajl_gen_double() (formats doubles) and yajl_gen_number()
>>> (you provide literal strings formatted how you like).
>> 
>> Good.
>
> And one of the open bugs on the tracker was the same one we've
> encountered ourselves: yajl is locale-sensitive when using strtod() for
> parsing and when using printf() for printing doubles:
>
> https://github.com/lloyd/yajl/issues/79
>
> [I would love for C/POSIX to add strtod_l and printf_l, but that's a
> thread for another day]
>
>> 
>>>> More extensions or pitfalls might be lurking in our parser.  Therefore,
>>>> replacing our parser by a suitable library is not without risk.  I guess
>>>> we could do it over a full development cycle.  No way for 2.5.
>>>>
>>>
>>> Absolutely not for 2.5; we've missed softfreeze, and this would count as
>>> a new feature.
>
> Another interesting bug report against yajl: one reporter made the point
> [1] that yajl is already a superset of the canonical RFC 4627 definition
> of JSON [2], because while the RFC states that a JSON document has this
> terminal state:
>       JSON-text = object / array
> yajl will accept _any_ JSON value (not just objects/arrays) as the
> overall document.
>
> [1] https://github.com/lloyd/yajl/issues/154
> [2] https://www.ietf.org/rfc/rfc4627.txt

I believe our parser does the same.

> So at this point, I want to see if lloyd makes any progress towards an
> actual yajl release and/or adding a co-maintainer, before even trying to
> get formal upstream support for single quoting.  We could always create
> a git submodule with our own choice of fork (since there are already
> forks that do single-quote parsing) - but the mantra of 'upstream first'
> has a lot of merit (I'm reluctant to fork without good reason).

The value proposition of replacing our flawed JSON parser isn't in
saving big on maintenance, it's in not having to find and fix its flaws.

If the replacement needs a lot of work to fit our needs, the value
proposition becomes negative.

A JSON parser shouldn't require much maintenance, as JSON is simple,
doesn't change, and parsing has few system dependencies.

That said, maintaining fork of an unresponsive upstream is always
awkward.

Wait and see how yajl makes progress sounds sensible to me.  However, I
already hate the idea of releasing 2.5 with the colossal memory waste
unfixed.  I'm willing to sit on my hands for 2.5 only because it's not a
regression.  I'm unwilling to leave it unfixed much longer.

Point is, the longer we wait, the more we'll get invested in our own
parser.

Re: [Qemu-devel] RFC: libyajl for JSON

[Luiz Capitulino]

On Tue, 03 Nov 2015 08:17:58 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> > So at this point, I want to see if lloyd makes any progress towards an
> > actual yajl release and/or adding a co-maintainer, before even trying to
> > get formal upstream support for single quoting.  We could always create
> > a git submodule with our own choice of fork (since there are already
> > forks that do single-quote parsing) - but the mantra of 'upstream first'
> > has a lot of merit (I'm reluctant to fork without good reason).
> 
> The value proposition of replacing our flawed JSON parser isn't in
> saving big on maintenance, it's in not having to find and fix its flaws.
> 
> If the replacement needs a lot of work to fit our needs, the value
> proposition becomes negative.
> 
> A JSON parser shouldn't require much maintenance, as JSON is simple,
> doesn't change, and parsing has few system dependencies.

Let me suggest this crazy idea: have you guys considered breaking
compatibility?

Re: [Qemu-devel] RFC: libyajl for JSON

[Paolo Bonzini]

On 03/11/2015 14:19, Luiz Capitulino wrote:
> > The value proposition of replacing our flawed JSON parser isn't in
> > saving big on maintenance, it's in not having to find and fix its flaws.
> > 
> > If the replacement needs a lot of work to fit our needs, the value
> > proposition becomes negative.
> > 
> > A JSON parser shouldn't require much maintenance, as JSON is simple,
> > doesn't change, and parsing has few system dependencies.
> 
> Let me suggest this crazy idea: have you guys considered breaking
> compatibility?

Can you explain why that would make sense? :)  (Especially since there
is another extension---JSON5---that does exactly what we're doing, so it
probably wasn't that stupid an idea).

Paolo

Re: [Qemu-devel] RFC: libyajl for JSON

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1894 bytes --]

On 11/03/2015 06:19 AM, Luiz Capitulino wrote:
> On Tue, 03 Nov 2015 08:17:58 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
> 
>>> So at this point, I want to see if lloyd makes any progress towards an
>>> actual yajl release and/or adding a co-maintainer, before even trying to
>>> get formal upstream support for single quoting.  We could always create
>>> a git submodule with our own choice of fork (since there are already
>>> forks that do single-quote parsing) - but the mantra of 'upstream first'
>>> has a lot of merit (I'm reluctant to fork without good reason).
>>
>> The value proposition of replacing our flawed JSON parser isn't in
>> saving big on maintenance, it's in not having to find and fix its flaws.
>>
>> If the replacement needs a lot of work to fit our needs, the value
>> proposition becomes negative.
>>
>> A JSON parser shouldn't require much maintenance, as JSON is simple,
>> doesn't change, and parsing has few system dependencies.
> 
> Let me suggest this crazy idea: have you guys considered breaking
> compatibility?

As in, requiring QMP clients to send "quotes" rather than 'quotes'?
It's worth considering (we already guarantee that our output is strict
JSON, and that the 'quotes' on input is merely for convenience).  If we
want to go that route, than 2.5 should document loudly that we are
deprecating 'quotes' in QMP, so that 2.6 can actually remove it when
switching to yajl.  And as single quotes appears to be the only JSON
extension we have been relying on, I think that would indeed free us
from having to wait for a yajl release or carry our own yajl fork.

Interesting idea; I'm still thinking whether it would help us more than
it would hurt lazy clients that were depending on the extension.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] RFC: libyajl for JSON

[Daniel P. Berrange]

On Tue, Nov 03, 2015 at 06:40:32AM -0700, Eric Blake wrote:
> On 11/03/2015 06:19 AM, Luiz Capitulino wrote:
> > On Tue, 03 Nov 2015 08:17:58 +0100
> > Markus Armbruster <armbru@redhat.com> wrote:
> > 
> >>> So at this point, I want to see if lloyd makes any progress towards an
> >>> actual yajl release and/or adding a co-maintainer, before even trying to
> >>> get formal upstream support for single quoting.  We could always create
> >>> a git submodule with our own choice of fork (since there are already
> >>> forks that do single-quote parsing) - but the mantra of 'upstream first'
> >>> has a lot of merit (I'm reluctant to fork without good reason).
> >>
> >> The value proposition of replacing our flawed JSON parser isn't in
> >> saving big on maintenance, it's in not having to find and fix its flaws.
> >>
> >> If the replacement needs a lot of work to fit our needs, the value
> >> proposition becomes negative.
> >>
> >> A JSON parser shouldn't require much maintenance, as JSON is simple,
> >> doesn't change, and parsing has few system dependencies.
> > 
> > Let me suggest this crazy idea: have you guys considered breaking
> > compatibility?
> 
> As in, requiring QMP clients to send "quotes" rather than 'quotes'?
> It's worth considering (we already guarantee that our output is strict
> JSON, and that the 'quotes' on input is merely for convenience).  If we
> want to go that route, than 2.5 should document loudly that we are
> deprecating 'quotes' in QMP, so that 2.6 can actually remove it when
> switching to yajl.  And as single quotes appears to be the only JSON
> extension we have been relying on, I think that would indeed free us
> from having to wait for a yajl release or carry our own yajl fork.

FWIW, I think it is not unreasonable to consider dropping support for
'quotes' on the basis that I'd expect the overwhealming majority of apps
that talk to QMP are probably using a JSON library rather than home-grown
JSON code. As such I'd be surprised if any non-trivial app is actually
using 'quotes' instead of "quotes", as JSON libraries will be compliant
with the spec rather than using QEMU's extension for this.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] RFC: libyajl for JSON

[Luiz Capitulino]

On Tue, 3 Nov 2015 14:38:38 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> 
> 
> On 03/11/2015 14:19, Luiz Capitulino wrote:
> > > The value proposition of replacing our flawed JSON parser isn't in
> > > saving big on maintenance, it's in not having to find and fix its flaws.
> > > 
> > > If the replacement needs a lot of work to fit our needs, the value
> > > proposition becomes negative.
> > > 
> > > A JSON parser shouldn't require much maintenance, as JSON is simple,
> > > doesn't change, and parsing has few system dependencies.
> > 
> > Let me suggest this crazy idea: have you guys considered breaking
> > compatibility?
> 
> Can you explain why that would make sense? :)  (Especially since there
> is another extension---JSON5---that does exactly what we're doing, so it
> probably wasn't that stupid an idea).

Let's be pragmatic. *If* this is the only issue stopping us from
dropping our own parser in favor of something more widely used and
*if* libvirt doesn't make use of the feature, it's something we
should strongly consider.

Re: [Qemu-devel] RFC: libyajl for JSON

[Luiz Capitulino]

On Tue, 3 Nov 2015 06:40:32 -0700
Eric Blake <eblake@redhat.com> wrote:

> On 11/03/2015 06:19 AM, Luiz Capitulino wrote:
> > On Tue, 03 Nov 2015 08:17:58 +0100
> > Markus Armbruster <armbru@redhat.com> wrote:
> > 
> >>> So at this point, I want to see if lloyd makes any progress towards an
> >>> actual yajl release and/or adding a co-maintainer, before even trying to
> >>> get formal upstream support for single quoting.  We could always create
> >>> a git submodule with our own choice of fork (since there are already
> >>> forks that do single-quote parsing) - but the mantra of 'upstream first'
> >>> has a lot of merit (I'm reluctant to fork without good reason).
> >>
> >> The value proposition of replacing our flawed JSON parser isn't in
> >> saving big on maintenance, it's in not having to find and fix its flaws.
> >>
> >> If the replacement needs a lot of work to fit our needs, the value
> >> proposition becomes negative.
> >>
> >> A JSON parser shouldn't require much maintenance, as JSON is simple,
> >> doesn't change, and parsing has few system dependencies.
> > 
> > Let me suggest this crazy idea: have you guys considered breaking
> > compatibility?
> 
> As in, requiring QMP clients to send "quotes" rather than 'quotes'?

Yes.

> It's worth considering (we already guarantee that our output is strict
> JSON, and that the 'quotes' on input is merely for convenience).  If we
> want to go that route, than 2.5 should document loudly that we are
> deprecating 'quotes' in QMP, so that 2.6 can actually remove it when
> switching to yajl.  

I'd go for more craziness: disable the feature for the next release
and watch. If we suddenly found out that there are some big users of
the feature, we back off and re-enable it on a -stable release.

> And as single quotes appears to be the only JSON
> extension we have been relying on, I think that would indeed free us
> from having to wait for a yajl release or carry our own yajl fork.
> 
> Interesting idea; I'm still thinking whether it would help us more than
> it would hurt lazy clients that were depending on the extension.

The point is, apart from libvirt, I've never heard of any serious QMP
client that's not a simple automation script. And I'd bet those are
sending in "quotes" to talk to QMP.

Re: [Qemu-devel] RFC: libyajl for JSON

[Paolo Bonzini]

On 03/11/2015 14:46, Luiz Capitulino wrote:
>> > Can you explain why that would make sense? :)  (Especially since there
>> > is another extension---JSON5---that does exactly what we're doing, so it
>> > probably wasn't that stupid an idea).
> Let's be pragmatic. *If* this is the only issue stopping us from
> dropping our own parser in favor of something more widely used and
> *if* libvirt doesn't make use of the feature, it's something we
> should strongly consider.

I'm not sure what's so bad about our parser that makes it worthwhile to:

1) uglify all tests and make them inconsistent with the QAPI schemas,
which also uses single-quoted strings

2) waste time finding a replacement for % interpolation (the best
replacement here would be to rewrite the tests in Python IMHO, but
that's not a small ask)

Just let's remove the weird (to not say worse) usage of QDict/QList to
store tokens...

Paolo

Re: [Qemu-devel] RFC: libyajl for JSON

[Luiz Capitulino]

On Tue, 3 Nov 2015 14:53:59 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 03/11/2015 14:46, Luiz Capitulino wrote:
> >> > Can you explain why that would make sense? :)  (Especially since there
> >> > is another extension---JSON5---that does exactly what we're doing, so it
> >> > probably wasn't that stupid an idea).
> > Let's be pragmatic. *If* this is the only issue stopping us from
> > dropping our own parser in favor of something more widely used and
> > *if* libvirt doesn't make use of the feature, it's something we
> > should strongly consider.
> 
> I'm not sure what's so bad about our parser that makes it worthwhile to:

It's not that it's bad. It's about the advantages of dropping hundreds of
lines of NIH code and switching to something more widely used. Also,
any maintenance time we spend on libyajl will also be automatically
enjoyed by libvirt which is excellent.

On the other hand, I don't want to push too hard for it because I do
recognize that switching has a cost and I won't be able to help with
that myself.

> 1) uglify all tests and make them inconsistent with the QAPI schemas,
> which also uses single-quoted strings

This doesn't seem hard to fix, we could pre-process the test files,
say in Python, to add the needed escaping.

> 2) waste time finding a replacement for % interpolation (the best
> replacement here would be to rewrite the tests in Python IMHO, but
> that's not a small ask)

Is this only used by tests? Can you give an example of this feature?

> 
> Just let's remove the weird (to not say worse) usage of QDict/QList to
> store tokens...
> 
> Paolo
> 

Re: [Qemu-devel] RFC: libyajl for JSON

[Paolo Bonzini]

On 03/11/2015 15:26, Luiz Capitulino wrote:
> > 1) uglify all tests and make them inconsistent with the QAPI schemas,
> > which also uses single-quoted strings
>
> This doesn't seem hard to fix, we could pre-process the test files,
> say in Python, to add the needed escaping.

I'm talking about qtests:

    qmp_discard_response("{'execute':'change', 'arguments':{"
                         " 'device':'floppy0', 'target': %s, 'arg': 'raw' }}",

versus

    qmp_discard_response("{\"execute\":\"change\", \"arguments\":{"
                         " \"device\":\"floppy0\", \"target\": %s, \"arg\": \"raw\" }}",


> > 2) waste time finding a replacement for % interpolation (the best
> > replacement here would be to rewrite the tests in Python IMHO, but
> > that's not a small ask)
> 
> Is this only used by tests? Can you give an example of this feature?

See above.  Notice how the %s becomes a JSON string, with automatic
backslash escaping.

Paolo

Re: [Qemu-devel] RFC: libyajl for JSON

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Tue, 3 Nov 2015 14:53:59 +0100
> Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>> On 03/11/2015 14:46, Luiz Capitulino wrote:
>> >> > Can you explain why that would make sense? :)  (Especially since there
>> >> > is another extension---JSON5---that does exactly what we're doing, so it
>> >> > probably wasn't that stupid an idea).
>> > Let's be pragmatic. *If* this is the only issue stopping us from
>> > dropping our own parser in favor of something more widely used and
>> > *if* libvirt doesn't make use of the feature, it's something we
>> > should strongly consider.
>> 
>> I'm not sure what's so bad about our parser that makes it worthwhile to:
>
> It's not that it's bad. It's about the advantages of dropping hundreds of
> lines of NIH code and switching to something more widely used.

I wish we would've / could've avoided NIH back then, but I'm not sure
getting rid of this piece of NIH now is worthwhile.

json-{lexer,parser,streamer}.[ch] together have 949 SLOC.  I'm not
counting tests, because we'd most likely keep them anyway.  This is less
than 0.1% of the QEMU source code :)

Maintenance hasn't been costing us a fortune exactly: 40 commits in six
years.

I'm annoyed by its relative shoddiness, but the prospect of having to
fix it up isn't something that keeps me up at night.

>                                                                Also,
> any maintenance time we spend on libyajl will also be automatically
> enjoyed by libvirt which is excellent.

Excellent indeed *if* upstream is responsive.

> On the other hand, I don't want to push too hard for it because I do
> recognize that switching has a cost and I won't be able to help with
> that myself.
>
>> 1) uglify all tests and make them inconsistent with the QAPI schemas,
>> which also uses single-quoted strings
>
> This doesn't seem hard to fix, we could pre-process the test files,
> say in Python, to add the needed escaping.

The test files are actually C code... sure you want to mangle C code in
Python?

>> 2) waste time finding a replacement for % interpolation (the best
>> replacement here would be to rewrite the tests in Python IMHO, but
>> that's not a small ask)
>
> Is this only used by tests?

No.

>                             Can you give an example of this feature?

Yes:

    static QDict *build_qmp_error_dict(Error *err)
    {
        QObject *obj;

        obj = qobject_from_jsonf("{ 'error': { 'class': %s, 'desc': %s } }",
                                 ErrorClass_lookup[error_get_class(err)],
                                 error_get_pretty(err));

        return qobject_to_qdict(obj);
    }

Builds a QDict with a single key "error".  Its value is a QDict with key
"class", value ErrorClass_lookup[error_get_class(err)], and key "desc",
value error_get_pretty(err), where "value" really means the C string
quoted for JSON and wrapped in a QString.

As I wrote elsewhere in the thread, we could build this by hand.  Much
less readable, but that might be tolerable, as the feature isn't widely
used.  I'm not thrilled about it, though.  It's too easy to forget the
quoting.

To find more examples, try "git-grep _json[fv]".

>> Just let's remove the weird (to not say worse) usage of QDict/QList to
>> store tokens...

Any replacement effort will have to compete on merits with fixing up
what we have.