On Thu, Jan 31, 2019 at 11:26:37PM +0300, Julia Suvorova wrote: > The whitelist option allows to run a reduced monitor with a subset of > QMP commands. This allows the monitor to run in secure mode, which is > convenient for sending commands via the WebSocket monitor using the > web UI. This is planned to be done on micro:bit board. > > The list of allowed commands should be written to a file, one per line. > The command line will look like this: > -mon chardev_name,mode=control,whitelist=path_to_file > > Signed-off-by: Julia Suvorova <jus...@mail.ru> > ---
Please include a test case. tests/qmp-cmd-test.c looks like a starting
point.
Interesting cases:
1. QMP negotiation still works.
2. A whitelisted command succeeds.
3. A forbidden command fails with the expected error.
> +static void monitor_qmp_cleanup_commands(Monitor *mon)
> +{
> + QmpCommand *cmd, *next_cmd;
> +
> + if (!monitor_is_qmp(mon) ||
> + mon->qmp.commands == &qmp_cap_negotiation_commands) {
> + return;
> + }
Checking side-effects can be avoided with a bool flag:
if (!mon->qmp_commands_needs_free) {
return;
}
Any place that allocates qmp.commands must set this flag and then we
don't need to worry whether we're checking the right side-effects in the
cleanup function.
I think this makes the code easier to read but it's just a suggestion.
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 521511ec13..e5d1b7dfb5 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3195,12 +3195,16 @@ Like -qmp but uses pretty JSON formatting.
> ETEXI
>
> DEF("mon", HAS_ARG, QEMU_OPTION_mon, \
> - "-mon [chardev=]name[,mode=readline|control][,pretty[=on|off]]\n",
> QEMU_ARCH_ALL)
> + "-mon [chardev=]name[,mode=readline|control][,pretty[=on|off]]" \
> + "[,whitelist=file]\n", QEMU_ARCH_ALL)
> STEXI
> -@item -mon [chardev=]name[,mode=readline|control][,pretty[=on|off]]
> +@item -mon
> [chardev=]name[,mode=readline|control][,pretty[=on|off]][,whitelist=@var{file}]
Your change reminded me that "[chardev=]name" should be
"[chardev=]@var{name}" since 'name' isn't a string literal but a
variable.
This is a pre-existing issue but please add a patch to this series to
fix it.
> @findex -mon
> Setup monitor on chardev @var{name}. @code{pretty} turns on JSON pretty
> printing
> easing human reading and debugging.
> +The @code{whitelist} option disables all commands except those specified in
> +@var{file}. The file must contain one command name per line. This option is
> only
> +avaliable in 'control' mode.
s/avaliable/available/
signature.asc
Description: PGP signature
