Re: [Qemu-devel] [PATCH v3 09/21] qapi: add QMP dispatch functions

[Luiz Capitulino]

On Mon, 13 Jun 2011 21:31:14 -0500
Michael Roth <address@hidden> wrote:

> Given an object recieved via QMP, this code uses the dispatch table
> provided by qmp_registry.c to call the corresponding marshalling/dispatch
> function and format return values/errors for delivery to the QMP.
> Currently only synchronous QMP functions are supported, but this will
> also be used for async QMP functions and QMP guest proxy dispatch as
> well.
> 
> Signed-off-by: Michael Roth <address@hidden>
> ---
>  Makefile.objs       |    2 +-
>  qapi/qmp-core.h     |    1 +
>  qapi/qmp-dispatch.c |   76 
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 78 insertions(+), 1 deletions(-)
>  create mode 100644 qapi/qmp-dispatch.c
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index 092f314..7e82587 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -368,7 +368,7 @@ libcacard-y = cac.o event.o vcard.o vreader.o 
> vcard_emul_nss.o vcard_emul_type.o
>  # qapi
>  
>  qapi-nested-y = qapi-visit-core.o qmp-input-visitor.o qmp-output-visitor.o 
> qapi-dealloc-visitor.o
> -qapi-nested-y += qmp-registry.o
> +qapi-nested-y += qmp-registry.o qmp-dispatch.o
>  qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y))
>  
>  vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
> diff --git a/qapi/qmp-core.h b/qapi/qmp-core.h
> index 99e929f..f1c26e4 100644
> --- a/qapi/qmp-core.h
> +++ b/qapi/qmp-core.h
> @@ -35,6 +35,7 @@ typedef struct QmpCommand
>  
>  void qmp_register_command(const char *name, QmpCommandFunc *fn);
>  QmpCommand *qmp_find_command(const char *name);
> +QObject *qmp_dispatch(QObject *request);
>  
>  #endif
>  
> diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
> new file mode 100644
> index 0000000..5bf41ea
> --- /dev/null
> +++ b/qapi/qmp-dispatch.c
> @@ -0,0 +1,76 @@
> +#include "qemu-objects.h"
> +#include "qapi/qmp-core.h"
> +#include "json-parser.h"
> +#include "error.h"
> +#include "error_int.h"
> +#include "qerror.h"
> +
> +static QObject *qmp_dispatch_err(QObject *request, Error **errp)

Please, call this do_qmp_dispatch() instead. Every time I turn to this
function I tend to think this is only called when dispatch fails :)

> +{
> +    const char *command;
> +    QDict *args, *dict;
> +    QmpCommand *cmd;
> +    QObject *ret = NULL;
> +
> +    if (qobject_type(request) != QTYPE_QDICT) {
> +        error_set(errp, QERR_JSON_PARSE_ERROR, "request is not a 
> dictionary");
> +        goto out;
> +    }
> +
> +    dict = qobject_to_qdict(request);
> +    if (!qdict_haskey(dict, "execute")) {
> +        error_set(errp, QERR_JSON_PARSE_ERROR, "no execute key");
> +        goto out;
> +    }
> +
> +    command = qdict_get_str(dict, "execute");
> +    cmd = qmp_find_command(command);
> +    if (cmd == NULL) {
> +        error_set(errp, QERR_COMMAND_NOT_FOUND, command);
> +        goto out;
> +    }
> +
> +    if (!qdict_haskey(dict, "arguments")) {
> +        args = qdict_new();
> +    } else {
> +        args = qdict_get_qdict(dict, "arguments");
> +        QINCREF(args);
> +    }

This function doesn't seem to handle extra keys in the command dict, like:

 { "execute": "query-block", "foo": "bar" }

You probably want to use qmp_check_input_obj() here.

> +
> +    switch (cmd->type) {
> +    case QCT_NORMAL:
> +        cmd->fn(args, &ret, errp);
> +        if (!error_is_set(errp) && ret == NULL) {
> +            ret = QOBJECT(qdict_new());
> +        }
> +        break;
> +    }
> +
> +    QDECREF(args);
> +
> +out:
> +
> +    return ret;
> +}
> +
> +QObject *qmp_dispatch(QObject *request)
> +{
> +    Error *err = NULL;
> +    QObject *ret;
> +    QDict *rsp;
> +
> +    ret = qmp_dispatch_err(request, &err);
> +
> +    rsp = qdict_new();
> +    if (err) {
> +        qdict_put_obj(rsp, "error", error_get_qobject(err));
> +        error_free(err);
> +    } else if (ret) {
> +        qdict_put_obj(rsp, "return", ret);
> +    } else {
> +        QDECREF(rsp);
> +        return NULL;

When does the 'else' condition happens?

> +    }
> +
> +    return QOBJECT(rsp);
> +}