Re: [Qemu-devel] [PATCH 02/14] qerror: add new errors

[Kevin Wolf <kwolf@redhat.com>]

Am 31.05.2012 18:08, schrieb Luiz Capitulino:
> On Thu, 31 May 2012 17:49:42 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
>> Il 31/05/2012 17:44, Luiz Capitulino ha scritto:
>>>> One is "do not shoehorn errors into errno values".  So for QOM invalid values we
>>>> have PropertyValueBad, not a generic InvalidArgument value.  We convert everything
>>>> to Error rather than returning negative errno values and then returning generic
>>>> error codes, because those would be ugly and non-descriptive.  I agree with that.
>>>>
>>>> The other is "when errors come straight from the OS, _do_ use errno values".
>>>> This is for OpenFileFailed, for the new socket errors and so on.  This is what
>>>> I am proposing.
>>>>
>>>> These two rules together match what most other programs do.
>>>>
>>>>     $ echo | sed p > /dev/full
>>>>     sed: couldn't flush stdout: No space left on device
>>>>          ^^^^^^^^^^^^^^                                 error type
>>>>                         ^^^^^^  ^^^^^^^^^^^^^^^^^^^^^^^ arguments
>>>>
>>>> That would become, in JSON:
>>>>
>>>>     { 'error': 'FlushFailed',
>>>>       'file': 'stdout',
>>>>       'os_error': 'enospc' }
>>>
>>> Actually, I did propose something similar in the past but Anthony objected.
>>
>> Looks like in the meanwhile we moved closer to this mechanism
>> (OpenFileFailed, Sock*, etc.), except we have no clear way to identify
>> _what_ error actually happened rather than _where_.
> 
> In fact, it's the other way around. OpenFileFailed, for example, is an old
> error. We thought it would be ok to have it that way (also because of shallow
> QMP conversion, as it would take ages to convert all possible errors). But today
> it's clear it's bad and we're trying to move away from it.

It's not all that clear for me. Or actually, it is rather clear that
it's basically the right thing, but some additional information is required.

> The socket ones repeat this, but it's probably because people usually go
> for the generic error first because having detailed errors with qerror is
> cumbersome. I have some ideas to improve it that could _mitigate_ that problem,
> like having a schema file qapi-errors.json:
> 
>  { 'error': 'InvalidParameter', 'data': { 'name': 'str' } }

Errors that are as simple as that are useless if they are all you get.
Even for InvalidParameter this is true when you have more than a flat
arguments dict.

Maybe what is required in order to fully describe an error is a whole
chain of error objects that describe which error caused which other
action to fail:

{ 'error': 'TransactionCommandFailed', 'data': {
  'command': 'blockdev-snapshot-sync',
  'cause': {
      'error': 'FileOpenFailed', 'data' : {
        'filename': '/tmp/foo.img',
        'cause': {
          'error': 'PermissionDenied', 'data': {}
        }
      }
   }
}

Not sure if that would be easy to process for a QMP client, though...

Kevin