| Message ID | 20200528153742.274164-6-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | monitor: Optionally run handlers in coroutines | expand |
On 5/28/20 10:37 AM, Kevin Wolf wrote: > The correct way to set the current monitor for a coroutine handler is > different that for a blocking handler, so monitor_set_cur() can only be s/that/than/ > called in qmp_dispatch(). > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > include/qapi/qmp/dispatch.h | 3 ++- > monitor/qmp.c | 7 +------ > qapi/qmp-dispatch.c | 8 +++++++- > qga/main.c | 2 +- > stubs/monitor-core.c | 4 ++++ > tests/test-qmp-cmds.c | 6 +++--- > 6 files changed, 18 insertions(+), 12 deletions(-) > > +++ b/stubs/monitor-core.c > @@ -8,6 +8,10 @@ Monitor *monitor_cur(void) > return NULL; > } > > +void monitor_set_cur(Monitor *mon) > +{ > +} > + Should this stub assert(!mon)? Otherwise looks reasonable.
Kevin Wolf <kwolf@redhat.com> writes: > The correct way to set the current monitor for a coroutine handler is > different that for a blocking handler, so monitor_set_cur() can only be will be different > called in qmp_dispatch(). needs to be called in > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > include/qapi/qmp/dispatch.h | 3 ++- > monitor/qmp.c | 7 +------ > qapi/qmp-dispatch.c | 8 +++++++- > qga/main.c | 2 +- > stubs/monitor-core.c | 4 ++++ > tests/test-qmp-cmds.c | 6 +++--- > 6 files changed, 18 insertions(+), 12 deletions(-) > > diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h > index 5a9cf82472..0c2f467028 100644 > --- a/include/qapi/qmp/dispatch.h > +++ b/include/qapi/qmp/dispatch.h > @@ -14,6 +14,7 @@ > #ifndef QAPI_QMP_DISPATCH_H > #define QAPI_QMP_DISPATCH_H > > +#include "monitor/monitor.h" > #include "qemu/queue.h" > > typedef void (QmpCommandFunc)(QDict *, QObject **, Error **); > @@ -49,7 +50,7 @@ const char *qmp_command_name(const QmpCommand *cmd); > bool qmp_has_success_response(const QmpCommand *cmd); > QDict *qmp_error_response(Error *err); > QDict *qmp_dispatch(const QmpCommandList *cmds, QObject *request, > - bool allow_oob); > + bool allow_oob, Monitor *cur_mon); > bool qmp_is_oob(const QDict *dict); > > typedef void (*qmp_cmd_callback_fn)(const QmpCommand *cmd, void *opaque); Tying dispatch to the current monitor when two out of three users don't have a monitor is ugly. Let's not worry about that now. > diff --git a/monitor/qmp.c b/monitor/qmp.c > index a04c512e3a..922fdb5541 100644 > --- a/monitor/qmp.c > +++ b/monitor/qmp.c > @@ -138,12 +138,7 @@ static void monitor_qmp_dispatch(MonitorQMP *mon, QObject *req) > QDict *rsp; > QDict *error; > > - assert(monitor_cur() == NULL); > - monitor_set_cur(&mon->common); > - > - rsp = qmp_dispatch(mon->commands, req, qmp_oob_enabled(mon)); > - > - monitor_set_cur(NULL); > + rsp = qmp_dispatch(mon->commands, req, qmp_oob_enabled(mon), &mon->common); Long line. > > if (mon->commands == &qmp_cap_negotiation_commands) { > error = qdict_get_qdict(rsp, "error"); > diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c > index 79347e0864..2fdbc0fba4 100644 > --- a/qapi/qmp-dispatch.c > +++ b/qapi/qmp-dispatch.c > @@ -89,7 +89,7 @@ bool qmp_is_oob(const QDict *dict) > } > > QDict *qmp_dispatch(const QmpCommandList *cmds, QObject *request, > - bool allow_oob) > + bool allow_oob, Monitor *cur_mon) > { > Error *err = NULL; > bool oob; > @@ -152,7 +152,13 @@ QDict *qmp_dispatch(const QmpCommandList *cmds, QObject *request, > args = qdict_get_qdict(dict, "arguments"); > qobject_ref(args); > } > + > + assert(monitor_cur() == NULL); > + monitor_set_cur(cur_mon); > + > cmd->fn(args, &ret, &err); > + > + monitor_set_cur(NULL); > qobject_unref(args); > if (err) { > /* or assert(!ret) after reviewing all handlers: */ > diff --git a/qga/main.c b/qga/main.c > index f0e454f28d..1042c4e7d3 100644 > --- a/qga/main.c > +++ b/qga/main.c > @@ -574,7 +574,7 @@ static void process_event(void *opaque, QObject *obj, Error *err) > } > > g_debug("processing command"); > - rsp = qmp_dispatch(&ga_commands, obj, false); > + rsp = qmp_dispatch(&ga_commands, obj, false, NULL); > > end: > ret = send_response(s, rsp); > diff --git a/stubs/monitor-core.c b/stubs/monitor-core.c > index 0cd2d864b2..e493df1027 100644 > --- a/stubs/monitor-core.c > +++ b/stubs/monitor-core.c > @@ -8,6 +8,10 @@ Monitor *monitor_cur(void) > return NULL; > } > > +void monitor_set_cur(Monitor *mon) > +{ > +} > + > void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp) > { > } > diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c > index d12ff47e26..5f1b245e19 100644 > --- a/tests/test-qmp-cmds.c > +++ b/tests/test-qmp-cmds.c > @@ -152,7 +152,7 @@ static QObject *do_qmp_dispatch(bool allow_oob, const char *template, ...) > req = qdict_from_vjsonf_nofail(template, ap); > va_end(ap); > > - resp = qmp_dispatch(&qmp_commands, QOBJECT(req), allow_oob); > + resp = qmp_dispatch(&qmp_commands, QOBJECT(req), allow_oob, NULL); > g_assert(resp); > ret = qdict_get(resp, "return"); > g_assert(ret); > @@ -175,7 +175,7 @@ static void do_qmp_dispatch_error(bool allow_oob, ErrorClass cls, > req = qdict_from_vjsonf_nofail(template, ap); > va_end(ap); > > - resp = qmp_dispatch(&qmp_commands, QOBJECT(req), allow_oob); > + resp = qmp_dispatch(&qmp_commands, QOBJECT(req), allow_oob, NULL); > g_assert(resp); > error = qdict_get_qdict(resp, "error"); > g_assert(error); > @@ -231,7 +231,7 @@ static void test_dispatch_cmd_success_response(void) > QDict *resp; > > qdict_put_str(req, "execute", "cmd-success-response"); > - resp = qmp_dispatch(&qmp_commands, QOBJECT(req), false); > + resp = qmp_dispatch(&qmp_commands, QOBJECT(req), false, NULL); > g_assert_null(resp); > qobject_unref(req); > } This works because qmp_dispatch() doesn't bother to check its @mon argument is sane, and the test's commands don't use monitor_cur().
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h index 5a9cf82472..0c2f467028 100644 --- a/include/qapi/qmp/dispatch.h +++ b/include/qapi/qmp/dispatch.h @@ -14,6 +14,7 @@ #ifndef QAPI_QMP_DISPATCH_H #define QAPI_QMP_DISPATCH_H +#include "monitor/monitor.h" #include "qemu/queue.h" typedef void (QmpCommandFunc)(QDict *, QObject **, Error **); @@ -49,7 +50,7 @@ const char *qmp_command_name(const QmpCommand *cmd); bool qmp_has_success_response(const QmpCommand *cmd); QDict *qmp_error_response(Error *err); QDict *qmp_dispatch(const QmpCommandList *cmds, QObject *request, - bool allow_oob); + bool allow_oob, Monitor *cur_mon); bool qmp_is_oob(const QDict *dict); typedef void (*qmp_cmd_callback_fn)(const QmpCommand *cmd, void *opaque); diff --git a/monitor/qmp.c b/monitor/qmp.c index a04c512e3a..922fdb5541 100644 --- a/monitor/qmp.c +++ b/monitor/qmp.c @@ -138,12 +138,7 @@ static void monitor_qmp_dispatch(MonitorQMP *mon, QObject *req) QDict *rsp; QDict *error; - assert(monitor_cur() == NULL); - monitor_set_cur(&mon->common); - - rsp = qmp_dispatch(mon->commands, req, qmp_oob_enabled(mon)); - - monitor_set_cur(NULL); + rsp = qmp_dispatch(mon->commands, req, qmp_oob_enabled(mon), &mon->common); if (mon->commands == &qmp_cap_negotiation_commands) { error = qdict_get_qdict(rsp, "error"); diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c index 79347e0864..2fdbc0fba4 100644 --- a/qapi/qmp-dispatch.c +++ b/qapi/qmp-dispatch.c @@ -89,7 +89,7 @@ bool qmp_is_oob(const QDict *dict) } QDict *qmp_dispatch(const QmpCommandList *cmds, QObject *request, - bool allow_oob) + bool allow_oob, Monitor *cur_mon) { Error *err = NULL; bool oob; @@ -152,7 +152,13 @@ QDict *qmp_dispatch(const QmpCommandList *cmds, QObject *request, args = qdict_get_qdict(dict, "arguments"); qobject_ref(args); } + + assert(monitor_cur() == NULL); + monitor_set_cur(cur_mon); + cmd->fn(args, &ret, &err); + + monitor_set_cur(NULL); qobject_unref(args); if (err) { /* or assert(!ret) after reviewing all handlers: */ diff --git a/qga/main.c b/qga/main.c index f0e454f28d..1042c4e7d3 100644 --- a/qga/main.c +++ b/qga/main.c @@ -574,7 +574,7 @@ static void process_event(void *opaque, QObject *obj, Error *err) } g_debug("processing command"); - rsp = qmp_dispatch(&ga_commands, obj, false); + rsp = qmp_dispatch(&ga_commands, obj, false, NULL); end: ret = send_response(s, rsp); diff --git a/stubs/monitor-core.c b/stubs/monitor-core.c index 0cd2d864b2..e493df1027 100644 --- a/stubs/monitor-core.c +++ b/stubs/monitor-core.c @@ -8,6 +8,10 @@ Monitor *monitor_cur(void) return NULL; } +void monitor_set_cur(Monitor *mon) +{ +} + void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp) { } diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c index d12ff47e26..5f1b245e19 100644 --- a/tests/test-qmp-cmds.c +++ b/tests/test-qmp-cmds.c @@ -152,7 +152,7 @@ static QObject *do_qmp_dispatch(bool allow_oob, const char *template, ...) req = qdict_from_vjsonf_nofail(template, ap); va_end(ap); - resp = qmp_dispatch(&qmp_commands, QOBJECT(req), allow_oob); + resp = qmp_dispatch(&qmp_commands, QOBJECT(req), allow_oob, NULL); g_assert(resp); ret = qdict_get(resp, "return"); g_assert(ret); @@ -175,7 +175,7 @@ static void do_qmp_dispatch_error(bool allow_oob, ErrorClass cls, req = qdict_from_vjsonf_nofail(template, ap); va_end(ap); - resp = qmp_dispatch(&qmp_commands, QOBJECT(req), allow_oob); + resp = qmp_dispatch(&qmp_commands, QOBJECT(req), allow_oob, NULL); g_assert(resp); error = qdict_get_qdict(resp, "error"); g_assert(error); @@ -231,7 +231,7 @@ static void test_dispatch_cmd_success_response(void) QDict *resp; qdict_put_str(req, "execute", "cmd-success-response"); - resp = qmp_dispatch(&qmp_commands, QOBJECT(req), false); + resp = qmp_dispatch(&qmp_commands, QOBJECT(req), false, NULL); g_assert_null(resp); qobject_unref(req); }
The correct way to set the current monitor for a coroutine handler is different that for a blocking handler, so monitor_set_cur() can only be called in qmp_dispatch(). Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- include/qapi/qmp/dispatch.h | 3 ++- monitor/qmp.c | 7 +------ qapi/qmp-dispatch.c | 8 +++++++- qga/main.c | 2 +- stubs/monitor-core.c | 4 ++++ tests/test-qmp-cmds.c | 6 +++--- 6 files changed, 18 insertions(+), 12 deletions(-)