On Mon, 26 Mar 2012 15:13:36 +0200
Kevin Wolf<address@hidden> wrote:
Am 26.03.2012 14:46, schrieb Luiz Capitulino:
On Mon, 26 Mar 2012 10:39:50 +0200
Kevin Wolf<address@hidden> wrote:
Hi,
I keep getting reports of problems, with nice error descriptions that
usually look very similar to what I produced here:
{"execute":"blockdev-snapshot-sync","arguments":{"device":"ide0-hd0","snapshot-file":"/tmp/backing.qcow2"}}
{"error": {"class": "OpenFileFailed", "desc": "Could not open
'/tmp/backing.qcow2'", "data": {"filename": "/tmp/backing.qcow2"}}}
Who can tell me what has happened here? Oh, yes, the command failed, I
would have guessed that from the "error" key. But the actual error
description is as useless as it gets. It doesn't tell me anything about
_why_ the snapshot couldn't be created. ("Permission denied" would have
been the helpful additional information in this case)
There's a function called qemu_fopen_err() in the screendump conversion series
that return more specific errors. It will be trivial to add qemu_open_err()
as soon as qemu_fopen_err() is merged.
We're adding a bunch of more precise errors (some map directly to errno). That's
the easy part. The hard part is to convert everything to use them.
Note that while it's true that this shouldn't have leaked to QMP, good error
reporting is a general problem in QEMU.
I guess my point is that we're actually moving backwards here. In HMP
things like this do print the right error message (using error_report).
And the return code is passed all the way down to where the QMP error is
generated, it's just ignored there.
The last time I checked there was no easy way to handle it there because
errno and strerror(-errno) aren't things allowed in QMP messages. This
is the problem for which I wanted to get some attention.
What we're doing now is to add QErrors that map to errno. So, for example,
we have PermissionDenied for EPERM.
I think this is exactly the same thing, except:
1. We don't use the errno number, because this may differ among OSs
2. We don't use the sterrror() string to allow for internationalization,
but we have our own string that should have the same end result
Does the patch that you mentioned add a generic way for adding an
(converted) errno to QMP errors? Or does it split up existing errors
into more and finer grained errors?
The latter. The QMP errors have to be added manually. But it's just a matter
of time to get the most used errnos added.