From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56415) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ShQlX-0002EU-F2 for qemu-devel@nongnu.org; Wed, 20 Jun 2012 15:40:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ShQlV-0001oF-7M for qemu-devel@nongnu.org; Wed, 20 Jun 2012 15:40:34 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58972) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ShQlU-0001nn-Va for qemu-devel@nongnu.org; Wed, 20 Jun 2012 15:40:33 -0400 Date: Wed, 20 Jun 2012 16:40:43 -0300 From: Luiz Capitulino Message-ID: <20120620164043.2e1ff013@doriath.home> In-Reply-To: <4FE21A70.2060107@codemonkey.ws> 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> <20120615170350.GO16777@redhat.com> <4FDF7418.9070206@codemonkey.ws> <20120619101229.52273d97@doriath.home> <20120620144838.2abb5a4f@doriath.home> <4FE21A70.2060107@codemonkey.ws> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC] Fixing the error failure List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Kevin Wolf , Paolo Bonzini , Anthony Liguori , Markus Armbruster , qemu-devel@nongnu.org On Wed, 20 Jun 2012 13:46:08 -0500 Anthony Liguori wrote: > On 06/20/2012 12:48 PM, Luiz Capitulino wrote: > > Yet another thread fork. > > > > After talking with Daniel and Markus about QMP errors (which is not just about > > QMP, as this affects QEMU as whole), I've put together the proposal below. > > > > I'll discuss three points. First, the error format and classes. Second, the > > internal API and third compatibility. > > > > Don't be afraid about the length of this email, only the first section is long > > but it mostly contains error classes listings. > > > > 1. Error format and classes > > > > We can keep the same error format we have today, which is: > > > > { "error": { "class": json-string, > > "data": json-object, > > "desc": json-string }, "id": json-value } > > > > where 'data', 'desc' and 'id' are optional fields. > > > > However, we'd change how we use 'desc' and our error classes. 'desc' would > > become a string which is filled by a printf-like function (see section 2) and > > we'd replace all error classes we have today by the following ones: > > > > o ParameterError: any error which involves a bad parameter. Replaces > > InvalidParameter, InvalidParameterCombination, InvalidParameterType, > > InvalidParameterValue, MissingParameter > > > > o SystemError: syscall or library errors. Replaces BufferOverrun, > > IOError, OpenFileFailed, PermissionDenied, TooManyFiles, > > SockConnectInprogress, SockConnectFailed, SockListenFailed, > > SockBindFailed, SockCreateFailed. > > > > This error can include an optional 'os-error' field in the 'data' > > member (see section 2). > > > > o QEMUError: errors that are internal to QEMU, like AmbiguousPath, > > BadBusForDevice, BusNoHotplug, BusNotFound, CommandDisabled, > > CommandNotFound, DuplicateId, FeatureDisabled, JSONParseError, > > JSONParsing, KVMMissingCap, MigrationActive, MigrationNotSupported, > > MigrationExpected, NoBusForDevice, NotSupported, PropertyValueBad, > > PropertyValueInUse, PropertyValueNotFound, PropertyValueNotPowerOf2, > > PropertyValueOutOfRange, ResetRequired, SetPasswdFailed, Unsupported, > > VirtFSFeatureBlocksMigration, VNCServerFailed > > > > o UndefinedError: the same it's today, undefined :) > > > > Now, there are two important points to be observed: > > > > - We check for DeviceEncrypted in hmp.c and fetch its parameters. This > > probably indicates that we might want to create specialized classes > > when necessary > > > > - I don't know where to put all the DeviceFoo errors, but they probably fit > > in QEMUError or a new class like DeviceError > > > > 2. Internal API > > > > This is very straightforward: > > > > o error_set_deprecated(); > > > > Current error_set(), case we keep it for compatibility (see section 3). > > > > o error_set(ErrorClass err_class, const char *fmt, ...); > > > > Main function, sets the error classes and allow for a free human-readable > > error message. > > > > o error_set_sys_fmt(int err_no, const char *fmt, ...); > > o error_set_sys(int err_no); > > > > Helpers for setting SystemError errors. error_set_sys() gets the 'desc' > > string from strerror(). > > Um, why not just do: > > #define GENERIC_ERROR "{'class': 'GenericError', 'data': { 'domain': %s, 'msg': %s}" > > And then just use: > > error_set(errp, GENERIC_ERROR, SOME_DOMAIN, "This operation failed!"); > > If you want to make a little wrapper that does: > > static void error_set_generic(Error **errp, const char *domain, const char *msg, > ...); > > That's fine too. Would 'domain' be one of the classes I suggested above? I'm not sure this is better because it suggests that all classes we have today are still valid. My main goal is to simplify, so keep using the current classes goes against that. I think we should deprecate the current errors (vs. adding a new one to the pile). Also, maybe it's just how I'm interpreting it, but GenericError reminds of UndefinedError in the sense that we'd prefer commands to use more specific errors instead. Actually, it's not clear to me when a command should use GenericError vs. a more specific error? > But let's not overdo this and let's absolutely not change existing code! > There's simply no need to go through a convert-everything project here. I'm fine with keeping the current code, but I think this proposal overly simplifies something that we're already overdoing.