Re: [Qemu-devel] [PATCH v4 05/28] qapi: Support multiple command registries per program

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2593 bytes --]

On 03/03/2017 06:32 AM, Markus Armbruster wrote:
> The command registry encapsulates a single command list.  Give the
> functions using it a parameter instead.  Define suitable command lists
> in monitor, guest agent and test-qmp-commands.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/qapi/qmp/dispatch.h | 22 ++++++++++++++--------
>  monitor.c                   | 31 +++++++++++++++++--------------
>  qapi/qmp-dispatch.c         | 17 +++++++++++++----
>  qapi/qmp-registry.c         | 37 ++++++++++++++++++-------------------
>  qga/commands.c              |  2 +-
>  qga/guest-agent-core.h      |  2 ++
>  qga/main.c                  | 19 ++++++++++---------
>  scripts/qapi-commands.py    | 16 ++++++++++------
>  tests/test-qmp-commands.c   | 12 +++++++-----
>  9 files changed, 92 insertions(+), 66 deletions(-)
> 

> +++ b/qapi/qmp-dispatch.c
> @@ -67,7 +67,11 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp)
>      return dict;
>  }
>  
> -static QObject *do_qmp_dispatch(QObject *request, Error **errp)
> +volatile QmpCommand *save_cmd;

A comment why volatile is necessary would be nice...

> +QmpCommand cmd2;
> +
> +static QObject *do_qmp_dispatch(QmpCommandList *cmds, QObject *request,
> +                                Error **errp)
>  {
>      Error *local_err = NULL;
>      const char *command;
> @@ -81,7 +85,7 @@ static QObject *do_qmp_dispatch(QObject *request, Error **errp)
>      }
>  
>      command = qdict_get_str(dict, "execute");
> -    cmd = qmp_find_command(command);
> +    cmd = qmp_find_command(cmds, command);
>      if (cmd == NULL) {
>          error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
>                    "The command %s has not been found", command);
> @@ -93,6 +97,9 @@ static QObject *do_qmp_dispatch(QObject *request, Error **errp)
>          return NULL;
>      }
>  
> +    assert(!cmd->options & QCO_NO_SUCCESS_RESP);
> +    save_cmd = cmd;
> +    cmd2 = *cmd;
>      if (!qdict_haskey(dict, "arguments")) {
>          args = qdict_new();
>      } else {
> @@ -111,6 +118,8 @@ static QObject *do_qmp_dispatch(QObject *request, Error **errp)
>  
>      QDECREF(args);
>  
> +    assert(!cmd->options & QCO_NO_SUCCESS_RESP);
> +    assert(ret || local_err);

...or is this leftovers from your debugging?

The rest of the patch looks fine; it is converting a global variable
into a per-instance variable.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]