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

[Jeff Cody <jcody@redhat.com>]

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<jcody@redhat.com>
>> ---
>> 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 {
>