From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35808) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SuNOD-0003Hj-8K for qemu-devel@nongnu.org; Thu, 26 Jul 2012 08:42:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SuNOB-0003bA-Tg for qemu-devel@nongnu.org; Thu, 26 Jul 2012 08:42:01 -0400 Received: from e32.co.us.ibm.com ([32.97.110.150]:58325) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SuNOB-0003ax-Nv for qemu-devel@nongnu.org; Thu, 26 Jul 2012 08:41:59 -0400 Received: from /spool/local by e32.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 26 Jul 2012 06:41:58 -0600 Received: from d03relay01.boulder.ibm.com (d03relay01.boulder.ibm.com [9.17.195.226]) by d03dlp02.boulder.ibm.com (Postfix) with ESMTP id B2A1F3E40043 for ; Thu, 26 Jul 2012 12:41:11 +0000 (WET) Received: from d03av06.boulder.ibm.com (d03av06.boulder.ibm.com [9.17.195.245]) by d03relay01.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q6QCfA33137296 for ; Thu, 26 Jul 2012 06:41:10 -0600 Received: from d03av06.boulder.ibm.com (loopback [127.0.0.1]) by d03av06.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q6QCgLaF030843 for ; Thu, 26 Jul 2012 06:42:21 -0600 From: Anthony Liguori In-Reply-To: <501111CD.9000701@redhat.com> References: <1343249431-9245-1-git-send-email-lcapitulino@redhat.com> <87boj3dyxg.fsf@codemonkey.ws> <501111CD.9000701@redhat.com> Date: Thu, 26 Jul 2012 07:41:07 -0500 Message-ID: <87fw8eofto.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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: Kevin Wolf Cc: peter.maydell@linaro.org, qemu-devel@nongnu.org, armbru@redhat.com, pbonzini@redhat.com, Luiz Capitulino , afaerber@suse.de 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. We don't gain anything by touching every user of error and the code gets more verbose. If we want to modify an existing error for some good reason, we can do so my changing error types. >> 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") Or pick your favorite short name. Regards, Anthony Liguori > > Kevin