From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59532) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SCATj-0005fT-BR for Qemu-devel@nongnu.org; Mon, 26 Mar 2012 10:01:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SCATe-0008Sj-5A for Qemu-devel@nongnu.org; Mon, 26 Mar 2012 10:00:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:26571) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SCATd-0008SU-T7 for Qemu-devel@nongnu.org; Mon, 26 Mar 2012 10:00:54 -0400 Message-ID: <4F70776A.1010100@redhat.com> Date: Mon, 26 Mar 2012 16:04:26 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <4F702B56.8030400@redhat.com> <20120326094610.2ee5abc8@doriath.home> <4F706B80.9080402@redhat.com> <20120326102814.20e2cb87@doriath.home> In-Reply-To: <20120326102814.20e2cb87@doriath.home> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] Ignoring errno makes QMP errors suck List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: Qemu-devel@nongnu.org, Anthony Liguori Am 26.03.2012 15:28, schrieb Luiz Capitulino: > On Mon, 26 Mar 2012 15:13:36 +0200 > Kevin Wolf wrote: > >> Am 26.03.2012 14:46, schrieb Luiz Capitulino: >>> On Mon, 26 Mar 2012 10:39:50 +0200 >>> Kevin Wolf 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