Re: [Qemu-devel] [PATCH 1/2] qemu-error: Introduce get_errno_name()

[Markus Armbruster]

Luiz Capitulino <address@hidden> writes:

> On Tue, 04 May 2010 16:56:19 -0500
> Anthony Liguori <address@hidden> wrote:
>
>> On 05/04/2010 03:30 PM, Luiz Capitulino wrote:
>> >
>> >   StateVmSaveFailed is not like CommandFailed, there are five errors
>> > in do_savevm() and StateVmSaveFailed happens to be one of them.
>> >
>> >   But I understand what you mean and I have considered doing something
>> > like it, one of the problems though is that I'm not sure 'source' is
>> > enough to determine where the error has happened.
>> >
>> >   Consider do_savevm() again. We have three 'operations' that might
>> > fail: delete an existing snapshot, save the VM state and create the
>> > snapshot. All those operations can return -EIO as an error.
>> >    
>> 
>> Maybe those three operations should return distinct errnos?
>
>  I don't think this is possible, as we would have to guarantee that no
> function called by a handler return the same errno.
>
>  Taking the block layer as an example. Most block drivers handlers check
> if the driver really exist (-ENOMEDIUM) and if the driver supports the
> operation being requested (-ENOTSUP).
>
>  How can we have unique errnos in this case?
>
>  Also remember that we're only talking about the surface. The call
> chain is deep. We have almost a hundred handlers, they use functions
> from almost all subsystems.
>
>> That way, we can make more useful QErrors.
>
>  Perhaps, the question boils down to how QErrors should be done.
>
>  Today, we're doing it like this, consider handler foo(), it does the 
> following:
>
>       1. connect somewhere
>       2. send some data
>       3. close
>
>  All operations performed can fail and we want to report that. Currently,
> afaiu we're doing the following (Markus correct me if I'm wrong):
>
>       1. ConnectFailed
>       2. SendFailed
>       3. CloseFailed
>
>  An obvious problem is that we don't say why it has failed. But this is
> what errno is for and I thought we could use it someway. The advantage
> of this approach is that, errors are high-level. It's easy to identify
> what went wrong and we can have very good error messages for them.
>
>  Now, if I got it right, you're suggesting we should do:
>
>       1. BadFileDescriptor, Interrupted, NoPermission ...
>          (or anything connect() may return)
>       2. IOError ...
>       3. IOError, BadFileDescriptor
>
>  This makes sense, but if I'm a user (or a QMP client) I don't want this:
>
> (qemu) savevm foobar
> Bad file descriptor
>
>  I'd prefer this instead:
>
> (qemu) savevm foobar
> Can't delete 'foobar': Bad file descriptor
>
>  Or:
>
> (qemu) savevm foobar
> Can't save VM state: I/O Error

These are of the form "what failed: how it failed".

Aside: not mentioned, because it's obvious for monitor commands: where
it failed.  error_report() adds that information, but only when it's not
obvious, see error_print_loc().

We keep these parts separate because squashing them together into a
single QError class multiplies the classes for no good reason, and makes
it needlessly hard to group errors.  E.g. sometimes we care only about
"what failed".  Then a squashed class is most unwelcome, because to
recognize the "what failed" part you have to know all the squashed
classes, hence all possible how-it-faileds, including the ones that will
be invented after your program ships.

[...]