Re: [Qemu-devel] [PATCH v4 09/17] monitor: implement 'qmp_query_commands' without qmp_cmds

[Markus Armbruster]

address@hidden writes:

> From: Marc-André Lureau <address@hidden>
>
> One step towards getting rid of the static qmp_cmds table.
>
> Signed-off-by: Marc-André Lureau <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
> ---
>  monitor.c | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 2050698..cddc737 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -957,21 +957,28 @@ static void hmp_info_help(Monitor *mon, const QDict 
> *qdict)
>      help_cmd(mon, "info");
>  }
>  
> -CommandInfoList *qmp_query_commands(Error **errp)
> +static void query_commands_cb(QmpCommand *cmd, void *opaque)
>  {
> -    CommandInfoList *info, *cmd_list = NULL;
> -    const mon_cmd_t *cmd;
> -
> -    for (cmd = qmp_cmds; cmd->name != NULL; cmd++) {
> -        info = g_malloc0(sizeof(*info));
> -        info->value = g_malloc0(sizeof(*info->value));
> -        info->value->name = g_strdup(cmd->name);
> +    CommandInfoList *info, **list = opaque;
>  
> -        info->next = cmd_list;
> -        cmd_list = info;
> +    if (!cmd->enabled) {
> +        return;
>      }
>  
> -    return cmd_list;
> +    info = g_malloc0(sizeof(*info));
> +    info->value = g_malloc0(sizeof(*info->value));
> +    info->value->name = g_strdup(cmd->name);
> +    info->next = *list;
> +    *list = info;
> +}
> +
> +CommandInfoList *qmp_query_commands(Error **errp)
> +{
> +    CommandInfoList *list = NULL;
> +
> +    qmp_for_each_command(query_commands_cb, &list);
> +
> +    return list;
>  }
>  
>  EventInfoList *qmp_query_events(Error **errp)

This patch flips query-commands from qmp-commands.hx / qmp_cmds[] to
QAPI / qmp_for_each_command().  PATCH 06 "monitor: unregister
conditional commands" is needed here to keep the compile-time
conditional commands out of query-commands.

The previous patch started the transition from qmp-commands.hx /
qmp_cmds[] to QAPI / qmp_find_command().  It'll be completed in patch
14.  PATCH 06 will be needed then so we keep rejecting the compile-time
conditional commands.

The compile-time conditional commands remain in query-qmp-schema despite
PATCH 06.  No change.  Okay.

PATCH 06's commit message could perhaps spell out these connections more
clearly.  Here's my attempt:

    monitor: unregister conditional commands

    QMP commands are currently defined in two places: the QAPI schema
    and qmp-commands.hx.

    qmp-commands.hx uses the C preprocessor to define a number of
    commands only conditionally.  For instance, query-spice is #ifdef
    CONFIG_SPICE.

    The QAPI schema does no such thing.

    The current QMP command dispatch and query-commands use a command
    table generated from qmp-commands.hx.  This table contains the
    conditionally defined commands only when their conditions are met.
    For instance, query-spice is accepted by command dispatch and shown
    by query-commands only when CONFIG_SPICE is defined.

    In contrast, query-qmp-schema uses the QAPI schema, and shows
    conditional commands whether they're defined or not.

    We're going to change QMP command dispatch and query-commands to use
    QAPI, so we can define QMP commands in just one place.  We need to
    do something to avoid changing dispatch and query-spice.

    Fortunately, a suitable mechanism already exists: we can make
    commands disabled.  Do that now.  The patches that change dispatch
    and query-commands will use it to avoid undue change.

Feel free to edit to taste.  If you're happy with it as is, I can
replace the message on commit.