[Qemu-devel] Re: [PATCH v3 13/17] monitor: Allow to exclude commands from QMP

[Luiz Capitulino <lcapitulino@redhat.com>]

On Sun, 23 May 2010 12:59:26 +0200
Jan Kiszka <jan.kiszka@web.de> wrote:

> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Ported commands that are marked 'user_only' will not be considered for
> QMP monitor sessions. This allows to implement new commands that do not
> (yet) provide a sufficiently stable interface for QMP use (e.g.
> device_show).

 This is fine for me, but two things I've been wondering:

 1. Isn't a 'flags' struct member better? So that we can do (in the
    qemu-monitor.hx entry):

        .flags = MONITOR_USER_ONLY | MONITOR_HANDLER_ASYNC,

    I'm not suggesting this is an async handler, just exemplifying multiple
    flags.

  2. Getting QMP handlers right in the first time might be difficult, so
     we could have a way to mark them unstable. Maybe a different namespace
     which is only enabled at configure time with:

         --enable-qmp-unstable-commands

     If this were possible, we could have device_show and any command we
     aren't sure is QMP-ready working in QMP this way.

> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  monitor.c |   13 ++++++++++---
>  1 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index 6766e49..5768c6e 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -114,6 +114,7 @@ typedef struct mon_cmd_t {
>                            MonitorCompletion *cb, void *opaque);
>      } mhandler;
>      int async;
> +    bool user_only;
>  } mon_cmd_t;
>  
>  /* file descriptors passed via SCM_RIGHTS */
> @@ -635,6 +636,11 @@ static int do_info(Monitor *mon, const QDict *qdict, QObject **ret_data)
>          goto help;
>      }
>  
> +    if (monitor_ctrl_mode(mon) && cmd->user_only) {
> +        qerror_report(QERR_COMMAND_NOT_FOUND, item);
> +        return -1;
> +    }
> +
>      if (monitor_handler_is_async(cmd)) {
>          if (monitor_ctrl_mode(mon)) {
>              qmp_async_info_handler(mon, cmd);
> @@ -732,13 +738,14 @@ static void do_info_commands(Monitor *mon, QObject **ret_data)
>      cmd_list = qlist_new();
>  
>      for (cmd = mon_cmds; cmd->name != NULL; cmd++) {
> -        if (monitor_handler_ported(cmd) && !compare_cmd(cmd->name, "info")) {
> +        if (monitor_handler_ported(cmd) && !cmd->user_only &&
> +            !compare_cmd(cmd->name, "info")) {
>              qlist_append_obj(cmd_list, get_cmd_dict(cmd->name));
>          }
>      }
>  
>      for (cmd = info_cmds; cmd->name != NULL; cmd++) {
> -        if (monitor_handler_ported(cmd)) {
> +        if (monitor_handler_ported(cmd) && !cmd->user_only) {
>              char buf[128];
>              snprintf(buf, sizeof(buf), "query-%s", cmd->name);
>              qlist_append_obj(cmd_list, get_cmd_dict(buf));
> @@ -4416,7 +4423,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
>                        qobject_from_jsonf("{ 'item': %s }", info_item));
>      } else {
>          cmd = monitor_find_command(cmd_name);
> -        if (!cmd || !monitor_handler_ported(cmd)) {
> +        if (!cmd || !monitor_handler_ported(cmd) || cmd->user_only) {
>              qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
>              goto err_input;
>          }