Re: [Qemu-devel] [PATCH 1/3] qapi: Allow QMP/QAPI commands to have array inputs

[Jeff Cody]

On 02/22/2012 09:53 AM, Anthony Liguori wrote:
> On 02/20/2012 11:31 AM, Jeff Cody wrote:
> > The QAPI scripts allow for generating commands that receive parameter
> > input consisting of a list of custom structs, but the QMP input paramter
> > checking did not support receiving a qlist as opposed to a qdict for
> > input.
>
> What are you trying to send on the wire? Something like
>
> {"execute": "foo", "arguments": [ "a", "b", "c" ]}
>
> ?
>
> That's not valid per the QMP protocol. "arguments" must always be a QMP
> dictionary.

Not a bare list like that (perhaps my wording was poor), but rather an 
array of custom structs, that contain QMP dicts.  In this particular case:

{"execute": "blockdev-group-snapshot-sync", "arguments": [{ "device": 
"ide-hd0","snapshot-file":"/some/place/my-image","format": "qcow2" }]}

When specifying this in the schema JSON file as shown below[1], the QAPI 
scripts generate the marshaller & visitor functions that handle 
everything correctly.  The monitor code, however, could not deal with 
the QList container for the input parameters, prior to passing the data 
to the generated functions.


[1] JSON schema definition:

{ 'type': 'SnapshotDev',
  'data': {'device': 'str', 'snapshot-file': 'str', '*format': 'str' } }

{ 'command': 'blockdev-group-snapshot-sync',
  'data': { 'dev': [ 'SnapshotDev' ] } }



