On Fri, May 04, 2012 at 05:20:17PM -0300, Luiz Capitulino wrote:
> Options allow for changes in commands behavior. This commit introduces
> the QCO_NO_SUCCESS_RESP option, which causes a command to not emit a
> success response.
>
> This is needed by commands such as qemu-ga's guest-shutdown, which
> may not be able to complete before the VM vanishes. In this case, it's
> useful and simpler not to bother sending a success response.
>
> Signed-off-by: Luiz Capitulino <lcapitul...@redhat.com>
> ---
> qapi/qmp-core.h | 10 +++++++++-
> qapi/qmp-dispatch.c | 8 ++++++--
> qapi/qmp-registry.c | 4 +++-
> scripts/qapi-commands.py | 14 ++++++++++++--
> 4 files changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/qapi/qmp-core.h b/qapi/qmp-core.h
> index 431ddbb..b0f64ba 100644
> --- a/qapi/qmp-core.h
> +++ b/qapi/qmp-core.h
> @@ -25,16 +25,24 @@ typedef enum QmpCommandType
> QCT_NORMAL,
> } QmpCommandType;
>
> +typedef enum QmpCommandOptions
> +{
> + QCO_NO_OPTIONS = 0x0,
> + QCO_NO_SUCCESS_RESP = 0x1,
> +} QmpCommandOptions;
> +
> typedef struct QmpCommand
> {
> const char *name;
> QmpCommandType type;
> QmpCommandFunc *fn;
> + QmpCommandOptions options;
> QTAILQ_ENTRY(QmpCommand) node;
> bool enabled;
> } QmpCommand;
>
> -void qmp_register_command(const char *name, QmpCommandFunc *fn);
> +void qmp_register_command(const char *name, QmpCommandFunc *fn,
> + QmpCommandOptions options);
> QmpCommand *qmp_find_command(const char *name);
> QObject *qmp_dispatch(QObject *request);
> void qmp_disable_command(const char *name);
> diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
> index 43f640a..122c1a2 100644
> --- a/qapi/qmp-dispatch.c
> +++ b/qapi/qmp-dispatch.c
> @@ -94,8 +94,12 @@ static QObject *do_qmp_dispatch(QObject *request, Error
> **errp)
> switch (cmd->type) {
> case QCT_NORMAL:
> cmd->fn(args, &ret, errp);
> - if (!error_is_set(errp) && ret == NULL) {
> - ret = QOBJECT(qdict_new());
> + if (!error_is_set(errp)) {
> + if (cmd->options & QCO_NO_SUCCESS_RESP) {
> + g_assert(!ret);
> + } else if (!ret) {
> + ret = QOBJECT(qdict_new());
> + }
> }
> break;
> }
> diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
> index 43d5cde..5414613 100644
> --- a/qapi/qmp-registry.c
> +++ b/qapi/qmp-registry.c
> @@ -17,7 +17,8 @@
> static QTAILQ_HEAD(QmpCommandList, QmpCommand) qmp_commands =
> QTAILQ_HEAD_INITIALIZER(qmp_commands);
>
> -void qmp_register_command(const char *name, QmpCommandFunc *fn)
> +void qmp_register_command(const char *name, QmpCommandFunc *fn,
> + QmpCommandOptions options)
> {
> QmpCommand *cmd = g_malloc0(sizeof(*cmd));
>
> @@ -25,6 +26,7 @@ void qmp_register_command(const char *name, QmpCommandFunc
> *fn)
> cmd->type = QCT_NORMAL;
> cmd->fn = fn;
> cmd->enabled = true;
> + cmd->options = options;
> QTAILQ_INSERT_TAIL(&qmp_commands, cmd, node);
> }
>
> diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> index 0b4f0a0..e746333 100644
> --- a/scripts/qapi-commands.py
> +++ b/scripts/qapi-commands.py
> @@ -291,14 +291,24 @@ out:
>
> return ret
>
> +def option_is_enabled(opt, val, cmd):
> + if opt in cmd and cmd[opt] == val:
> + return True
> + return False
> +
> def gen_registry(commands):
> registry=""
> push_indent()
> for cmd in commands:
> + options = 'QCO_NO_OPTIONS'
> + if option_is_enabled('success-response', 'no', cmd):
> + options = 'QCO_NO_SUCCESS_RESP'
> +
Hate to hold things up for a nit, but the naming of option_is_enabled()
is just plain wrong here. option_is_enabled() is returning true for a
case where we're actually checking for an option being disabled. I'm
guessing it looks this way because we're trying to determine if the
internal QCO_NO_SUCCESS_RESP option is enabled, but option_is_enabled()
only has knowledge of the QAPI directive so I think that's backwards.
option_value_matches() would indicate the purpose better, or
option_is_disabled() and then moving the "no"/"false"/"disabled"
matching into it.
Patch looks good otherwise.
Reviewed-by: Michael Roth <mdr...@linux.vnet.ibm.com>
> registry += mcgen('''
> -qmp_register_command("%(name)s", qmp_marshal_input_%(c_name)s);
> +qmp_register_command("%(name)s", qmp_marshal_input_%(c_name)s, %(opts)s);
> ''',
> - name=cmd['command'], c_name=c_fun(cmd['command']))
> + name=cmd['command'], c_name=c_fun(cmd['command']),
> + opts=options)
> pop_indent()
> ret = mcgen('''
> static void qmp_init_marshal(void)
> --
> 1.7.9.2.384.g4a92a
>
