On Thu, 31 May 2012 16:54:47 +0200
Paolo Bonzini <pbonz...@redhat.com> wrote:
> 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.
[ cutting examples, hope to not cut anything important ]
> (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' }
I'm more or less fine with either way. I have some concerns on carrying
cruft if we keep changing our minds, but that's probably affordable. I also
think Anthony's way (ie. what we do today) is simpler, but again I wouldn't
oppose to do what you're suggesting.
Actually, I did propose something similar in the past but Anthony objected.
> 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.
We could have optional arguments to NoSpaceLeftOnDevice. Could bloat the error,
or not (as most error scenarios can be common to each other).
