From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59816) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YiS7z-0007Wi-UU for qemu-devel@nongnu.org; Wed, 15 Apr 2015 14:33:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YiS7u-0002fL-3e for qemu-devel@nongnu.org; Wed, 15 Apr 2015 14:33:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57962) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YiS7t-0002fE-Qd for qemu-devel@nongnu.org; Wed, 15 Apr 2015 14:33:30 -0400 Message-ID: <552EAEF8.4030102@redhat.com> Date: Wed, 15 Apr 2015 14:33:28 -0400 From: John Snow MIME-Version: 1.0 References: <1429111163-27870-1-git-send-email-eblake@redhat.com> <552EAA42.9020400@redhat.com> <552EAE9D.90703@redhat.com> In-Reply-To: <552EAE9D.90703@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] qmp: Give saner messages related to qmp_capabilities misuse List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: erik.rull@rdsoftware.de, paulo.vital@profitbricks.com, lcapitulino@redhat.com On 04/15/2015 02:31 PM, Eric Blake wrote: > On 04/15/2015 12:13 PM, John Snow wrote: >> >> >> On 04/15/2015 11:19 AM, Eric Blake wrote: >>> Pretending that QMP doesn't understand a command merely because >>> we are not in the right mode doesn't help first-time users figure >>> out what to do to correct things. Although the documentation for >>> QMP calls out capabilities negotiation, we should also make it >>> clear in our error messages what we were expecting. With this >>> patch, I now get the following transcript: >>> >>> $ ./x86_64-softmmu/qemu-system-x86_64 -qmp stdio -nodefaults >>> {"QMP": {"version": {"qemu": {"micro": 93, "minor": 2, "major": 2}, >>> "package": ""}, "capabilities": []}} >>> {"execute":"huh"} >>> {"error": {"class": "CommandNotFound", "desc": "The command huh has >>> not been found"}} >>> {"execute":"quit"} >>> {"error": {"class": "CommandNotFound", "desc": "Expecting capabilities >>> negotiation with 'qmp_capabilities' before command 'quit'"}} >> >> Any particular reason why we should keep the "CommandNotFound" error >> class here? Backwards compat? Inertia? >> >>> {"execute":"qmp_capabilities"} >>> {"return": {}} >>> {"execute":"qmp_capabilities"} >>> {"error": {"class": "CommandNotFound", "desc": "Capabilities >>> negotiation is already complete, command 'qmp_capabilities' ignored"}} >> >> Same here. > > Backwards compat. I can't prove that anyone else was relying on specific > classes (in particular, although it is unlikely that anyone was issuing > qmp_capabilities more than once, or cares what error class was returned, > it IS a useful test for probing if the connection is in capability > negotiation mode when reconnecting to a monitor after a libvirtd > restart). It's better to be conservative and avoid changing the error > class (which must be reliable to machine readers) and only impact the > error message (which is human readable and is documented to not be > machine parseable, so we can change that at will). > Blast ye, Backwards Compat... Reviewed-by: John Snow