Luiz Capitulino <lcapitul...@redhat.com> writes: > On Tue, 04 May 2010 16:56:19 -0500 > Anthony Liguori <anth...@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
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. [...]