From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58073) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SfaFV-00059S-G5 for qemu-devel@nongnu.org; Fri, 15 Jun 2012 13:23:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SfaFT-00083M-Gn for qemu-devel@nongnu.org; Fri, 15 Jun 2012 13:23:53 -0400 Received: from mx1.redhat.com ([209.132.183.28]:28187) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SfaFT-00082r-8R for qemu-devel@nongnu.org; Fri, 15 Jun 2012 13:23:51 -0400 Message-ID: <4FDB6FA2.7060605@redhat.com> Date: Fri, 15 Jun 2012 19:23:46 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1338387301-10074-1-git-send-email-lcapitulino@redhat.com> <1338387301-10074-3-git-send-email-lcapitulino@redhat.com> <4FC74B1A.8080700@redhat.com> <20120531110608.4dc3fe22@doriath.home> <4FC77F6C.8000008@redhat.com> <20120531113127.1c8aa213@doriath.home> <4FC78637.4040605@redhat.com> <20120613144910.598bfe24@doriath.home> <4FDB6869.1000509@us.ibm.com> <20120615140223.675f6426@doriath.home> In-Reply-To: <20120615140223.675f6426@doriath.home> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] Adding errno to QMP errors List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: Paolo Bonzini , Anthony Liguori , qemu-devel@nongnu.org, armbru@redhat.com Am 15.06.2012 19:02, schrieb Luiz Capitulino: > On Fri, 15 Jun 2012 11:52:57 -0500 > Anthony Liguori wrote: > >> On 06/13/2012 12:49 PM, Luiz Capitulino wrote: >>> On Thu, 31 May 2012 16:54:47 +0200 >>> Paolo Bonzini wrote: >>> >>>> Wait, I think you're conflating two things. >>>> >>>> One is "do not shoehorn errors into errno values". So for QOM invalid values we >>>> have PropertyValueBad, not a generic InvalidArgument value. We convert everything >>>> to Error rather than returning negative errno values and then returning generic >>>> error codes, because those would be ugly and non-descriptive. I agree with that. >>>> >>>> The other is "when errors come straight from the OS, _do_ use errno values". >>>> This is for OpenFileFailed, for the new socket errors and so on. This is what >>>> I am proposing. >>> >>> [...] >>> >>>> $ echo | sed p> /dev/full >>>> sed: couldn't flush stdout: No space left on device >>>> ^^^^^^^^^^^^^^ error type >>>> ^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^ arguments >>>> >>>> That would become, in JSON: >>>> >>>> { 'error': 'FlushFailed', >>>> 'file': 'stdout', >>>> 'os_error': 'enospc' } >>> >>> This is not a new discussion and what we're doing today is to return errno >>> as a QError class name. So, for the example above we'd return something like: >>> >>> { 'error': 'NoSpace' } >> >> >> No, you're confusing things I think. { 'error': 'NoSpace' } is bad. errno is >> not an intrinsically bad thing but errno critically relies on the *caller* to >> understand the context that the error has occurred in. Just returning { >> 'error': 'NoSpace' } is not good enough in QMP because the caller doesn't know >> the context. What was the command doing such that that error was returning? > > The screendump command (not merged yet) can return it during open() or write(). > >> In many cases, errno has different meanings depending on the context. EINVAL is >> a good example of this. >> >> The devil is in the details here. Having an error like: >> >> { 'error': 'OpenFileFailed', 'file': 'filename', 'mode': 'r/w', 'os_error': >> 'enospc' } >> >> is actually pretty reasonable for something like a memory dump command where the >> user specifies a file. > > And WriteFailed, ReadFailed, etc? This is what Paolo is suggesting I think. If > you agree on doing this, then I'll switch the screendump conversion to do this > and we'll have to extend existing errors. > > And, I'm afraid that some not so new qapi conversions did the NoSpace example > above... > >> OTOH, for something complex like live snapshotting which many involve opening >> multiple files, it may not be good enough. > > Kevin, I think you had a different proposal for the live snapshot command? http://lists.gnu.org/archive/html/qemu-devel/2012-06/msg00035.html You mean this one? Note that this wouldn't only be for live snapshots, but a general approach to error handling, even though most command wouldn't nest very deep. I'm not sure whether I'm really proposing this, I just wanted to bring it up as a possible alternative to be discussed. I'm pretty sure that it would work and it would cover everything we need (which I'm not for the other proposals). But I'm not quite sure if it would be easy enough to use for a QMP client. Kevin