From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:38244) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SCBJK-00050H-Bf for Qemu-devel@nongnu.org; Mon, 26 Mar 2012 10:54:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SCBJI-0001AI-DU for Qemu-devel@nongnu.org; Mon, 26 Mar 2012 10:54:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38117) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SCBJI-00019d-4V for Qemu-devel@nongnu.org; Mon, 26 Mar 2012 10:54:16 -0400 Date: Mon, 26 Mar 2012 11:54:19 -0300 From: Luiz Capitulino Message-ID: <20120326115419.212cbb11@doriath.home> In-Reply-To: <4F708197.10802@redhat.com> References: <4F702B56.8030400@redhat.com> <20120326094610.2ee5abc8@doriath.home> <4F706B80.9080402@redhat.com> <20120326102814.20e2cb87@doriath.home> <4F70776A.1010100@redhat.com> <20120326113312.145235e6@doriath.home> <4F708197.10802@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII 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: Kevin Wolf Cc: Qemu-devel@nongnu.org, Anthony Liguori On Mon, 26 Mar 2012 16:47:51 +0200 Kevin Wolf wrote: > Am 26.03.2012 16:33, schrieb Luiz Capitulino: > > On Mon, 26 Mar 2012 16:04:26 +0200 > > Kevin Wolf wrote: > > > >>>> 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). > > > > Yes, but it's not a severe problem. Not yet. Because most of the time the > > error context can be inferred from the command's context. Say, you fail to > > create a file where only one file could be created. > > So how do you like the 'transaction' command? :-) That's the only exception I think. If this is hurting you, then we have two options: think of a way to add optional parameters to errors right now or add new QERR_s macros that accept the filename argument (note that we wouldn't add a new error, just a macro that accepts a new argument). > But even if you could infer the context, the possible error details may > still differ in the possible contexts. With -blockdev this will become > even more fun, because even for bdrv_open you may or may not have a > filename. Maybe you rather have a hostname, port and sheepdog volume ID. > Or directory name, FAT type and stuff. Right, and the solution for this is really extending the error infra to accept optional parameters. Although I don't know how to support different error messages. > > But yes, we need to add a way for errors to accept optional parameters. > > > > I have a series that's almost done that moves all errors to a qapi-error.json > > file and auto-generates the QErrors tables. Maybe that's a first step towards > > more flexible errors. > > > >> 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. > > > > I think the end result is the same, but I prefer PermissionDenied as the > > top level because it's slightly simpler. > > How would you do the conversion in a compatible way? Returning additional errors is not incompatible.