Re: [Qemu-devel] Adding errno to QMP errors

[Anthony Liguori <anthony@codemonkey.ws>]

On 06/18/2012 10:41 AM, Markus Armbruster wrote:
> "Daniel P. Berrange"<berrange@redhat.com>  writes:
>
>> On Fri, Jun 15, 2012 at 11:52:57AM -0500, Anthony Liguori wrote:
>>> On 06/13/2012 12:49 PM, Luiz Capitulino wrote:
>>> No, you're confusing things I think.  { 'error': 'NoSpace' } is bad.
>>> errno is not an intrinsically bad thing but errno critically relies
>>> on the *caller* to understand the context that the error has
>>> occurred in.  Just returning { 'error': 'NoSpace' } is not good
>>> enough in QMP because the caller doesn't know the context.  What was
>>> the command doing such that that error was returning?
>>>
>>> In many cases, errno has different meanings depending on the
>>> context.  EINVAL is a good example of this.
>>>
>>> The devil is in the details here.  Having an error like:
>>>
>>> { 'error': 'OpenFileFailed', 'file': 'filename', 'mode': 'r/w',
>>> 'os_error': 'enospc' }
>>>
>>> is actually pretty reasonable for something like a memory dump
>>> command where the user specifies a file.
>>
>> I can't help thinking that we're still over-engineering the error
>> reporting for QMP, and that really all we need is a reasonably
>> coarse error code/class, and an informal string.
>>
>> eg,
>>
>>     { 'error': "SystemError", msg = "failed to open file '/foo/bar' for writing: no space on device" }
>>
>>     { 'error': "DNSError", msg = "unable to resolve hostname 'foo': cannot reach nameserver"}
>>
>>     etc
>>
>> In libvirt we started with a ridiculously complicated virErrorPtr
>> struct, which no one ever remembered to fill our details in, or
>> filledout details inconsistently. These days we only ever bother
>> with a coarse error class, and a string, and in the case of a
>> system error, we also include the raw errno value.
>
> Good match for real-world error handling, which is usually a minor
> variation of
>
>      if (didn't work)
>          if (retry might fix it)
>              retry
>          else if (I got a plan B to try)
>              try plan B
>          else
>              punt to human
>
> Error information used:
>
> 1. whether it failed
>
> 2. whether a failure is transient or permanent
>
> 3. a description of the failure fit for human consumption
>
>> Pretty much all common APIs / languages focus primarily on just
>> an error code/class and a informal string too, with the odd
>> exception eg Python's OSException provides you the errno value
>> too
>>
>> Are any users of QMP actually asking for this kind of advanced
>> error reporting ?  From libvirt's POV we're perfectly content
>> with just an error class&  string.
>
> Real users, please, not theoretical ones.

Irrespective of anything else, I think it's safe to say the experiment of "rich 
errors" has been a failure.  We still have way too many places using error_report.

As I mentioned in another thread, I think we should:

1) Introduce a GENERIC_ERROR QError type.  It could have a 'domain' and a 'msg' 
field.

2) Focus on converting users of error_report over to use propagated Error objects.

We shouldn't/can't change existing QError users.  We also shouldn't consider 
changing the wire protocol.  But for new error users, we should/can relax the 
reported errors.

We need a clear support policy on whether the contents of 'msg' are stable or 
not too.

Regards,

Anthony Liguori