> > This patch allows for array input parameters, although no argument
> > validation
> > is performed for commands that accept a list input.
> >
> > Signed-off-by: Jeff Cody<address@hidden>
> > ---
> > monitor.c | 72
> > ++++++++++++++++++++++++++++++++++++++++++++++++++----------
> > monitor.h | 1 +
> > 2 files changed, 61 insertions(+), 12 deletions(-)
> >
> > diff --git a/monitor.c b/monitor.c
> > index a7df995..98e6017 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -125,8 +125,12 @@ typedef struct mon_cmd_t {
> > void (*info)(Monitor *mon);
> > void (*cmd)(Monitor *mon, const QDict *qdict);
> > int (*cmd_new)(Monitor *mon, const QDict *params, QObject **ret_data);
> > + int (*cmd_new_list)(Monitor *mon, const QList *params,
> > + QObject **ret_data);
> > int (*cmd_async)(Monitor *mon, const QDict *params,
> > MonitorCompletion *cb, void *opaque);
> > + int (*cmd_async_list)(Monitor *mon, const QList *params,
> > + MonitorCompletion *cb, void *opaque);
> > } mhandler;
> > bool qapi;
> > int flags;
> > @@ -353,6 +357,11 @@ static inline bool handler_is_async(const
> > mon_cmd_t *cmd)
> > return cmd->flags& MONITOR_CMD_ASYNC;
> > }
> >
> > +static inline bool handler_accepts_array(const mon_cmd_t *cmd)
> > +{
> > + return cmd->flags& MONITOR_CMD_ARRAY_INPUT;
> > +}
> > +
> > static inline int monitor_has_error(const Monitor *mon)
> > {
> > return mon->error != NULL;
> > @@ -671,6 +680,12 @@ static int qmp_async_cmd_handler(Monitor *mon,
> > const mon_cmd_t *cmd,
> > return cmd->mhandler.cmd_async(mon, params, qmp_monitor_complete, mon);
> > }
> >
> > +static int qmp_async_cmd_handler_array(Monitor *mon, const mon_cmd_t
> > *cmd,
> > + const QList *params)
> > +{
> > + return cmd->mhandler.cmd_async_list(mon, params,
> > qmp_monitor_complete, mon);
> > +}
> > +
> > static void user_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd,
> > const QDict *params)
> > {
> > @@ -4310,7 +4325,8 @@ static QDict *qmp_check_input_obj(QObject
> > *input_obj)
> > }
> > has_exec_key = 1;
> > } else if (!strcmp(arg_name, "arguments")) {
> > - if (qobject_type(arg_obj) != QTYPE_QDICT) {
> > + if ((qobject_type(arg_obj) != QTYPE_QDICT)&&
> > + (qobject_type(arg_obj) != QTYPE_QLIST)) {
> > qerror_report(QERR_QMP_BAD_INPUT_OBJECT_MEMBER, "arguments",
> > "object");
> > return NULL;
> > @@ -4345,11 +4361,26 @@ static void qmp_call_cmd(Monitor *mon, const
> > mon_cmd_t *cmd,
> > qobject_decref(data);
> > }
> >
> > +static void qmp_call_cmd_array(Monitor *mon, const mon_cmd_t *cmd,
> > + const QList *params)
> > +{
> > + int ret;
> > + QObject *data = NULL;
> > +
> > + mon_print_count_init(mon);
> > +
> > + ret = cmd->mhandler.cmd_new_list(mon, params,&data);
> > + handler_audit(mon, cmd, ret);
> > + monitor_protocol_emitter(mon, data);
> > + qobject_decref(data);
> > +}
> > +
> > static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
> > {
> > int err;
> > QObject *obj;
> > QDict *input, *args;
> > + QList *args_list = NULL;
> > const mon_cmd_t *cmd;
> > const char *cmd_name;
> > Monitor *mon = cur_mon;
> > @@ -4386,26 +4417,42 @@ static void
> > handle_qmp_command(JSONMessageParser *parser, QList *tokens)
> > }
> >
> > obj = qdict_get(input, "arguments");
> > - if (!obj) {
> > - args = qdict_new();
> > + if (handler_accepts_array(cmd)) {
> > + if (!obj || (qobject_type(obj) != QTYPE_QLIST)) {
> > + args_list = qlist_new();
> > + } else {
> > + args_list = qobject_to_qlist(obj);
> > + QINCREF(args_list);
> > + }
> > } else {
> > - args = qobject_to_qdict(obj);
> > - QINCREF(args);
> > - }
> > -
> > - err = qmp_check_client_args(cmd, args);
> > - if (err< 0) {
> > - goto err_out;
> > + if (!obj || (qobject_type(obj) != QTYPE_QDICT)) {
> > + args = qdict_new();
> > + } else {
> > + args = qobject_to_qdict(obj);
> > + QINCREF(args);
> > + }
> > + err = qmp_check_client_args(cmd, args);
> > + if (err< 0) {
> > + goto err_out;
> > + }
> > }
> >
> > if (handler_is_async(cmd)) {
> > - err = qmp_async_cmd_handler(mon, cmd, args);
> > + if (handler_accepts_array(cmd)) {
> > + err = qmp_async_cmd_handler_array(mon, cmd, args_list);
> > + } else {
> > + err = qmp_async_cmd_handler(mon, cmd, args);
> > + }
> > if (err) {
> > /* emit the error response */
> > goto err_out;
> > }
> > } else {
> > - qmp_call_cmd(mon, cmd, args);
> > + if (handler_accepts_array(cmd)) {
> > + qmp_call_cmd_array(mon, cmd, args_list);
> > + } else {
> > + qmp_call_cmd(mon, cmd, args);
> > + }
> > }
> >
> > goto out;
> > @@ -4415,6 +4462,7 @@ err_out:
> > out:
> > QDECREF(input);
> > QDECREF(args);
> > + QDECREF(args_list);
> > }
> >
> > /**
> > diff --git a/monitor.h b/monitor.h
> > index b72ea07..499fc1f 100644
> > --- a/monitor.h
> > +++ b/monitor.h
> > @@ -19,6 +19,7 @@ extern Monitor *default_mon;
> >
> > /* flags for monitor commands */
> > #define MONITOR_CMD_ASYNC 0x0001
> > +#define MONITOR_CMD_ARRAY_INPUT 0x0002
> >
> > /* QMP events */
> > typedef enum MonitorEvent {