Am 26.03.2012 15:28, schrieb Luiz Capitulino: > On Mon, 26 Mar 2012 15:13:36 +0200 > Kevin Wolf <kw...@redhat.com> wrote: > >> Am 26.03.2012 14:46, schrieb Luiz Capitulino: >>> On Mon, 26 Mar 2012 10:39:50 +0200 >>> Kevin Wolf <kw...@redhat.com> 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.
EACCES in this case, but yes. > 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 Yes, I know the reasons why we can't use these options and they are good reasons. If we get something to replace it (so that we can have a errno_to_qmp()), that part should be solved. >> 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. Your PermissionDenied example doesn't really do this. It is a generic error message that may occur in multiple contexts, right? So in one context you may need a file name as additional information, in another context permission for something completely different may be missing (especially if you include EPERM in the same error). I think 'OpenFileFailed' is not too bad, it's just missing a field that gives the detail 'PermissionDenied'. I'm not sure if having 'PermissionDenied' as the top-level error object is the best idea. Kevin