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

[Paolo Bonzini]

Il 31/05/2012 16:31, Luiz Capitulino ha scritto:
>>>> Errors are not QAPI-ized yet, so we can add errno values to the above
>>>> five errors too.
>>>
>>> We've preferred adding new errors instead of adding errno values to
>>> existing errors. I don't remember exactly why, but I personally don't care.
>>
>> Then let's not do it anymore. :)
> 
> We already did, changing this now will probably be worse.

Wait, I think you're conflating two things.

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.

For example:

    $ sed UUUU
    sed: -e expression #1, char 1: unknown command: `U'
                                   ^^^^^^^^^^^^^^^^     error type
         ^^^^^^^^^^^^^  ^       ^                   ^^^ arguments

in JSON:

    { 'error': 'UnknownCommand',
      'source' : { 'type': 'expression',
                   'data': { 'number': 1, 'char': 1 } },
      'command': 'U'
    }

It does not say

    sed: parsing program: No such system call

(aka ENOSYS).  So far that's obvious. :)  Now let's look at the second part, OS
errors:

    $ 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' }

Here is similar example:

    $ yes | sed p > /dev/full
    sed: couldn't write 1 item to stdout: No space left on device
         ^^^^^^^^^^^^^^                                            error type
                        ^         ^^^^^^  ^^^^^^^^^^^^^^^^^^^^^^^  arguments

    { 'error': 'WriteFailed',
      'file': 'stdout', 'count': 1,
      'os_error': 'enospc' }


Note how restricting everything to a single NoSpaceLeftOnDevice error would
force you to drop information such as filename or count.

Is it clearer?

Paolo