From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:54920) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SuOnM-0004vH-Ur for qemu-devel@nongnu.org; Thu, 26 Jul 2012 10:12:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SuOnD-0005Ay-0Q for qemu-devel@nongnu.org; Thu, 26 Jul 2012 10:12:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37478) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SuOnC-0005AZ-Fo for qemu-devel@nongnu.org; Thu, 26 Jul 2012 10:11:54 -0400 Date: Thu, 26 Jul 2012 11:12:29 -0300 From: Luiz Capitulino Message-ID: <20120726111229.22e5a71e@doriath.home> In-Reply-To: <87fw8eofto.fsf@codemonkey.ws> References: <1343249431-9245-1-git-send-email-lcapitulino@redhat.com> <87boj3dyxg.fsf@codemonkey.ws> <501111CD.9000701@redhat.com> <87fw8eofto.fsf@codemonkey.ws> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC 00/14]: add printf-like human msg to error_set() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Kevin Wolf , peter.maydell@linaro.org, qemu-devel@nongnu.org, armbru@redhat.com, pbonzini@redhat.com, afaerber@suse.de On Thu, 26 Jul 2012 07:41:07 -0500 Anthony Liguori wrote: > Kevin Wolf writes: > > > Am 26.07.2012 04:43, schrieb Anthony Liguori: > >> Luiz Capitulino writes: > >> > >>> Basically, this series changes a call like: > >>> > >>> error_set(errp, QERR_DEVICE_NOT_FOUND, device); > >>> > >>> to: > >>> > >>> error_set(errp, QERR_DEVICE_NOT_FOUND, > >>> "Device 'device=%s' not found", device); > >>> > >>> In the first call, QERR_DEVICE_NOT_FOUND is a string containing a json dict: > >>> > >>> "{ 'class': 'DeviceNotFound', 'data': { 'device': %s } }" > >> > >> This is the wrong direction. Looking through the patch, this makes the > >> code much more redundant overall. You have dozens of calls that are > >> duplicating the same error message. This is not progress. > > > > I believe this is mostly because it's a mechanical conversion. Once this > > is done, we can change error messages to better fit the individual > > cases. Correct. > > We don't gain anything by touching every user of error and the code gets > more verbose. We do gain the possibility to have better and different human messages for the same error class. That's impossible today. And creating a different error class just to have a different error message is just crazy (which is what we do today, btw). Now, if the problem you see is that we shouldn't touch current users but add a new function for new users to use (or change old users incrementally, when it matters), then we can discuss that. > If we want to modify an existing error for some good > reason, we can do so my changing error types. You mean, creating a new error class just to have a different human message? We have 70+ classes today, how many will we have in another year? > > >> We should just stick with a simple QERR_GENERIC and call it a day. > >> Let's not needlessly complicate existing code. > > > > Why even have error codes when everything should become QERR_GENERIC? Or > > am I misunderstanding? > > If we want to add an error code, we can do: > > error_set(QERR_GENERIC, "domain", "My free form text") > > And then yes, we can change this to: > > error_setf(errp, "domain", "My free form text") Would domain be the error classes we have today? If error_setf() ends result is similar to what this series does with error_set(), then that might be acceptable, although I fear we keep adding new ways to report errors.