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

[Paolo Bonzini]

Il 31/05/2012 18:08, Luiz Capitulino ha scritto:
> On Thu, 31 May 2012 17:49:42 +0200
> Paolo Bonzini <address@hidden> 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.

Is it so clear?  It's bad that we do not have a way to report OS errors
precisely.  But what _is_ absolutely clear, is that we need to report
the "what" (EACCES etc.), but also either the "where" (filename) or the
"when" (bind/connect/...)

Adding an errno to the *Failed errors really seems like a no-brainer to me.

Paolo