Clean up qmp_dispatch usage to have consistant checks between qga &
qemu, and simplify QmpClient/parser_feed usage.
Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
---
include/qapi/qmp/dispatch.h | 13 +++-
monitor.c | 140 ++++++++------------------------------------
qapi/qmp-dispatch.c | 124 ++++++++++++++++++++++++++++++++++++++-
qga/main.c | 61 +------------------
qobject/json-lexer.c | 4 +-
tests/test-qmp-commands.c | 12 ++--
6 files changed, 172 insertions(+), 182 deletions(-)
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index 4dd6de5ab2..0a545935d5 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;
@@ -68,8 +74,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);
diff --git a/monitor.c b/monitor.c
index 3ff32c000c..f763da462d 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 "trace.h"
@@ -158,7 +157,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
@@ -600,9 +598,6 @@ static void monitor_data_init(Monitor *mon)
static void monitor_data_destroy(Monitor *mon)
{
qemu_chr_fe_deinit(&mon->chr);
- 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);
@@ -3700,62 +3695,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));
@@ -3774,71 +3713,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.client.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)
@@ -3849,7 +3759,7 @@ 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;
}
@@ -3926,7 +3836,10 @@ static void monitor_qmp_event(void *opaque, int event)
switch (event) {
case CHR_EVENT_OPENED:
- qmp_client_init(&mon->qmp.client, qmp_dispatch_return);
+ 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);
@@ -3934,8 +3847,6 @@ 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);
mon_refcount--;
monitor_fdsets_cleanup();
@@ -4094,7 +4005,6 @@ void monitor_init(CharDriverState *chr, int flags)
qemu_chr_fe_set_handlers(&mon->chr, monitor_can_read, monitor_qmp_read,
monitor_qmp_event, mon, NULL, true);
qemu_chr_fe_set_echo(&mon->chr, true);
- json_message_parser_init(&mon->qmp.parser, handle_qmp_command);
} else {
qemu_chr_fe_set_handlers(&mon->chr, monitor_can_read, monitor_read,
monitor_event, mon, NULL, true);
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 5bf4b1b520..9c5cfc6b5a 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;
@@ -180,10 +186,125 @@ 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)
{
assert(!client->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);
}
@@ -193,6 +314,7 @@ void qmp_client_destroy(QmpClient *client)
QmpReturn *ret, *next;
client->return_cb = NULL;
+ 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 a75544ed7a..d42b532cf4 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)
{
@@ -625,8 +572,8 @@ static gboolean channel_event_cb(GIOCondition condition,
gpointer data)
return false;
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);
+ g_debug("read data, count: %" G_GSIZE_FORMAT ", data: %s", count, buf);
+ qmp_client_feed(&s->client, buf, count);
break;
case G_IO_STATUS_EOF:
g_debug("received EOF");
@@ -1285,9 +1232,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");
@@ -1376,7 +1322,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 af4a75e05b..94f0db30df 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 4613a9a4c8..9656fbb529 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 = { 0, };
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 = { 0, };
- 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);
@@ -287,7 +287,7 @@ static void test_dispatch_cmd_async_no_id(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(args, "a", qint_from_int(99));
qdict_put(req, "arguments", args);
@@ -311,7 +311,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);
--
2.11.0.295.gd7dffce1c
