From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:51165) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S0EoM-0001jp-1n for qemu-devel@nongnu.org; Wed, 22 Feb 2012 11:13:03 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S0EoG-000410-Hb for qemu-devel@nongnu.org; Wed, 22 Feb 2012 11:12:57 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59352) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S0EoG-00040v-4x for qemu-devel@nongnu.org; Wed, 22 Feb 2012 11:12:52 -0500 Message-ID: <4F4513FE.4070100@redhat.com> Date: Wed, 22 Feb 2012 11:12:46 -0500 From: Jeff Cody MIME-Version: 1.0 References: <4F450170.5070806@codemonkey.ws> In-Reply-To: <4F450170.5070806@codemonkey.ws> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/3] qapi: Allow QMP/QAPI commands to have array inputs Reply-To: jcody@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Kevin Wolf , Luiz Capitulino , qemu-devel@nongnu.org, Markus Armbruster 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 >> --- >> 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 { >