From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Nc2Hq-0006MB-VD for qemu-devel@nongnu.org; Mon, 01 Feb 2010 14:50:19 -0500 Received: from [199.232.76.173] (port=54943 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Nc2Hq-0006L3-FR for qemu-devel@nongnu.org; Mon, 01 Feb 2010 14:50:18 -0500 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1Nc2Ho-0007nY-Bm for qemu-devel@nongnu.org; Mon, 01 Feb 2010 14:50:18 -0500 Received: from mx1.redhat.com ([209.132.183.28]:1025) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Nc2Hn-0007nQ-V4 for qemu-devel@nongnu.org; Mon, 01 Feb 2010 14:50:16 -0500 Received: from int-mx08.intmail.prod.int.phx2.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o11JoENZ027547 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 1 Feb 2010 14:50:15 -0500 Date: Mon, 1 Feb 2010 17:50:04 -0200 From: Luiz Capitulino Subject: Re: [Qemu-devel] [PATCH 0/8]: QMP feature negotiation support Message-ID: <20100201175004.5b1b5cc8@doriath> In-Reply-To: References: <1264686180-29845-1-git-send-email-lcapitulino@redhat.com> <20100201162234.0f144f3f@doriath> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org On Mon, 01 Feb 2010 20:37:41 +0100 Markus Armbruster wrote: > Luiz Capitulino writes: > > > On Mon, 01 Feb 2010 18:08:27 +0100 > > Markus Armbruster wrote: > > > >> Luiz Capitulino writes: > >> > >> > Feature negotiation allows clients to enable new QMP capabilities they > >> > support and thus allows QMP to envolve in a compatible way. > >> > > >> > A capability is a new QMP feature and/or protocol change which is not part of > >> > the core protocol as defined in the QMP spec. > >> > >> Well, it becomes part of the protocol then. But I understand what you > >> mean. > >> > >> > Feature negotiation is implemented by, among other changes, adding > >> > mode-oriented support to QMP, which comprehends the following: > >> > > >> > o Two modes: handshake and operational > >> > o All QMP Monitors start in handshake mode > >> > o In handshake mode only commands to query/enable/disable QMP capabilities are > >> > allowed (there are few exceptions) > >> > o Clients can switch to the operational mode at any time > >> > o In Operational mode most commands are allowed and QMP capabilities changes > >> > made in handshake mode take effect > >> > > >> > Please, note that we don't have any capability yet. So, the most visable > >> > change in this series is that now Clients must switch to operational mode to > >> > be able to issue 'regular' commands. > >> > > >> > Session example: > >> > > >> > """ > >> > {"QMP": {"capabilities": []}} > >> > > >> > { "execute": "query-qmp-mode" } > >> > {"return": {"mode": "handshake"}} > >> > > >> > { "execute": "stop" } > >> > {"error": {"class": "CommandNotFound", "desc": "The command stop has not been found", "data": {"name": "stop"}}} > >> > > >> > { "execute": "qmp_capability_enable", "arguments": { "name": "foobar" } } > >> > {"error": {"class": "InvalidParameter", "desc": "Invalid parameter name", "data": {"name": "name"}}} > >> > > >> > { "execute": "qmp_switch_mode", "arguments": { "mode": "operational" } } > >> > {"return": {}} > >> > > >> > { "execute": "query-qmp-mode" } > >> > {"return": {"mode": "operational"}} > >> > > >> > { "execute": "stop" } > >> > {"return": {}} > >> > > >> > """ > >> > >> I don't doubt your design does the job. I just think it's overly > >> general. I had something far more stupid in mind: > >> > >> client connects > >> server -> client: version & capability offer (one message) > >> again: > >> client -> server: capability selection (one message) > >> server -> client: either okay or error (one message) > >> if error goto again > >> connection is now ready for commands > >> > >> No modes. The distinct lack of generality is a design feature. > > > > I like the simplicity and if we were allowed to change later I'd > > do it. > > > > The question is if we will ever want features to be _configured_ > > before the protocol is operational. In this case we'd need to > > pass feature arguments through the capability selection command, > > which will get ugly and hard to use/understand. > > > > Mode oriented support doesn't have this limitation. Maybe we > > won't never really use it, but it's safer. > > Capability selection could be done as an object where the name/value > pairs are capability/argument. If you need multiple arguments for a > capability, make the capability's value an object. That's exactly what seems complicated to me, because besides performing two functions (enable/configure) some feature setup could require more commands to be done in a clear way. The async messages setup in the previous series was an example of this. As said we might never use this, but I wouldn't like to regret later.