From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NHzUe-0007Gx-FF for qemu-devel@nongnu.org; Tue, 08 Dec 2009 07:48:40 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NHzUZ-0007A0-A4 for qemu-devel@nongnu.org; Tue, 08 Dec 2009 07:48:39 -0500 Received: from [199.232.76.173] (port=36042 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NHzUZ-00079a-0S for qemu-devel@nongnu.org; Tue, 08 Dec 2009 07:48:35 -0500 Received: from mx1.redhat.com ([209.132.183.28]:32661) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NHzUY-0002Gy-H3 for qemu-devel@nongnu.org; Tue, 08 Dec 2009 07:48:34 -0500 From: Markus Armbruster References: <1260218236-22143-1-git-send-email-armbru@redhat.com> <1260218236-22143-19-git-send-email-armbru@redhat.com> <20091208101148.119bed20@doriath> Date: Tue, 08 Dec 2009 13:48:30 +0100 In-Reply-To: <20091208101148.119bed20@doriath> (Luiz Capitulino's message of "Tue, 8 Dec 2009 10:11:48 -0200") Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: [Qemu-devel] Re: [FOR 0.12 PATCH 18/18] QMP: add human-readable description to error response List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org Luiz Capitulino writes: > On Mon, 7 Dec 2009 21:37:16 +0100 > Markus Armbruster wrote: > >> -{ "error": { "class": json-string, "data": json-value }, "id": json-value } >> +{ "error": { "class": json-string, "data": json-value, "desc": json-string }, >> + "id": json-value } >> >> Where, >> >> - The "class" member contains the error class name (eg. "ServiceUnavailable") >> - The "data" member contains specific error data and is defined in a >> per-command basis, it will be an empty json-object if the error has no data >> +- The "desc" member is a human-readable error message. Clients should >> + not attempt to parse this message. >> - The "id" member contains the transaction identification associated with >> the command execution (if issued by the Client) > > As we've talked on irc, I don't agree with this change. > > Basically, adding 'desc' to the standard error message introduces all > the problems we've discussed about free-form English strings. I disagree. See below. > I feel that QError is becoming the worst of all proposals. I'm not exactly thrilled about it either, but it could clearly be worse in many ways, so it can't be the worst of all :) > I agree with you that it's not as easy as it should be to report errors, > but as we're targeting on Clients I was convinced that we could not have the > best API internally but offer a good interface for Clients. > > Now, having 'desc' as part of the standard protocol is like not having > the best API internally and offering a bad interface for Clients. > > Not to mention that those strings can't be modified when the protocol > becomes stable This is incorrect. We explicitly threaten client writers that these strings are not to be interpreted, in qmp-spec.txt: "Clients should not attempt to parse this message." For me, that implies that they can change at any time. Maybe we could use even stronger language there. > and we're probably talking about dozens if not a hundred > of strings. Ok, there isn't a reason to change them often, but it's > still one more thing to maintain. > > Having said that, I would agree to have 'desc' as part of debug > information. I have patches in my tree which adds CONFIG_DEBUG_QMP, > if one enables it information about the error location will also > be part of the error message. I would agree having 'desc' there > too. Debugging is one use for human-readable text in the error reply. But it is *not* the only use. We discussed this at length in October[*], and again in November[**]. Any new arguments? [*] Sub-thread starting at http://lists.gnu.org/archive/html/qemu-devel/2009-10/msg01245.html [**] Sub-thread starting at http://lists.gnu.org/archive/html/qemu-devel/2009-11/msg01071.html