[Qemu-devel] Re: [FOR 0.12 PATCH 18/18] QMP: add human-readable description to error response

[Markus Armbruster <armbru@redhat.com>]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Mon,  7 Dec 2009 21:37:16 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> -{ "error": { "class": json-string, "data": json-value }, "id": json-value }
>> +{ "error": { "class": json-string, "data": json-value, "desc": json-string },
>> +  "id": json-value }
>>  
>>   Where,
>>  
>>  - The "class" member contains the error class name (eg. "ServiceUnavailable")
>>  - The "data" member contains specific error data and is defined in a
>>    per-command basis, it will be an empty json-object if the error has no data
>> +- The "desc" member is a human-readable error message.  Clients should
>> +  not attempt to parse this message.
>>  - The "id" member contains the transaction identification associated with
>>    the command execution (if issued by the Client)
>
>  As we've talked on irc, I don't agree with this change.
>
>  Basically, adding 'desc' to the standard error message introduces all
> the problems we've discussed about free-form English strings.

I disagree.  See below.

>  I feel that QError is becoming the worst of all proposals.

I'm not exactly thrilled about it either, but it could clearly be worse
in many ways, so it can't be the worst of all :)

>  I agree with you that it's not as easy as it should be to report errors,
> but as we're targeting on Clients I was convinced that we could not have the
> best API internally but offer a good interface for Clients.
>
>  Now, having 'desc' as part of the standard protocol is like not having
> the best API internally and offering a bad interface for Clients.
>
>  Not to mention that those strings can't be modified when the protocol
> becomes stable

This is incorrect.  We explicitly threaten client writers that these
strings are not to be interpreted, in qmp-spec.txt: "Clients should not
attempt to parse this message."  For me, that implies that they can
change at any time.  Maybe we could use even stronger language there.

>                and we're probably talking about dozens if not a hundred
> of strings. Ok, there isn't a reason to change them often, but it's
> still one more thing to maintain.
>
>  Having said that, I would agree to have 'desc' as part of debug
> information. I have patches in my tree which adds CONFIG_DEBUG_QMP,
> if one enables it information about the error location will also
> be part of the error message. I would agree having 'desc' there
> too.

Debugging is one use for human-readable text in the error reply.  But it
is *not* the only use.  We discussed this at length in October[*], and
again in November[**].  Any new arguments?


[*] Sub-thread starting at
http://lists.gnu.org/archive/html/qemu-devel/2009-10/msg01245.html

[**] Sub-thread starting at
http://lists.gnu.org/archive/html/qemu-devel/2009-11/msg01071.html