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

[Markus Armbruster]

Anthony Liguori <address@hidden> writes:

> Markus Armbruster wrote:
>>> It's highly likely that for this last case, you'd want generic code to
>>> generate this error.  Further more, in order to generate the error
>>> message for a user, you need to know what device does not have a
>>> functioning driver.  You may say it's obvious for something like info
>>> balloon but it's not so obvious if you assume we support things other
>>> than just virtio-pci-balloon.  For instance, with s390, it's a
>>> different balloon driver.  For xenner/xenpv, it's a different driver.
>>> It really suggests we should send the qdev device name for the current
>>> balloon driver.
>>>     
>>
>> This is your argument for structured error reporting.  It boils down to
>> client localization.
>>
>> I challenge the idea that we should even try to get that covered now.
>> We need to get the basics right, and the only way to do that is to use
>> them in anger.  The more baggage we design into the thing before we use
>> it, the higher the risk that we screw it up.
>>   
>
> Current proposal is:
>
> qemu_error_new(QERR_DEVICE_NOT_FOUND, "00:11:22");
>
> Alternative proposal[1]:
>
> qemu_error_new(QERR_DEVICE_NOT_FOUND, "Could not find PCI device 00:11:22");
>
> Alternative proposal[2]:
>
> qemu_error_new(QERR_DEVICE_NOT_FOUND);
>
> Your argument has merit if you're countering with [2].  [1] is
> incorrect because it makes localization impossible.  This isn't an
> evolutionary feature, this is a hard requirement as far as I'm
> concerned.  At least [2] allows localization.
>
> I dislike [2] though because it means that our errors are not going to
> be able to be generic enough.  I really want the information of which
> device was not found to be part of the error message.
>
>>> I've described my requirements for what a client can do.  I'd like to
>>> understand how you disagree.
>>>     
>>
>> I have essentially two complaints:
>>
>> * I'm afraid we've made QError much more complex than it need be (item 1
>>   above), at least for a first iteration of the protocol.
>>
>> * The design has shortcomings that effectively require clients to know
>>   all errors (items 2 & 3 above).
>>   
>
> My main concern is that we're making an important feature impossible.
> If we're arguing for errno style errors verses structured exceptions,
> I think that's a more reasonable argument.  I'm really concerned about
> the long term ramifications about combining errno style errors with
> free formed, non-localized text error messages.

We have:

(1) machine-readable error code

(2) human-readable error message

(3) machine-readable additional error data

The old monitor prints just (3).

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.

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

>>> I would never pass an error string from a third party directly to a
>>> user.  I doubt you'll find a lot of good applications that do.  From a
>>> usability perspective, you need to be in control of your interactions
>>> with the user.  They grammar needs to be consistent and it has to be
>>> localized.  The best you would do with a third party string is log it
>>> to some log file for later examination by support.  In that case,
>>> dumping the class code and the supporting information in JSON form is
>>> just as good as a text description.
>>>     
>>
>> How should the application report an error it doesn't know to the user?
>>   
>
> 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?

>>>> If we later realize that this solution was *too* stupid, we can simply
>>>> add a data member to the error object.
>>>>         
>>> It's never that easy because a management tool has to support a least
>>> common denominator.
>>>     
>>
>> If we build complex solutions for anticipated needs, we run a high risk
>> of missing real needs.  And then we'll evolve the protocol "sideways",
>> which is even less easy for management tools than evolving "upwards".
>>
>> We'll iterate anyway, so why not embrace it?  Start with something
>> simple and functional, then extend it to address what's found lacking.
>>   
>
> I'm not arguing for a mythical use-case that could come in five years.
> This is something I want to support right now.
>
> 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).

> 2) Client developers should not have to parse an english language,
> subject to change, string in order to get more information about an
> error.  IOW, the error object should always produce the same
> descriptive message regardless of where or how it's generated as long
> as it's fields are equal.
>
> For 2, there are two ways to do this:  have an error message table
> based on error codes with no structured data or have an error template
> table based on error codes and a dictionary of optional data.
>
> The later is a superset of the former so it seems to be really obvious
> to start with the later.