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

[Daniel P. Berrange]

On Tue, Dec 08, 2009 at 10:11:48AM -0200, Luiz Capitulino wrote:
> On Mon,  7 Dec 2009 21:37:16 +0100
> Markus Armbruster <address@hidden> 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 feel that QError is becoming the worst of all proposals.
> 
>  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 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.

I think it is fine to declare that 'desc' strings are subject to 
arbitrary change. Even if QEMU includes this 'desc' I think in libvirt
will end up doing its own error code -> human string conversion,
simply because we need to translate the strings. So we'd only use
'desc' in any logging calls if it were present.

>  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.


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|