From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42063) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ba4Gk-0002Sb-77 for qemu-devel@nongnu.org; Wed, 17 Aug 2016 13:04:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ba4Gg-0003zr-TU for qemu-devel@nongnu.org; Wed, 17 Aug 2016 13:04:46 -0400 Received: from mx3-phx2.redhat.com ([209.132.183.24]:34093) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ba4Gg-0003zk-Fe for qemu-devel@nongnu.org; Wed, 17 Aug 2016 13:04:42 -0400 Date: Wed, 17 Aug 2016 13:04:40 -0400 (EDT) From: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau Message-ID: <2142569659.532979.1471453480950.JavaMail.zimbra@redhat.com> In-Reply-To: <20160817165757.23486-18-marcandre.lureau@redhat.com> References: <20160817165757.23486-1-marcandre.lureau@redhat.com> <20160817165757.23486-18-marcandre.lureau@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v5 17/20] monitor: use qmp_dispatch() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau Cc: qemu-devel@nongnu.org, eblake@redhat.com, armbru@redhat.com Hi ----- Original Message ----- > Replace the old manual dispatch and validation code by the generic one > provided by qapi common code. >=20 > Note that it is now possible to call the following commands that used to > be disabled by compile-time conditionals: > - dump-skeys > - query-spice > - rtc-reset-reinjection > - query-gic-capabilities >=20 > Their fallback functions return an appropriate "feature disabled" error. This is no longer valid, I dropped that comment. > Signed-off-by: Marc-Andr=C3=A9 Lureau > --- > monitor.c | 326 > +++++++---------------------------------------------------- > trace-events | 1 - > 2 files changed, 34 insertions(+), 293 deletions(-) >=20 > diff --git a/monitor.c b/monitor.c > index f07fc31..81926c7 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -166,7 +166,6 @@ struct MonFdset { > }; > =20 > typedef struct { > - QObject *id; > JSONMessageParser parser; > /* > * When a client connects, we're in capabilities negotiation mode. > @@ -229,8 +228,6 @@ static int mon_refcount; > static mon_cmd_t mon_cmds[]; > static mon_cmd_t info_cmds[]; > =20 > -static const mon_cmd_t qmp_cmds[]; > - > Monitor *cur_mon; > =20 > static QEMUClockType event_clock_type =3D QEMU_CLOCK_REALTIME; > @@ -401,49 +398,6 @@ static void monitor_json_emitter(Monitor *mon, const > QObject *data) > QDECREF(json); > } > =20 > -static QDict *build_qmp_error_dict(Error *err) > -{ > - QObject *obj; > - > - obj =3D qobject_from_jsonf("{ 'error': { 'class': %s, 'desc': %s } }= ", > - QapiErrorClass_lookup[error_get_class(err)]= , > - error_get_pretty(err)); > - > - return qobject_to_qdict(obj); > -} > - > -static void monitor_protocol_emitter(Monitor *mon, QObject *data, > - Error *err) > -{ > - QDict *qmp; > - > - trace_monitor_protocol_emitter(mon); > - > - if (!err) { > - /* success response */ > - qmp =3D qdict_new(); > - if (data) { > - qobject_incref(data); > - qdict_put_obj(qmp, "return", data); > - } else { > - /* return an empty QDict by default */ > - qdict_put(qmp, "return", qdict_new()); > - } > - } else { > - /* error response */ > - qmp =3D build_qmp_error_dict(err); > - } > - > - if (mon->qmp.id) { > - qdict_put_obj(qmp, "id", mon->qmp.id); > - mon->qmp.id =3D NULL; > - } > - > - monitor_json_emitter(mon, QOBJECT(qmp)); > - QDECREF(qmp); > -} > - > - > static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] =3D= { > /* Limit guest-triggerable events to 1 per second */ > [QAPI_EVENT_RTC_CHANGE] =3D { 1000 * SCALE_MS }, > @@ -2180,11 +2134,6 @@ static mon_cmd_t mon_cmds[] =3D { > { NULL, NULL, }, > }; > =20 > -static const mon_cmd_t qmp_cmds[] =3D { > -#include "qmp-commands-old.h" > - { /* NULL */ }, > -}; > - > /*******************************************************************/ > =20 > static const char *pch; > @@ -2535,11 +2484,6 @@ static const mon_cmd_t *search_dispatch_table(cons= t > mon_cmd_t *disp_table, > return NULL; > } > =20 > -static const mon_cmd_t *qmp_find_cmd(const char *cmdname) > -{ > - return search_dispatch_table(qmp_cmds, cmdname); > -} > - > /* > * Parse command name from @cmdp according to command table @table. > * If blank, return NULL. > @@ -3690,199 +3634,6 @@ static bool invalid_qmp_mode(const Monitor *mon, > const char *cmd, > return false; > } > =20 > -/* > - * Argument validation rules: > - * > - * 1. The argument must exist in cmd_args qdict > - * 2. The argument type must be the expected one > - * > - * Special case: If the argument doesn't exist in cmd_args and > - * the QMP_ACCEPT_UNKNOWNS flag is set, then the > - * checking is skipped for it. > - */ > -static void check_client_args_type(const QDict *client_args, > - const QDict *cmd_args, int flags, > - Error **errp) > -{ > - const QDictEntry *ent; > - > - for (ent =3D qdict_first(client_args); ent;ent =3D > qdict_next(client_args,ent)){ > - QObject *obj; > - QString *arg_type; > - const QObject *client_arg =3D qdict_entry_value(ent); > - const char *client_arg_name =3D qdict_entry_key(ent); > - > - obj =3D qdict_get(cmd_args, client_arg_name); > - if (!obj) { > - if (flags & QMP_ACCEPT_UNKNOWNS) { > - /* handler accepts unknowns */ > - continue; > - } > - /* client arg doesn't exist */ > - error_setg(errp, QERR_INVALID_PARAMETER, client_arg_name); > - return; > - } > - > - arg_type =3D qobject_to_qstring(obj); > - assert(arg_type !=3D NULL); > - > - /* check if argument's type is correct */ > - switch (qstring_get_str(arg_type)[0]) { > - case 'F': > - case 'B': > - case 's': > - if (qobject_type(client_arg) !=3D QTYPE_QSTRING) { > - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, > - client_arg_name, "string"); > - return; > - } > - break; > - case 'i': > - case 'l': > - case 'M': > - case 'o': > - if (qobject_type(client_arg) !=3D QTYPE_QINT) { > - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, > - client_arg_name, "int"); > - return; > - } > - break; > - case 'T': > - if (qobject_type(client_arg) !=3D QTYPE_QINT && > - qobject_type(client_arg) !=3D QTYPE_QFLOAT) { > - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, > - client_arg_name, "number"); > - return; > - } > - break; > - case 'b': > - case '-': > - if (qobject_type(client_arg) !=3D QTYPE_QBOOL) { > - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, > - client_arg_name, "bool"); > - return; > - } > - break; > - case 'O': > - assert(flags & QMP_ACCEPT_UNKNOWNS); > - break; > - case 'q': > - /* Any QObject can be passed. */ > - break; > - case '/': > - case '.': > - /* > - * These types are not supported by QMP and thus are not > - * handled here. Fall through. > - */ > - default: > - abort(); > - } > - } > -} > - > -/* > - * - Check if the client has passed all mandatory args > - * - Set special flags for argument validation > - */ > -static void check_mandatory_args(const QDict *cmd_args, > - const QDict *client_args, int *flags, > - Error **errp) > -{ > - const QDictEntry *ent; > - > - for (ent =3D qdict_first(cmd_args); ent; ent =3D qdict_next(cmd_args= , ent)) > { > - const char *cmd_arg_name =3D qdict_entry_key(ent); > - QString *type =3D qobject_to_qstring(qdict_entry_value(ent)); > - assert(type !=3D NULL); > - > - if (qstring_get_str(type)[0] =3D=3D 'O') { > - assert((*flags & QMP_ACCEPT_UNKNOWNS) =3D=3D 0); > - *flags |=3D QMP_ACCEPT_UNKNOWNS; > - } else if (qstring_get_str(type)[0] !=3D '-' && > - qstring_get_str(type)[1] !=3D '?' && > - !qdict_haskey(client_args, cmd_arg_name)) { > - error_setg(errp, QERR_MISSING_PARAMETER, cmd_arg_name); > - return; > - } > - } > -} > - > -static QDict *qdict_from_args_type(const char *args_type) > -{ > - int i; > - QDict *qdict; > - QString *key, *type, *cur_qs; > - > - assert(args_type !=3D NULL); > - > - qdict =3D qdict_new(); > - > - if (args_type =3D=3D NULL || args_type[0] =3D=3D '\0') { > - /* no args, empty qdict */ > - goto out; > - } > - > - key =3D qstring_new(); > - type =3D qstring_new(); > - > - cur_qs =3D key; > - > - for (i =3D 0;; i++) { > - switch (args_type[i]) { > - case ',': > - case '\0': > - qdict_put(qdict, qstring_get_str(key), type); > - QDECREF(key); > - if (args_type[i] =3D=3D '\0') { > - goto out; > - } > - type =3D qstring_new(); /* qdict has ref */ > - cur_qs =3D key =3D qstring_new(); > - break; > - case ':': > - cur_qs =3D type; > - break; > - default: > - qstring_append_chr(cur_qs, args_type[i]); > - break; > - } > - } > - > -out: > - return qdict; > -} > - > -/* > - * Client argument checking rules: > - * > - * 1. Client must provide all mandatory arguments > - * 2. Each argument provided by the client must be expected > - * 3. Each argument provided by the client must have the type expected > - * by the command > - */ > -static void qmp_check_client_args(const mon_cmd_t *cmd, QDict *client_ar= gs, > - Error **errp) > -{ > - Error *err =3D NULL; > - int flags; > - QDict *cmd_args; > - > - cmd_args =3D qdict_from_args_type(cmd->args_type); > - > - flags =3D 0; > - check_mandatory_args(cmd_args, client_args, &flags, &err); > - if (err) { > - goto out; > - } > - > - check_client_args_type(client_args, cmd_args, flags, &err); > - > -out: > - error_propagate(errp, err); > - QDECREF(cmd_args); > -} > - > /* > * Input object checking rules > * > @@ -3941,67 +3692,58 @@ static QDict *qmp_check_input_obj(QObject *input_= obj, > Error **errp) > =20 > static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens= ) > { > - Error *local_err =3D NULL; > - QObject *obj, *data; > - QDict *input, *args; > - const mon_cmd_t *cmd; > - QmpCommand *qcmd; > + QObject *req, *rsp =3D NULL, *id =3D NULL; > + QDict *qdict =3D NULL; > const char *cmd_name; > Monitor *mon =3D cur_mon; > + Error *err =3D NULL; > =20 > - args =3D input =3D NULL; > - data =3D NULL; > - > - obj =3D json_parser_parse(tokens, NULL); > - if (!obj) { > - // FIXME: should be triggered in json_parser_parse() > - error_setg(&local_err, QERR_JSON_PARSING); > + req =3D json_parser_parse_err(tokens, NULL, &err); > + if (err || !req || qobject_type(req) !=3D QTYPE_QDICT) { > + if (!err) { > + error_setg(&err, QERR_JSON_PARSING); > + } > goto err_out; > } > =20 > - input =3D qmp_check_input_obj(obj, &local_err); > - if (!input) { > - qobject_decref(obj); > + qdict =3D qmp_check_input_obj(req, &err); > + if (!qdict) { > goto err_out; > } > =20 > - mon->qmp.id =3D qdict_get(input, "id"); > - qobject_incref(mon->qmp.id); > + id =3D qdict_get(qdict, "id"); > + qobject_incref(id); > + qdict_del(qdict, "id"); > =20 > - cmd_name =3D qdict_get_str(input, "execute"); > + cmd_name =3D qdict_get_str(qdict, "execute"); > trace_handle_qmp_command(mon, cmd_name); > - cmd =3D qmp_find_cmd(cmd_name); > - qcmd =3D qmp_find_command(cmd_name); > - if (!qcmd || !cmd) { > - error_set(&local_err, ERROR_CLASS_COMMAND_NOT_FOUND, > - "The command %s has not been found", cmd_name); > - goto err_out; > - } > - if (invalid_qmp_mode(mon, cmd_name, &local_err)) { > + > + if (invalid_qmp_mode(mon, cmd_name, &err)) { > goto err_out; > } > =20 > - obj =3D qdict_get(input, "arguments"); > - if (!obj) { > - args =3D qdict_new(); > - } else { > - args =3D qobject_to_qdict(obj); > - QINCREF(args); > - } > + rsp =3D qmp_dispatch(req); > =20 > - qmp_check_client_args(cmd, args, &local_err); > - if (local_err) { > - goto err_out; > +err_out: > + if (err) { > + qdict =3D qdict_new(); > + qdict_put_obj(qdict, "error", qmp_build_error_object(err)); > + error_free(err); > + rsp =3D QOBJECT(qdict); > } > =20 > - qcmd->fn(args, &data, &local_err); > + if (rsp) { > + if (id) { > + qdict_put_obj(qobject_to_qdict(rsp), "id", id); > + id =3D NULL; > + } > =20 > -err_out: > - monitor_protocol_emitter(mon, data, local_err); > - qobject_decref(data); > - error_free(local_err); > - QDECREF(input); > - QDECREF(args); > + monitor_json_emitter(mon, rsp); > + } > + > + qobject_decref(id); > + qobject_decref(rsp); > + qobject_decref(req); > } > =20 > static void monitor_qmp_read(void *opaque, const uint8_t *buf, int size) > diff --git a/trace-events b/trace-events > index 616cc52..8d59631 100644 > --- a/trace-events > +++ b/trace-events > @@ -98,7 +98,6 @@ qemu_co_mutex_unlock_return(void *mutex, void *self) "m= utex > %p self %p" > =20 > # monitor.c > handle_qmp_command(void *mon, const char *cmd_name) "mon %p cmd_name \"%= s\"" > -monitor_protocol_emitter(void *mon) "mon %p" > monitor_protocol_event_handler(uint32_t event, void *qdict) "event=3D%d > data=3D%p" > monitor_protocol_event_emit(uint32_t event, void *data) "event=3D%d data= =3D%p" > monitor_protocol_event_queue(uint32_t event, void *qdict, uint64_t rate) > "event=3D%d data=3D%p rate=3D%" PRId64 > -- > 2.9.0 >=20 >=20