Re: [Qemu-devel] [PATCH 00/10]: QError v4

[Anthony Liguori <anthony@codemonkey.ws>]

Markus Armbruster wrote:
> We have:
>
> (1) machine-readable error code
>
> (2) human-readable error message
>
> (3) machine-readable additional error data
>
> The old monitor prints just (3).
>   

s:(3):(2):

> You propose to have QMP send (1) and (3).  This forces all clients to
> come up with (2) themselves.  Why that's problematic is discussed
> elsewhere in this thread.
>   

As I said else where, sending (2) along with (1) and (3) doesn't bother 
me.  It's whether (2) is generated automatically from (1) and (2) or 
whether within qemu, the caller who creates the error supplies that text 
free hand.  That's what I'm fundamentally opposed to.

> Imagine we already had a QMP that sent (1) and (2), encoded in JSON.
> Then nothing could stop you to add a "data" part holding (3).

If (2) is generated from (1) based on a table, then I agree with you.  
If (2) is open coded, then I disagree fundamentally because you're 
mixing in information from (3) in there.

>   Ergo,
> keeping things simple by sending just (1) and (2) now does not make it
> impossible to send (3) later, hence does not make it impossible to
> provide your important feature in a future iteration.
>   

Let me give a concrete example of where your logic falls apart.  Say you 
start out with:

qemu_error_new(QERR_DEVICE_NOT_READY, "Ballooning is not enabled in the 
guest");

And somewhere else you have:

qemu_error_new(QERR_DEVICE_NOT_READY, "The virtio-net driver is not 
loaded in the guest");

There is no way to unify these into a coherent message.  It's forever 
free form and the there will always be additional information in the 
description that requires parsing from a client.  This is obviously more 
complicated for the client.  It's also impossible to internationalize 
effectively because even if you introduce a data field, you aren't 
passing enough information to generate these messages.  In this case, 
it's a subtle case of grammatical mismatch although both messages are 
saying the exact same thing.

>> An error it doesn't know about is a bug in the application.  Adding a
>> new type of error to a monitor function is equivalent to changing it's
>> behavior.  It should require a versioning change.
>>     
>
> What do you mean by versioning change?  And what does it mean for
> clients?
>   

We really haven't gotten into versioning/feature negotation, but in my 
mind, if we add a new error type for a function, that's the same as 
adding an additional argument.  It's a new feature that requires some 
sort of version/feature mechanism.

>> My basic concerns boil down to:
>>
>> 1) It must be possible to support localization from the very beginning
>>     
>
> If you insist on supporting localization in the client right now,
> despite costs & risks, there's nothing I can do, and nothing more for me
> to say on my first concern (QMP overengineered for a first iteration of
> the protocol).
>   

Your proposal makes it impossible forever so yes, it's a hard 
requirement.  I'm making a lot of attempts to meet you half way but if 
your argument is "I don't like this, it's too complicated, I don't care 
about localization" then I don't think we're going to get over that.

Regards,

Anthony Liguori