From 4eaca8de268d74ac5daaf8938abcb69d37ba2889 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 20 Feb 2019 16:42:53 +0100 Subject: [PATCH] qmp: common 'id' handling & make QGA conform to QMP spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let qmp_dispatch() copy the 'id' field. That way any qmp client will conform to the specification, including QGA. Furthermore, it simplifies the work for qemu monitor. CC: Michael Roth Signed-off-by: Marc-André Lureau Reviewed-by: Markus Armbruster Signed-off-by: Michael Roth --- monitor.c | 33 ++++++++++++--------------------- qapi/qmp-dispatch.c | 10 ++++++++-- tests/test-qga.c | 13 +++++-------- 3 files changed, 25 insertions(+), 31 deletions(-) diff --git a/monitor.c b/monitor.c index 72061d5bae..4807bbe811 100644 --- a/monitor.c +++ b/monitor.c @@ -250,8 +250,6 @@ QEMUBH *qmp_dispatcher_bh; struct QMPRequest { /* Owner of the request */ Monitor *mon; - /* "id" field of the request */ - QObject *id; /* * Request object to be handled or Error to be reported * (exactly one of them is non-null) @@ -353,7 +351,6 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func, static void qmp_request_free(QMPRequest *req) { - qobject_unref(req->id); qobject_unref(req->req); error_free(req->err); g_free(req); @@ -4108,18 +4105,14 @@ static int monitor_can_read(void *opaque) * Null @rsp can only happen for commands with QCO_NO_SUCCESS_RESP. * Nothing is emitted then. */ -static void monitor_qmp_respond(Monitor *mon, QDict *rsp, QObject *id) +static void monitor_qmp_respond(Monitor *mon, QDict *rsp) { if (rsp) { - if (id) { - qdict_put_obj(rsp, "id", qobject_ref(id)); - } - qmp_send_response(mon, rsp); } } -static void monitor_qmp_dispatch(Monitor *mon, QObject *req, QObject *id) +static void monitor_qmp_dispatch(Monitor *mon, QObject *req) { Monitor *old_mon; QDict *rsp; @@ -4144,7 +4137,7 @@ static void monitor_qmp_dispatch(Monitor *mon, QObject *req, QObject *id) } } - monitor_qmp_respond(mon, rsp, id); + monitor_qmp_respond(mon, rsp); qobject_unref(rsp); } @@ -4208,13 +4201,15 @@ static void monitor_qmp_bh_dispatcher(void *data) mon->qmp.qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1; qemu_mutex_unlock(&mon->qmp.qmp_queue_lock); if (req_obj->req) { - trace_monitor_qmp_cmd_in_band(qobject_get_try_str(req_obj->id) ?: ""); - monitor_qmp_dispatch(mon, req_obj->req, req_obj->id); + QDict *qdict = qobject_to(QDict, req_obj->req); + QObject *id = qdict ? qdict_get(qdict, "id") : NULL; + trace_monitor_qmp_cmd_in_band(qobject_get_try_str(id) ?: ""); + monitor_qmp_dispatch(mon, req_obj->req); } else { assert(req_obj->err); rsp = qmp_error_response(req_obj->err); req_obj->err = NULL; - monitor_qmp_respond(mon, rsp, NULL); + monitor_qmp_respond(mon, rsp); qobject_unref(rsp); } @@ -4239,8 +4234,7 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) qdict = qobject_to(QDict, req); if (qdict) { - id = qobject_ref(qdict_get(qdict, "id")); - qdict_del(qdict, "id"); + id = qdict_get(qdict, "id"); } /* else will fail qmp_dispatch() */ if (req && trace_event_get_state_backends(TRACE_HANDLE_QMP_COMMAND)) { @@ -4251,17 +4245,14 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) if (qdict && qmp_is_oob(qdict)) { /* OOB commands are executed immediately */ - trace_monitor_qmp_cmd_out_of_band(qobject_get_try_str(id) - ?: ""); - monitor_qmp_dispatch(mon, req, id); + trace_monitor_qmp_cmd_out_of_band(qobject_get_try_str(id) ?: ""); + monitor_qmp_dispatch(mon, req); qobject_unref(req); - qobject_unref(id); return; } req_obj = g_new0(QMPRequest, 1); req_obj->mon = mon; - req_obj->id = id; req_obj->req = req; req_obj->err = err; @@ -4281,7 +4272,7 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) /* * Put the request to the end of queue so that requests will be - * handled in time order. Ownership for req_obj, req, id, + * handled in time order. Ownership for req_obj, req, * etc. will be delivered to the handler side. */ assert(mon->qmp.qmp_requests->length < QMP_REQ_QUEUE_LEN_MAX); diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c index 1d922e04f7..5f812bb9f2 100644 --- a/qapi/qmp-dispatch.c +++ b/qapi/qmp-dispatch.c @@ -58,6 +58,8 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, bool allow_oob, "QMP input member 'arguments' must be an object"); return NULL; } + } else if (!strcmp(arg_name, "id")) { + continue; } else { error_setg(errp, "QMP input member '%s' is unexpected", arg_name); @@ -165,11 +167,11 @@ QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request, bool allow_oob) { Error *err = NULL; - QObject *ret; + QDict *dict = qobject_to(QDict, request); + QObject *ret, *id = dict ? qdict_get(dict, "id") : NULL; QDict *rsp; ret = do_qmp_dispatch(cmds, request, allow_oob, &err); - if (err) { rsp = qmp_error_response(err); } else if (ret) { @@ -180,5 +182,9 @@ QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request, rsp = NULL; } + if (rsp && id) { + qdict_put_obj(rsp, "id", qobject_ref(id)); + } + return rsp; } diff --git a/tests/test-qga.c b/tests/test-qga.c index 3d6377436a..891aa3d322 100644 --- a/tests/test-qga.c +++ b/tests/test-qga.c @@ -225,18 +225,15 @@ static void test_qga_ping(gconstpointer fix) qobject_unref(ret); } -static void test_qga_invalid_id(gconstpointer fix) +static void test_qga_id(gconstpointer fix) { const TestFixture *fixture = fix; - QDict *ret, *error; - const char *class; + QDict *ret; ret = qmp_fd(fixture->fd, "{'execute': 'guest-ping', 'id': 1}"); g_assert_nonnull(ret); - - error = qdict_get_qdict(ret, "error"); - class = qdict_get_try_str(error, "class"); - g_assert_cmpstr(class, ==, "GenericError"); + qmp_assert_no_error(ret); + g_assert_cmpint(qdict_get_int(ret, "id"), ==, 1); qobject_unref(ret); } @@ -992,7 +989,7 @@ int main(int argc, char **argv) g_test_add_data_func("/qga/file-ops", &fix, test_qga_file_ops); g_test_add_data_func("/qga/file-write-read", &fix, test_qga_file_write_read); g_test_add_data_func("/qga/get-time", &fix, test_qga_get_time); - g_test_add_data_func("/qga/invalid-id", &fix, test_qga_invalid_id); + g_test_add_data_func("/qga/id", &fix, test_qga_id); g_test_add_data_func("/qga/invalid-oob", &fix, test_qga_invalid_oob); g_test_add_data_func("/qga/invalid-cmd", &fix, test_qga_invalid_cmd); g_test_add_data_func("/qga/invalid-args", &fix, test_qga_invalid_args);