Re: [Qemu-devel] [PATCH 1/2] monitor: add sub_args_type for second level parameters

[Luiz Capitulino <lcapitulino@redhat.com>]

On Thu, 21 Oct 2010 08:37:19 +0200
Alon Levy <alevy@redhat.com> wrote:

> ---
>  hmp-commands.hx |    1 +
>  monitor.c       |   13 ++++++++++++-
>  2 files changed, 13 insertions(+), 1 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 3014b17..289fbcb 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1158,6 +1158,7 @@ ETEXI
>          .args_type  = "item:s?",
>          .params     = "[subcommand]",
>          .help       = "show various information about the system state",
> +        .sub_args_type = do_info_sub_args_type,
>          .mhandler.cmd = do_info,
>      },
>  
> diff --git a/monitor.c b/monitor.c
> index 260cc02..7d1d3b1 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -107,6 +107,7 @@ typedef struct mon_cmd_t {
>      const char *params;
>      const char *help;
>      void (*user_print)(Monitor *mon, const QObject *data);
> +    const char *(*sub_args_type)(const QDict *qdict);
>      union {
>          void (*info)(Monitor *mon);
>          void (*info_new)(Monitor *mon, QObject **ret_data);
> @@ -3500,6 +3501,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>      char cmdname[256];
>      char buf[1024];
>      char *key;
> +    int did_sub_args_type = 0;
>  
>  #ifdef DEBUG
>      monitor_printf(mon, "command='%s'\n", cmdline);
> @@ -3520,8 +3522,17 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>      typestr = cmd->args_type;
>      for(;;) {
>          typestr = key_get_info(typestr, &key);
> -        if (!typestr)
> +        /* Allow for two level parameters definition. Call sub_args_type only
> +         * after finished parsing existing args */
> +        if (!typestr && cmd->sub_args_type != NULL && !did_sub_args_type) {
> +            if ((typestr = cmd->sub_args_type(qdict)) != NULL) {
> +                typestr = key_get_info(typestr, &key);
> +            }
> +            did_sub_args_type = 1;
> +        }
> +        if (!typestr) {
>              break;
> +        }

Unfortunately, the args_type thing is a bad interface and very hacked
already. I think we're adding more to the pile.

What I'm going to do for QMP is to convert all info_new() into cmd_new(),
this way query- commands can have their arguments just like any command.

I'm wondering if we can do the same for HMP, that's turn all info() into
cmd(). But I'm not sure if this can get tricky, as info commands have
special handling in a few different places.

Care to check?

>          c = *typestr;
>          typestr++;
>          switch(c) {