* [Qemu-devel] [PATCH] qmp-commands: fix incorrect uses of ":O" specifier
@ 2015-04-15 11:30 Paolo Bonzini
2015-04-15 12:18 ` Alberto Garcia
2015-04-21 14:09 ` Luiz Capitulino
0 siblings, 2 replies; 4+ messages in thread
From: Paolo Bonzini @ 2015-04-15 11:30 UTC (permalink / raw)
To: qemu-devel; +Cc: berto, armbru, Luiz Capitulino
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 <berto@igalia.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
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,
},
--
2.3.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] qmp-commands: fix incorrect uses of ":O" specifier
2015-04-15 11:30 [Qemu-devel] [PATCH] qmp-commands: fix incorrect uses of ":O" specifier Paolo Bonzini
@ 2015-04-15 12:18 ` Alberto Garcia
2015-04-15 12:21 ` Paolo Bonzini
2015-04-21 14:09 ` Luiz Capitulino
1 sibling, 1 reply; 4+ messages in thread
From: Alberto Garcia @ 2015-04-15 12:18 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: armbru, Luiz Capitulino
On Wed 15 Apr 2015 01:30:04 PM CEST, 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.
Is that documentation the comments in monitor.c? It could also be a good
moment to document there 'q' as well.
Otherwise the patch looks good to me,
Reviewed-by: Alberto Garcia <berto@igalia.com>
Berto
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] qmp-commands: fix incorrect uses of ":O" specifier
2015-04-15 12:18 ` Alberto Garcia
@ 2015-04-15 12:21 ` Paolo Bonzini
0 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2015-04-15 12:21 UTC (permalink / raw)
To: Alberto Garcia, qemu-devel; +Cc: armbru, Luiz Capitulino
On 15/04/2015 14:18, Alberto Garcia wrote:
> On Wed 15 Apr 2015 01:30:04 PM CEST, 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.
>
> Is that documentation the comments in monitor.c? It could also be a good
> moment to document there 'q' as well.
No, I mean what you can learn by parsing the .args_type line.
'foo:O' means there will typically be no argument named foo, and the
dictionary will be treated as a series of key/value pairs.
'foo:q' means there will be an argument named foo, which is a JSON
object of some kind.
Paolo
> Otherwise the patch looks good to me,
>
> Reviewed-by: Alberto Garcia <berto@igalia.com>
>
> Berto
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] qmp-commands: fix incorrect uses of ":O" specifier
2015-04-15 11:30 [Qemu-devel] [PATCH] qmp-commands: fix incorrect uses of ":O" specifier Paolo Bonzini
2015-04-15 12:18 ` Alberto Garcia
@ 2015-04-21 14:09 ` Luiz Capitulino
1 sibling, 0 replies; 4+ messages in thread
From: Luiz Capitulino @ 2015-04-21 14:09 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: berto, qemu-devel, armbru
On Wed, 15 Apr 2015 13:30:04 +0200
Paolo Bonzini <pbonzini@redhat.com> 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 <berto@igalia.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
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,
> },
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-04-21 14:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-15 11:30 [Qemu-devel] [PATCH] qmp-commands: fix incorrect uses of ":O" specifier Paolo Bonzini
2015-04-15 12:18 ` Alberto Garcia
2015-04-15 12:21 ` Paolo Bonzini
2015-04-21 14:09 ` Luiz Capitulino
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.