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

[Luiz Capitulino <lcapitulino@redhat.com>]

On Tue, 04 May 2010 16:56:19 -0500
Anthony Liguori <anthony@codemonkey.ws> 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

> >   So, the first question is: would you map EIO to an QError? Just like
> > you did for SocketIOError? If so, how would you know which operation
> > has failed? Would you put its name in source? Or have an additional
> > 'operation' key?
> >
> >   A related problem is not to degrade the current set of error messages
> > we offer to the users. For do_savevm()'s 'save state' operation, current
> > message is:
> >
> >     "Error -7 while writing VM"
> >
> >   With StateVmSaveFailed it becomes:
> >
> >     "Failed to save VM state ("EIO")"
> >    
> 
> I don't think this is that useful to preserve because as you said, it 
> could be one of three things.

 But as I said above, we have to say which operation has failed, otherwise
it's very difficult to diagnose the problem.