From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:36093) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SCA8q-00070v-Co for Qemu-devel@nongnu.org; Mon, 26 Mar 2012 09:39:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SCA8g-0001DG-Ay for Qemu-devel@nongnu.org; Mon, 26 Mar 2012 09:39:23 -0400 Received: from mail-ob0-f173.google.com ([209.85.214.173]:61913) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SCA8g-0001Cr-4y for Qemu-devel@nongnu.org; Mon, 26 Mar 2012 09:39:14 -0400 Received: by obbwd20 with SMTP id wd20so6028043obb.4 for ; Mon, 26 Mar 2012 06:39:12 -0700 (PDT) Message-ID: <4F70717D.1010000@codemonkey.ws> Date: Mon, 26 Mar 2012 08:39:09 -0500 From: Anthony Liguori 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; format=flowed 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: Kevin Wolf , Qemu-devel@nongnu.org On 03/26/2012 08:28 AM, Luiz Capitulino wrote: > 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. > > 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. Note that this whole discussion is somewhat irrelevant. The error gets "converted" in the block code, not in generic QMP code. If you want a error_set_from_errno() function, that's fairly trivial to add although you're missing the opportunity to add more useful information (like the name of the file). Regards, Anthony Liguori