[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH 24/24] qmp: move json-message-parser to QmpClient
From: |
Marc-André Lureau |
Subject: |
[Qemu-devel] [PATCH 24/24] qmp: move json-message-parser to QmpClient |
Date: |
Mon, 10 Oct 2016 13:23:01 +0400 |
Clean up qmp_dispatch usage to have consistant checks between qga &
qemu, and simplify QmpClient/parser_feed usage.
Signed-off-by: Marc-André Lureau <address@hidden>
---
monitor.c | 142 ++++++++------------------------------------
qapi/qmp-dispatch.c | 125 +++++++++++++++++++++++++++++++++++++-
qga/main.c | 59 +-----------------
qobject/json-lexer.c | 4 +-
tests/test-qmp-commands.c | 10 ++--
include/qapi/qmp/dispatch.h | 13 +++-
6 files changed, 172 insertions(+), 181 deletions(-)
diff --git a/monitor.c b/monitor.c
index 0b47f83..2b60885 100644
--- a/monitor.c
+++ b/monitor.c
@@ -56,7 +56,6 @@
#include "qapi/qmp/qerror.h"
#include "qapi/qmp/types.h"
#include "qapi/qmp/qjson.h"
-#include "qapi/qmp/json-streamer.h"
#include "qapi/qmp/json-parser.h"
#include "qom/object_interfaces.h"
#include "cpu.h"
@@ -166,7 +165,6 @@ struct MonFdset {
};
typedef struct {
- JSONMessageParser parser;
/*
* When a client connects, we're in capabilities negotiation mode.
* When command qmp_capabilities succeeds, we go into command
@@ -610,9 +608,6 @@ static void monitor_data_destroy(Monitor *mon)
if (mon->chr) {
qemu_chr_add_handlers(mon->chr, NULL, NULL, NULL, NULL);
}
- if (monitor_is_qmp(mon)) {
- json_message_parser_destroy(&mon->qmp.parser);
- }
qmp_client_destroy(&mon->qmp.client);
g_free(mon->rs);
QDECREF(mon->outbuf);
@@ -3708,62 +3703,6 @@ static bool invalid_qmp_mode(const Monitor *mon, const
char *cmd,
return false;
}
-/*
- * Input object checking rules
- *
- * 1. Input object must be a dict
- * 2. The "execute" key must exist
- * 3. The "execute" key must be a string
- * 4. If the "arguments" key exists, it must be a dict
- * 5. If the "id" key exists, it can be anything (ie. json-value)
- * 6. Any argument not listed above is considered invalid
- */
-static QDict *qmp_check_input_obj(QObject *input_obj, Error **errp)
-{
- const QDictEntry *ent;
- int has_exec_key = 0;
- QDict *input_dict;
-
- if (qobject_type(input_obj) != QTYPE_QDICT) {
- error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT, "object");
- return NULL;
- }
-
- input_dict = qobject_to_qdict(input_obj);
-
- for (ent = qdict_first(input_dict); ent; ent = qdict_next(input_dict,
ent)){
- const char *arg_name = qdict_entry_key(ent);
- const QObject *arg_obj = qdict_entry_value(ent);
-
- if (!strcmp(arg_name, "execute")) {
- if (qobject_type(arg_obj) != QTYPE_QSTRING) {
- error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT_MEMBER,
- "execute", "string");
- return NULL;
- }
- has_exec_key = 1;
- } else if (!strcmp(arg_name, "arguments")) {
- if (qobject_type(arg_obj) != QTYPE_QDICT) {
- error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT_MEMBER,
- "arguments", "object");
- return NULL;
- }
- } else if (!strcmp(arg_name, "id")) {
- /* Any string is acceptable as "id", so nothing to check */
- } else {
- error_setg(errp, QERR_QMP_EXTRA_MEMBER, arg_name);
- return NULL;
- }
- }
-
- if (!has_exec_key) {
- error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT, "execute");
- return NULL;
- }
-
- return input_dict;
-}
-
static void monitor_qmp_suspend(Monitor *mon, QObject *req)
{
assert(monitor_is_qmp(mon));
@@ -3782,71 +3721,42 @@ static void monitor_qmp_resume(Monitor *mon)
mon->qmp.suspended = NULL;
}
-static void qmp_dispatch_return(QmpClient *client, QObject *rsp)
+static bool qmp_pre_dispatch(QmpClient *client, QObject *req, Error **errp)
{
Monitor *mon = container_of(client, Monitor, qmp.client);
+ QDict *qdict = qobject_to_qdict(req);
+ const char *cmd_name = qdict_get_str(qdict, "execute");
- monitor_json_emitter(mon, rsp);
+ trace_handle_qmp_command(mon, cmd_name);
- if (mon->qmp.suspended) {
- monitor_qmp_resume(mon);
+ if (invalid_qmp_mode(mon, cmd_name, errp)) {
+ return false;
}
+
+ return true;
}
-static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
+static bool qmp_post_dispatch(QmpClient *client, QObject *req, Error **errp)
{
- QObject *req, *id = NULL;
- QDict *qdict, *rqdict = qdict_new();
- const char *cmd_name;
- Monitor *mon = cur_mon;
- Error *err = NULL;
-
- req = json_parser_parse_err(tokens, NULL, &err);
- if (err || !req || qobject_type(req) != QTYPE_QDICT) {
- if (!err) {
- error_setg(&err, QERR_JSON_PARSING);
- }
- goto err_out;
- }
-
- qdict = qmp_check_input_obj(req, &err);
- if (!qdict) {
- goto err_out;
- }
-
- id = qdict_get(qdict, "id");
- if (id) {
- qobject_incref(id);
- qdict_del(qdict, "id");
- qdict_put_obj(rqdict, "id", id);
- }
-
- cmd_name = qdict_get_str(qdict, "execute");
- trace_handle_qmp_command(mon, cmd_name);
-
- if (invalid_qmp_mode(mon, cmd_name, &err)) {
- goto err_out;
- }
-
- qmp_dispatch(&mon->qmp.client, req, rqdict);
+ Monitor *mon = container_of(client, Monitor, qmp.client);
/* suspend if the command is on-going and client doesn't support async */
if (!QLIST_EMPTY(&mon->qmp.client.pending) && !mon->qmp.has_async) {
monitor_qmp_suspend(mon, req);
}
- qobject_decref(req);
- return;
+ return true;
+}
-err_out:
- if (err) {
- qdict_put_obj(rqdict, "error", qmp_build_error_object(err));
- error_free(err);
- monitor_json_emitter(mon, QOBJECT(rqdict));
- }
+static void qmp_dispatch_return(QmpClient *client, QObject *rsp)
+{
+ Monitor *mon = container_of(client, Monitor, qmp.client);
+
+ monitor_json_emitter(mon, rsp);
- QDECREF(rqdict);
- qobject_decref(req);
+ if (mon->qmp.suspended) {
+ monitor_qmp_resume(mon);
+ }
}
static void monitor_qmp_read(void *opaque, const uint8_t *buf, int size)
@@ -3857,7 +3767,8 @@ static void monitor_qmp_read(void *opaque, const uint8_t
*buf, int size)
assert(monitor_is_qmp(cur_mon));
- json_message_parser_feed(&cur_mon->qmp.parser, (const char *) buf, size);
+ qmp_client_feed(&cur_mon->qmp.client,
+ (const char *) buf, size);
cur_mon = old_mon;
}
@@ -3934,6 +3845,10 @@ static void monitor_qmp_event(void *opaque, int event)
switch (event) {
case CHR_EVENT_OPENED:
+ qmp_client_init(&mon->qmp.client,
+ qmp_pre_dispatch,
+ qmp_post_dispatch,
+ qmp_dispatch_return);
mon->qmp.in_command_mode = false;
data = get_qmp_greeting();
monitor_json_emitter(mon, data);
@@ -3941,10 +3856,7 @@ static void monitor_qmp_event(void *opaque, int event)
mon_refcount++;
break;
case CHR_EVENT_CLOSED:
- json_message_parser_destroy(&mon->qmp.parser);
- json_message_parser_init(&mon->qmp.parser, handle_qmp_command);
qmp_client_destroy(&mon->qmp.client);
- qmp_client_init(&mon->qmp.client, qmp_dispatch_return);
mon_refcount--;
monitor_fdsets_cleanup();
break;
@@ -4081,8 +3993,6 @@ void monitor_init(CharDriverState *chr, int flags)
qemu_chr_add_handlers(chr, monitor_can_read, monitor_qmp_read,
monitor_qmp_event, mon);
qemu_chr_fe_set_echo(chr, true);
- json_message_parser_init(&mon->qmp.parser, handle_qmp_command);
- qmp_client_init(&mon->qmp.client, qmp_dispatch_return);
} else {
qemu_chr_add_handlers(chr, monitor_can_read, monitor_read,
monitor_event, mon);
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index b6a1feb..d972b4c 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -20,6 +20,12 @@
#include "qapi-types.h"
#include "qapi/qmp/qerror.h"
+void qmp_client_feed(QmpClient *client,
+ const char *buffer, size_t size)
+{
+ json_message_parser_feed(&client->parser, buffer, size);
+}
+
static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp)
{
const QDictEntry *ent;
@@ -179,8 +185,123 @@ bool qmp_return_is_cancelled(QmpReturn *qret)
return false;
}
-void qmp_client_init(QmpClient *client, QmpDispatchReturn *return_cb)
+/*
+ * Input object checking rules
+ *
+ * 1. Input object must be a dict
+ * 2. The "execute" key must exist
+ * 3. The "execute" key must be a string
+ * 4. If the "arguments" key exists, it must be a dict
+ * 5. If the "id" key exists, it can be anything (ie. json-value)
+ * 6. Any argument not listed above is considered invalid
+ */
+static QDict *qmp_check_input_obj(QObject *input_obj, Error **errp)
+{
+ const QDictEntry *ent;
+ int has_exec_key = 0;
+ QDict *dict;
+
+ if (qobject_type(input_obj) != QTYPE_QDICT) {
+ error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT, "object");
+ return NULL;
+ }
+
+ dict = qobject_to_qdict(input_obj);
+
+ for (ent = qdict_first(dict); ent; ent = qdict_next(dict, ent)) {
+ const char *arg_name = qdict_entry_key(ent);
+ const QObject *arg_obj = qdict_entry_value(ent);
+
+ if (!strcmp(arg_name, "execute")) {
+ if (qobject_type(arg_obj) != QTYPE_QSTRING) {
+ error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT_MEMBER,
+ "execute", "string");
+ return NULL;
+ }
+ has_exec_key = 1;
+ } else if (!strcmp(arg_name, "arguments")) {
+ if (qobject_type(arg_obj) != QTYPE_QDICT) {
+ error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT_MEMBER,
+ "arguments", "object");
+ return NULL;
+ }
+ } else if (!strcmp(arg_name, "id")) {
+ /* Any string is acceptable as "id", so nothing to check */
+ } else {
+ error_setg(errp, QERR_QMP_EXTRA_MEMBER, arg_name);
+ return NULL;
+ }
+ }
+
+ if (!has_exec_key) {
+ error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT, "execute");
+ return NULL;
+ }
+
+ return dict;
+}
+
+static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
{
+ QmpClient *client = container_of(parser, QmpClient, parser);
+ QObject *req, *id = NULL;
+ QDict *qdict, *rqdict = qdict_new();
+ Error *err = NULL;
+
+ req = json_parser_parse_err(tokens, NULL, &err);
+ if (err || !req || qobject_type(req) != QTYPE_QDICT) {
+ if (!err) {
+ error_setg(&err, QERR_JSON_PARSING);
+ }
+ goto err_out;
+ }
+
+ qdict = qmp_check_input_obj(req, &err);
+ if (!qdict) {
+ goto err_out;
+ }
+
+ id = qdict_get(qdict, "id");
+ if (id) {
+ qobject_incref(id);
+ qdict_del(qdict, "id");
+ qdict_put_obj(rqdict, "id", id);
+ }
+
+ if (client->pre_dispatch_cb &&
+ !client->pre_dispatch_cb(client, QOBJECT(qdict), &err)) {
+ goto err_out;
+ }
+
+ qmp_dispatch(client, req, rqdict);
+
+ if (client->post_dispatch_cb &&
+ !client->post_dispatch_cb(client, QOBJECT(qdict), &err)) {
+ goto err_out;
+ }
+
+ qobject_decref(req);
+ return;
+
+err_out:
+ if (err) {
+ qdict_put_obj(rqdict, "error", qmp_build_error_object(err));
+ error_free(err);
+ client->return_cb(client, QOBJECT(rqdict));
+ }
+
+ QDECREF(rqdict);
+ qobject_decref(req);
+}
+
+void qmp_client_init(QmpClient *client,
+ QmpPreDispatch *pre_dispatch_cb,
+ QmpPostDispatch *post_dispatch_cb,
+ QmpDispatchReturn *return_cb)
+{
+ json_message_parser_init(&client->parser, handle_qmp_command);
+ client->pre_dispatch_cb = pre_dispatch_cb;
+ client->post_dispatch_cb = post_dispatch_cb;
client->return_cb = return_cb;
QLIST_INIT(&client->pending);
}
@@ -189,6 +310,8 @@ void qmp_client_destroy(QmpClient *client)
{
QmpReturn *ret, *next;
+ json_message_parser_destroy(&client->parser);
+
/* Remove the weak references to the pending returns. The
* dispatched function is the owner of QmpReturn, and will have to
* qmp_return(). (it might be interesting to have a way to notify
diff --git a/qga/main.c b/qga/main.c
index 267e3bb..8f6a6e7 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -65,7 +65,6 @@ typedef struct GAPersistentState {
} GAPersistentState;
struct GAState {
- JSONMessageParser parser;
GMainLoop *main_loop;
GAChannel *channel;
bool virtio; /* fastpath to check for virtio to deal with poll() quirks */
@@ -559,59 +558,7 @@ static void dispatch_return_cb(QmpClient *client, QObject
*rsp)
}
}
-static void process_command(GAState *s, QDict *req)
-{
- g_assert(req);
- g_debug("processing command");
- qmp_dispatch(&ga_state->client, QOBJECT(req), NULL);
-}
-
/* handle requests/control events coming in over the channel */
-static void process_event(JSONMessageParser *parser, GQueue *tokens)
-{
- GAState *s = container_of(parser, GAState, parser);
- QDict *qdict;
- Error *err = NULL;
- int ret;
-
- g_assert(s && parser);
-
- g_debug("process_event: called");
- qdict = qobject_to_qdict(json_parser_parse_err(tokens, NULL, &err));
- if (err || !qdict) {
- QDECREF(qdict);
- qdict = qdict_new();
- if (!err) {
- g_warning("failed to parse event: unknown error");
- error_setg(&err, QERR_JSON_PARSING);
- } else {
- g_warning("failed to parse event: %s", error_get_pretty(err));
- }
- qdict_put_obj(qdict, "error", qmp_build_error_object(err));
- error_free(err);
- }
-
- /* handle host->guest commands */
- if (qdict_haskey(qdict, "execute")) {
- process_command(s, qdict);
- } else {
- if (!qdict_haskey(qdict, "error")) {
- QDECREF(qdict);
- qdict = qdict_new();
- g_warning("unrecognized payload format");
- error_setg(&err, QERR_UNSUPPORTED);
- qdict_put_obj(qdict, "error", qmp_build_error_object(err));
- error_free(err);
- }
- ret = send_response(s, QOBJECT(qdict));
- if (ret < 0) {
- g_warning("error sending error response: %s", strerror(-ret));
- }
- }
-
- QDECREF(qdict);
-}
-
/* false return signals GAChannel to close the current client connection */
static gboolean channel_event_cb(GIOCondition condition, gpointer data)
{
@@ -626,7 +573,7 @@ static gboolean channel_event_cb(GIOCondition condition,
gpointer data)
case G_IO_STATUS_NORMAL:
buf[count] = 0;
g_debug("read data, count: %d, data: %s", (int)count, buf);
- json_message_parser_feed(&s->parser, (char *)buf, (int)count);
+ qmp_client_feed(&s->client, (char *)buf, (int)count);
break;
case G_IO_STATUS_EOF:
g_debug("received EOF");
@@ -1283,9 +1230,8 @@ static int run_agent(GAState *s, GAConfig *config)
s->command_state = ga_command_state_new();
ga_command_state_init(s, s->command_state);
ga_command_state_init_all(s->command_state);
- json_message_parser_init(&s->parser, process_event);
ga_state = s;
- qmp_client_init(&s->client, dispatch_return_cb);
+ qmp_client_init(&s->client, NULL, NULL, dispatch_return_cb);
#ifndef _WIN32
if (!register_signal_handlers()) {
g_critical("failed to register signal handlers");
@@ -1374,7 +1320,6 @@ end:
if (s->command_state) {
ga_command_state_cleanup_all(s->command_state);
ga_command_state_free(s->command_state);
- json_message_parser_destroy(&s->parser);
}
if (s->channel) {
ga_channel_free(s->channel);
diff --git a/qobject/json-lexer.c b/qobject/json-lexer.c
index af4a75e..94f0db3 100644
--- a/qobject/json-lexer.c
+++ b/qobject/json-lexer.c
@@ -382,5 +382,7 @@ int json_lexer_flush(JSONLexer *lexer)
void json_lexer_destroy(JSONLexer *lexer)
{
- g_string_free(lexer->token, true);
+ if (lexer->token) {
+ g_string_free(lexer->token, true);
+ }
}
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index cca5555..2965dc1 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -127,7 +127,7 @@ static void test_dispatch_cmd(void)
QmpClient client;
QDict *req = qdict_new();
- qmp_client_init(&client, dispatch_cmd_return);
+ qmp_client_init(&client, NULL, NULL, dispatch_cmd_return);
qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd")));
@@ -150,7 +150,7 @@ static void test_dispatch_cmd_failure(void)
QDict *req = qdict_new();
QDict *args = qdict_new();
- qmp_client_init(&client, dispatch_cmd_error_return);
+ qmp_client_init(&client, NULL, NULL, dispatch_cmd_error_return);
qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd2")));
@@ -192,7 +192,7 @@ static QObject *test_qmp_dispatch(QDict *req)
QObject *ret;
QmpClient client;
- qmp_client_init(&client, qmp_dispatch_return);
+ qmp_client_init(&client, NULL, NULL, qmp_dispatch_return);
qmp_dispatch(&client, QOBJECT(req), NULL);
qmp_client_destroy(&client);
@@ -258,7 +258,7 @@ static void test_dispatch_cmd_async(void)
QDict *args = qdict_new();
loop = g_main_loop_new(NULL, FALSE);
- qmp_client_init(&client, qmp_dispatch_return);
+ qmp_client_init(&client, NULL, NULL, qmp_dispatch_return);
qdict_put(args, "a", qint_from_int(99));
qdict_put(req, "arguments", args);
@@ -290,7 +290,7 @@ static void test_destroy_pending_async(void)
int npending = 0;
loop = g_main_loop_new(NULL, FALSE);
- qmp_client_init(&client, qmp_dispatch_return);
+ qmp_client_init(&client, NULL, NULL, qmp_dispatch_return);
qdict_put(args, "a", qint_from_int(99));
qdict_put(req, "arguments", args);
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index e13e381..ed9c062 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -16,9 +16,12 @@
#include "qapi/qmp/qobject.h"
#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/json-streamer.h"
typedef struct QmpClient QmpClient;
+typedef bool (QmpPreDispatch) (QmpClient *client, QObject *rsp, Error **err);
+typedef bool (QmpPostDispatch) (QmpClient *client, QObject *rsp, Error **err);
typedef void (QmpDispatchReturn) (QmpClient *client, QObject *rsp);
typedef struct QmpReturn {
@@ -29,6 +32,9 @@ typedef struct QmpReturn {
} QmpReturn;
struct QmpClient {
+ JSONMessageParser parser;
+ QmpPreDispatch *pre_dispatch_cb;
+ QmpPostDispatch *post_dispatch_cb;
QmpDispatchReturn *return_cb;
QLIST_HEAD(, QmpReturn) pending;
@@ -67,8 +73,13 @@ void qmp_register_async_command(const char *name,
QmpCommandFuncAsync *fn,
QmpCommandOptions options);
void qmp_unregister_command(const char *name);
QmpCommand *qmp_find_command(const char *name);
-void qmp_client_init(QmpClient *client, QmpDispatchReturn *return_cb);
+void qmp_client_init(QmpClient *client,
+ QmpPreDispatch *pre_dispatch_cb,
+ QmpPostDispatch *post_dispatch_cb,
+ QmpDispatchReturn *return_cb);
void qmp_client_destroy(QmpClient *client);
+void qmp_client_feed(QmpClient *client, const char *buffer, size_t size);
+
void qmp_dispatch(QmpClient *client, QObject *request, QDict *rsp);
void qmp_disable_command(const char *name);
void qmp_enable_command(const char *name);
--
2.10.0
- [Qemu-devel] [PATCH 14/24] monitor: add !qmp pre-conditions, (continued)
- [Qemu-devel] [PATCH 14/24] monitor: add !qmp pre-conditions, Marc-André Lureau, 2016/10/10
- [Qemu-devel] [PATCH 15/24] monitor: suspend when running async and client has no async, Marc-André Lureau, 2016/10/10
- [Qemu-devel] [PATCH 16/24] qmp: update qmp-spec about async capability, Marc-André Lureau, 2016/10/10
- [Qemu-devel] [PATCH 17/24] qtest: add qtest-timeout, Marc-André Lureau, 2016/10/10
- [Qemu-devel] [PATCH 19/24] tests: add tests for async and non-async clients, Marc-André Lureau, 2016/10/10
- [Qemu-devel] [PATCH 18/24] qtest: add qtest_init_qmp_caps(), Marc-André Lureau, 2016/10/10
- [Qemu-devel] [PATCH 20/24] qapi: improve 'screendump' documentation, Marc-André Lureau, 2016/10/10
- [Qemu-devel] [PATCH 21/24] console: graphic_hw_update return true if async, Marc-André Lureau, 2016/10/10
- [Qemu-devel] [PATCH 22/24] console: add graphic_hw_update_done(), Marc-André Lureau, 2016/10/10
- [Qemu-devel] [PATCH 23/24] console: make screendump async, Marc-André Lureau, 2016/10/10
- [Qemu-devel] [PATCH 24/24] qmp: move json-message-parser to QmpClient,
Marc-André Lureau <=