Re: [Qemu-devel] [PATCH 1/8] QMP: Initial mode-oriented support

[Markus Armbruster <armbru@redhat.com>]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> In order to have feature negotiation in QMP, the Monitor
> has to be modified to support different modes of operation.
>
> We need two modes:
>
> o Handshake: where features are negotiated, only commands
> which deal with protocol configuration are allowed
>
> o Operational: regular Monitor operations, all handlers
> (except the protocol configuration ones) are allowed
>
> This commit does the following:
>
> 1. Adds the QMPMode data type to MonitorControl and sets
> QMODE_HANDSHAKE as the default one
>
> 2. Grant permission to 'query-version' and 'query-commands'
> to run on handshake mode
>
> Note, however, that these changes are not visable yet and
> thus QMP's behavior is still the same.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  monitor.c |   14 ++++++++++++++
>  1 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index fb7c572..baae9d0 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -82,11 +82,16 @@ struct MonitorCompletionData {
>      void (*user_print)(Monitor *mon, const QObject *data);
>  };
>  
> +/* Handler flags */
> +#define HANDLER_HANDSHAKE      0x01 /* allowed to run on handshake mode */
> +#define HANDLER_HANDSHAKE_ONLY 0x02 /* can ONLY run on handshake mode */
> +

A command's flags encode the set of modes in which this command may be
used.  The obvious way to encode a set of modes is to have an enable bit
for each mode.

Your choice of flags has the advantage that the value zero, which
doesn't need an initializer, means "operational mode only".

Fine with me.

>  typedef struct mon_cmd_t {
>      const char *name;
>      const char *args_type;
>      const char *params;
>      const char *help;
> +    unsigned int flags;
>      void (*user_print)(Monitor *mon, const QObject *data);
>      union {
>          void (*info)(Monitor *mon);
> @@ -108,8 +113,14 @@ struct mon_fd_t {
>      QLIST_ENTRY(mon_fd_t) next;
>  };
>  
> +typedef enum QMPMode {
> +    QMODE_OPERATIONAL,
> +    QMODE_HANDSHAKE,
> +} QMPMode;
> +
>  typedef struct MonitorControl {
>      QObject *id;
> +    QMPMode mode;
>      int print_enabled;
>      JSONMessageParser parser;
>  } MonitorControl;
> @@ -2471,6 +2482,7 @@ static const mon_cmd_t info_cmds[] = {
>          .args_type  = "",
>          .params     = "",
>          .help       = "show the version of QEMU",
> +        .flags      = HANDLER_HANDSHAKE,
>          .user_print = do_info_version_print,
>          .mhandler.info_new = do_info_version,
>      },
> @@ -2479,6 +2491,7 @@ static const mon_cmd_t info_cmds[] = {
>          .args_type  = "",
>          .params     = "",
>          .help       = "list QMP available commands",
> +        .flags      = HANDLER_HANDSHAKE,
>          .user_print = monitor_user_noop,
>          .mhandler.info_new = do_info_commands,
>      },
> @@ -4293,6 +4306,7 @@ static void monitor_control_event(void *opaque, int event)
>          QObject *data;
>          Monitor *mon = opaque;
>  
> +        mon->mc->mode = QMODE_HANDSHAKE;
>          json_message_parser_init(&mon->mc->parser, handle_qmp_command);
>  
>          data = qobject_from_jsonf("{ 'QMP': { 'capabilities': [] } }");