From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56593) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YkYsI-0000D9-1i for qemu-devel@nongnu.org; Tue, 21 Apr 2015 10:10:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YkYsC-0004Lc-Tw for qemu-devel@nongnu.org; Tue, 21 Apr 2015 10:10:05 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45061) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YkYsC-0004LX-Oh for qemu-devel@nongnu.org; Tue, 21 Apr 2015 10:10:00 -0400 Date: Tue, 21 Apr 2015 10:09:10 -0400 From: Luiz Capitulino Message-ID: <20150421100910.51e40685@redhat.com> In-Reply-To: <1429097404-28027-1-git-send-email-pbonzini@redhat.com> References: <1429097404-28027-1-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] qmp-commands: fix incorrect uses of ":O" specifier List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: berto@igalia.com, qemu-devel@nongnu.org, armbru@redhat.com On Wed, 15 Apr 2015 13:30:04 +0200 Paolo Bonzini wrote: > As far as the QMP parser is concerned, neither the 'O' nor the 'q' format specifiers > put any constraint on the command. However, there are two differences: > > 1) from a documentation point of view 'O' says that this command takes > a dictionary. The dictionary will be converted to QemuOpts in the > handler to match the corresponding HMP command. > > 2) 'O' sets QMP_ACCEPT_UNKNOWNS, resulting in the command accepting invalid > extra arguments. For example the following is accepted: > > { "execute": "send-key", > "arguments": { "keys": [ { "type": "qcode", "data": "ctrl" }, > { "type": "qcode", "data": "alt" }, > { "type": "qcode", "data": "delete" } ], "foo": "bar" } } > > Neither send-key nor migrate-set-capabilities take a QemuOpts-like > dictionary; they take an array of dictionaries. And neither command > really wants to have extra unknown arguments. Thus, the right > specifier to use in this case is 'q'; with this patch the above > command fails with > > {"error": {"class": "GenericError", "desc": "Invalid parameter 'foo'"}} > > as intended. > > Reported-by: Alberto Garcia > Signed-off-by: Paolo Bonzini Applied to the qmp branch, thanks. > --- > qmp-commands.hx | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 3a42ad0..09f48ba 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -332,7 +332,7 @@ EQMP > > { > .name = "send-key", > - .args_type = "keys:O,hold-time:i?", > + .args_type = "keys:q,hold-time:i?", > .mhandler.cmd_new = qmp_marshal_input_send_key, > }, > > @@ -3288,7 +3288,7 @@ EQMP > > { > .name = "migrate-set-capabilities", > - .args_type = "capabilities:O", > + .args_type = "capabilities:q", > .params = "capability:s,state:b", > .mhandler.cmd_new = qmp_marshal_input_migrate_set_capabilities, > },