From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NYIKV-0008KY-EL for qemu-devel@nongnu.org; Fri, 22 Jan 2010 07:09:35 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NYIKR-0008KC-Sf for qemu-devel@nongnu.org; Fri, 22 Jan 2010 07:09:35 -0500 Received: from [199.232.76.173] (port=44029 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NYIKR-0008K9-NB for qemu-devel@nongnu.org; Fri, 22 Jan 2010 07:09:31 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55062) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NYIKR-0007NS-0M for qemu-devel@nongnu.org; Fri, 22 Jan 2010 07:09:31 -0500 Date: Fri, 22 Jan 2010 10:09:20 -0200 From: Luiz Capitulino Subject: Re: [Qemu-devel] [RFC 00/11]: QMP feature negotiation support Message-ID: <20100122100920.7127b410@doriath> In-Reply-To: References: <1264108180-3666-1-git-send-email-lcapitulino@redhat.com> 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: aliguori@us.ibm.com, qemu-devel@nongnu.org, avi@redhat.com On Fri, 22 Jan 2010 11:21:22 +0100 Markus Armbruster wrote: > A few quick questions before I dive into the patches... > > Luiz Capitulino writes: > > > Feature negotiation allows clients to enable QMP capabilities they are > > interested in using. This allows QMP to envolve without breaking old clients. > > > > 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. > > > > Feature negotiation is implemented by defining a set of rules and adding > > mode-oriented support. > > > > The set of rules are: > > > > o All QMP capabilities are disabled by default > > o All QMP capabilities must be advertised in the capabilities array > > o Commands to enable/disable capabilities must be provided > > > > NOTE: Asynchronous messages are now considered a capability. > > > > Mode-oriented support adds the following to QMP: > > > > o Two modes: handshake and operational > > o By default all QMP Monitors start in handshake mode > > "By default"? Is there a way to start a QMP monitor in another mode? No, you think it's worth or is it just about the English? > > o In handshake mode only commands to query/enable/disable QMP capabilities are > > allowed (there are few exceptions) > > Note to self: check out the exception, and why we might want them. The following handlers are handshake-only: - qmp_switch_mode - async_msg_enable - async_msg_disable The following handlers are allowed to run on both modes: - query-version - query-qmp-mode - query-commands Also, all the self-description commands (query-async-msg, query-errors etc) would be allowed on both modes. So, the only handler which is not completely related to feature negotiation is query-version. This is only a guess, but I think it might be worth to let clients learn the QEMU version they are talking to before setting protocol features. > > o Clients can switch to the operational mode at any time > > Can they switch back? I hope not. No, they can't. The only transition allowed is handshake -> operational. > > o In Operational mode most commands are allowed and QMP capabilities changes > > made in handshake mode take effect > > > > Also note that each QMP Monitor has its own mode state and set of capabilities, > > this means that if QEMU is started with N QMP Monitors protocol setup done in > > one of them doesn't affect the others. > > > > Session example: > > > > """ > > {"QMP": {"capabilities": ["async messages"]}} > > > > { "execute": "query-qmp-mode" } > > {"return": {"mode": "handshake"}} > > Why would clients want to query the mode? It's more useful for testing purposes. > > { "execute": "change", "arguments": { "device": "vnc", "target": "password", "arg": "1234" } } > > {"error": {"class": "QMPInvalidModeCommad", "desc": "The issued command is invalid in this mode", "data": {}}} > > I'd treat this like a completely unknown command. Really? This would simplify things a bit. > > { "execute": "async_msg_enable", "arguments": { "name": "STOP" } } > > {"return": {}} > > > > { "execute": "qmp_switch_mode", "arguments": { "mode": "operational" } } > > {"return": {}} > > Do we envisage mode transitions other than handshake -> operational? Today we don't, but this is about forward compatibility support right? :) So, IMO it makes sense to have a more general command instead of qmp_switch_operational.