Daniel P. Berrangé <berrange@xxxxxxxxxx> writes:
> On Tue, Feb 04, 2020 at 09:11:27AM +0100, Markus Armbruster wrote:
>> Eric Blake <eblake@xxxxxxxxxx> writes:
>>
>> > [adding Markus]
>> >
>> > On 2/3/20 4:13 AM, Daniel P. Berrangé wrote:
>> >> On Fri, Jan 31, 2020 at 06:44:42AM -0600, Eric Blake wrote:
>> >>> On 1/31/20 4:38 AM, Peter Luo wrote:
>> >>>
>> >>>> error: internal error: cannot parse json {"execute": "block-commit", "arguments": { "device": "drive-virtio-disk2", "job-id": "job100", "base":'json:{"encrypt.key-secret":"vol-38973xjl.secret","driver":"qcow2","file":{"driver":"file","filename":"/pitrix/data/container/vol-38973xjl.img"}}', "top": "/pitrix/data/container/vol-38973xjl_ss-2tw7v0mm.img"}}: lexical error: invalid char in json text.
>> >>>>
>> >>>> , "job-id": "job100", "base":'json:{"encrypt.key-secret":"vo
>> >>>>
>> >>>> (right here) ------^
>> >>>>
>> >>>
>> >>> qemu's QMP language has an extension where it accepts 'string' in addition
>> >>> to the JSON "string". But it uses a hand-rolled parser, so it can do
>> >>> whatever it wants.
>> >>
>> >> Can we deprecate & remove this extension in QEMU ?
>
> [snip]
>
>> >> On the flip side, if we're going to support extensions like single quoting,
>> >> then we should make it clear to applications that this is not really JSON
>> >> and that they need to provide an impl that is 100% matching QEMU's dialect.
>> >> This effectively means they need just import a copy of QEMU's code.
>>
>> To the best of my knowledge, the JSON parser interprets any valid strict
>> JSON input in accordance to RFC 8259. In other words, you don't notice
>> the extensions unless you use them, or rely on invalid strict JSON to be
>> rejected.
>>
>> Peter Luo's input uses one of QEMU's JSON parser's extensions like this:
>>
>> "base":'json:{...}'
>>
>> This is not valid strict JSON. Libvirt's JSON parser doesn't accept it.
>>
>> The problem is not presence of extensions in QEMU, it's the use of these
>> extensions in input for libvirt. Removing the extensions from QEMU will
>> not affect the error. Removing their use from the input will.
>
> The issue that I see is that QEMU accepts this input string when it
> parsers JSON provided by the user. This in turn means the user has
> an expectation that other tools based on QEMU will accept this exact
> same document. This is not the case because the other tools are
> using a stricter impl of JSON.
>
> IOW QEMU's extensions have mislead the users into believing their
> JSON input is valid for any tool based on QEMU.
>
> Thus overall I think it would be beneficial for any places where
> QEMU accepts JSON from external users or apps, to be restricted to
> common JSON syntax only, without any QEMU specific extensions.
I don't think the JSON extensions are much of a problem. But I also
don't think they provide much value in the external interface. Feel
free to post patches that deprecate them there.
Aside: deprecating stuff in QMP is awkward, because we don't have a good
way to tell users. We do it anyway.
