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.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
monitor.c | 31 ++++++++++++-------------------
qapi/qmp-dispatch.c | 10 ++++++++--
tests/test-qga.c | 13 +++++--------
3 files changed, 25 insertions(+), 29 deletions(-)
diff --git a/monitor.c b/monitor.c
index a3efe96d1d..bf192697e4 100644
--- a/monitor.c
+++ b/monitor.c
@@ -249,8 +249,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)
@@ -351,7 +349,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);
@@ -3991,18 +3988,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;
@@ -4027,7 +4020,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);
}
@@ -4081,12 +4074,15 @@ static void monitor_qmp_bh_dispatcher(void *data)
need_resume = !qmp_oob_enabled(req_obj->mon);
if (req_obj->req) {
- trace_monitor_qmp_cmd_in_band(qobject_get_try_str(req_obj->id) ?: "");
- monitor_qmp_dispatch(req_obj->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(req_obj->mon, req_obj->req);
} else {
assert(req_obj->err);
rsp = qmp_error_response(req_obj->err);
- monitor_qmp_respond(req_obj->mon, rsp, NULL);
+ req_obj->err = NULL;
+ monitor_qmp_respond(req_obj->mon, rsp);
qobject_unref(rsp);
}
@@ -4115,8 +4111,7 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
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 (trace_event_get_state_backends(TRACE_HANDLE_QMP_COMMAND)) {
@@ -4127,15 +4122,13 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
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);
return;
}
req_obj = g_new0(QMPRequest, 1);
req_obj->mon = mon;
- req_obj->id = id;
req_obj->req = req;
req_obj->err = err;
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 90ba5e3074..acea0fcfcd 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -59,6 +59,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);
@@ -166,8 +168,8 @@ QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request,
bool allow_oob)
{
Error *err = NULL;
- QObject *ret;
- QDict *rsp;
+ QDict *rsp, *dict = qobject_to(QDict, request);
+ QObject *ret, *id = dict ? qdict_get(dict, "id") : NULL;
ret = do_qmp_dispatch(cmds, request, allow_oob, &err);
@@ -181,5 +183,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 d638b1571a..4ee8b405f4 100644
--- a/tests/test-qga.c
+++ b/tests/test-qga.c
@@ -227,18 +227,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);
}
@@ -1014,7 +1011,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);
--
2.18.0.rc1
Copying the guest agent maintainer Michael Roth.
Patch needs a rebase.
Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> Let qmp_dispatch() copy the 'id' field. That way any qmp client will
> conform to the specification, including QGA.
Before this patch, users of the common core shared by QMP and QGA have
to do extra work to conform to qmp-spec.txt. QMP does, QGA doesn't.
I'm inclined to consider that a bug.
Judging from your description, this patch moves that work into the
shared part. The patch is therefore not just a rework of 'id' handling,
it's a QGA bug fix, if you're so inclined, else a QGA improvement.
Thus, 1. please rephrase the commit message accordingly, and 2. the
patch needs Michael's approval.
> Furthermore, it
> simplifies the work for qemu monitor.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> monitor.c | 31 ++++++++++++-------------------
> qapi/qmp-dispatch.c | 10 ++++++++--
> tests/test-qga.c | 13 +++++--------
> 3 files changed, 25 insertions(+), 29 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index a3efe96d1d..bf192697e4 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -249,8 +249,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)
> @@ -351,7 +349,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);
> @@ -3991,18 +3988,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;
> @@ -4027,7 +4020,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);
> }
>
> @@ -4081,12 +4074,15 @@ static void monitor_qmp_bh_dispatcher(void *data)
>
> need_resume = !qmp_oob_enabled(req_obj->mon);
> if (req_obj->req) {
> - trace_monitor_qmp_cmd_in_band(qobject_get_try_str(req_obj->id) ?: "");
> - monitor_qmp_dispatch(req_obj->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(req_obj->mon, req_obj->req);
> } else {
> assert(req_obj->err);
> rsp = qmp_error_response(req_obj->err);
> - monitor_qmp_respond(req_obj->mon, rsp, NULL);
> + req_obj->err = NULL;
> + monitor_qmp_respond(req_obj->mon, rsp);
Error response without ID before and after the patch. Okay.
> qobject_unref(rsp);
> }
>
> @@ -4115,8 +4111,7 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
>
> 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() */
Two users of @id remain: trace_monitor_qmp_cmd_out_of_band(), visible
below, and qapi_event_send_command_dropped(). We currently plan to kill
the latter. The former is guarded by if (qdict && qmp_is_oob(qdict)),
and could therefore safely use qdict_get(qdict, "id"). Together, this
will let us delete the whole conditional here.
>
> if (trace_event_get_state_backends(TRACE_HANDLE_QMP_COMMAND)) {
> @@ -4127,15 +4122,13 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
>
> 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);
> return;
> }
>
> req_obj = g_new0(QMPRequest, 1);
> req_obj->mon = mon;
> - req_obj->id = id;
> req_obj->req = req;
> req_obj->err = err;
>
> diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
> index 90ba5e3074..acea0fcfcd 100644
> --- a/qapi/qmp-dispatch.c
> +++ b/qapi/qmp-dispatch.c
> @@ -59,6 +59,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);
> @@ -166,8 +168,8 @@ QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request,
> bool allow_oob)
> {
> Error *err = NULL;
> - QObject *ret;
> - QDict *rsp;
> + QDict *rsp, *dict = qobject_to(QDict, request);
> + QObject *ret, *id = dict ? qdict_get(dict, "id") : NULL;
I dislike mixing initialized and uninitialized variables in the same
declaration.
>
> ret = do_qmp_dispatch(cmds, request, allow_oob, &err);
>
> @@ -181,5 +183,9 @@ QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request,
> rsp = NULL;
> }
>
> + if (rsp && id) {
> + qdict_put_obj(rsp, "id", qobject_ref(id));
> + }
> +
> return rsp;
> }
This puts the ID (if any) into the response. Good.
The monitor sends command responses only with monitor_qmp_respond().
It's called on two places:
* monitor_qmp_dispatch()
Gets its response from qmp_dispatch(), which now puts the ID (if any).
* monitor_qmp_bh_dispatcher()
Error response without ID before and after the patch (see above).
Good.
Not visible in the patch: QGA. It sends command responses only with
send_response(). Called only in process_event(). Two cases:
* json_parser_parse_err() sets an error
Error response without ID before and after the patch
* qmp_dispatch()
Now puts the ID (if any). This is the externally visible change.
Good.
> diff --git a/tests/test-qga.c b/tests/test-qga.c
> index d638b1571a..4ee8b405f4 100644
> --- a/tests/test-qga.c
> +++ b/tests/test-qga.c
> @@ -227,18 +227,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);
> }
> @@ -1014,7 +1011,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);
Hi
On Tue, Jul 17, 2018 at 6:05 PM, Markus Armbruster <armbru@redhat.com> wrote:
> Copying the guest agent maintainer Michael Roth.
>
> Patch needs a rebase.
>
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>
>> Let qmp_dispatch() copy the 'id' field. That way any qmp client will
>> conform to the specification, including QGA.
>
> Before this patch, users of the common core shared by QMP and QGA have
> to do extra work to conform to qmp-spec.txt. QMP does, QGA doesn't.
> I'm inclined to consider that a bug.
>
> Judging from your description, this patch moves that work into the
> shared part. The patch is therefore not just a rework of 'id' handling,
> it's a QGA bug fix, if you're so inclined, else a QGA improvement.
>
> Thus, 1. please rephrase the commit message accordingly, and 2. the
> patch needs Michael's approval.
>
ok
>> Furthermore, it
>> simplifies the work for qemu monitor.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>> monitor.c | 31 ++++++++++++-------------------
>> qapi/qmp-dispatch.c | 10 ++++++++--
>> tests/test-qga.c | 13 +++++--------
>> 3 files changed, 25 insertions(+), 29 deletions(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index a3efe96d1d..bf192697e4 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -249,8 +249,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)
>> @@ -351,7 +349,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);
>> @@ -3991,18 +3988,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;
>> @@ -4027,7 +4020,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);
>> }
>>
>> @@ -4081,12 +4074,15 @@ static void monitor_qmp_bh_dispatcher(void *data)
>>
>> need_resume = !qmp_oob_enabled(req_obj->mon);
>> if (req_obj->req) {
>> - trace_monitor_qmp_cmd_in_band(qobject_get_try_str(req_obj->id) ?: "");
>> - monitor_qmp_dispatch(req_obj->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(req_obj->mon, req_obj->req);
>> } else {
>> assert(req_obj->err);
>> rsp = qmp_error_response(req_obj->err);
>> - monitor_qmp_respond(req_obj->mon, rsp, NULL);
>> + req_obj->err = NULL;
>> + monitor_qmp_respond(req_obj->mon, rsp);
>
> Error response without ID before and after the patch. Okay.
>
>> qobject_unref(rsp);
>> }
>>
>> @@ -4115,8 +4111,7 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
>>
>> 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() */
>
> Two users of @id remain: trace_monitor_qmp_cmd_out_of_band(), visible
> below, and qapi_event_send_command_dropped(). We currently plan to kill
> the latter. The former is guarded by if (qdict && qmp_is_oob(qdict)),
> and could therefore safely use qdict_get(qdict, "id"). Together, this
> will let us delete the whole conditional here.
>
>>
>> if (trace_event_get_state_backends(TRACE_HANDLE_QMP_COMMAND)) {
>> @@ -4127,15 +4122,13 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
>>
>> 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);
>> return;
>> }
>>
>> req_obj = g_new0(QMPRequest, 1);
>> req_obj->mon = mon;
>> - req_obj->id = id;
>> req_obj->req = req;
>> req_obj->err = err;
>>
>> diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
>> index 90ba5e3074..acea0fcfcd 100644
>> --- a/qapi/qmp-dispatch.c
>> +++ b/qapi/qmp-dispatch.c
>> @@ -59,6 +59,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);
>> @@ -166,8 +168,8 @@ QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request,
>> bool allow_oob)
>> {
>> Error *err = NULL;
>> - QObject *ret;
>> - QDict *rsp;
>> + QDict *rsp, *dict = qobject_to(QDict, request);
>> + QObject *ret, *id = dict ? qdict_get(dict, "id") : NULL;
>
> I dislike mixing initialized and uninitialized variables in the same
> declaration.
ok
>
>>
>> ret = do_qmp_dispatch(cmds, request, allow_oob, &err);
>>
>> @@ -181,5 +183,9 @@ QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request,
>> rsp = NULL;
>> }
>>
>> + if (rsp && id) {
>> + qdict_put_obj(rsp, "id", qobject_ref(id));
>> + }
>> +
>> return rsp;
>> }
>
> This puts the ID (if any) into the response. Good.
>
> The monitor sends command responses only with monitor_qmp_respond().
> It's called on two places:
>
> * monitor_qmp_dispatch()
>
> Gets its response from qmp_dispatch(), which now puts the ID (if any).
>
> * monitor_qmp_bh_dispatcher()
>
> Error response without ID before and after the patch (see above).
>
> Good.
>
> Not visible in the patch: QGA. It sends command responses only with
> send_response(). Called only in process_event(). Two cases:
>
> * json_parser_parse_err() sets an error
>
> Error response without ID before and after the patch
It should be the same as qemu monitor (no qdict returned by json_parser_parse())
>
> * qmp_dispatch()
>
> Now puts the ID (if any). This is the externally visible change.
>
> Good.
>
>> diff --git a/tests/test-qga.c b/tests/test-qga.c
>> index d638b1571a..4ee8b405f4 100644
>> --- a/tests/test-qga.c
>> +++ b/tests/test-qga.c
>> @@ -227,18 +227,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);
>> }
>> @@ -1014,7 +1011,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);
>
--
Marc-André Lureau
© 2016 - 2025 Red Hat, Inc.