diff options
| -rw-r--r-- | chardev/char.c | 11 | ||||
| -rw-r--r-- | docs/interop/qmp-spec.txt | 5 | ||||
| -rw-r--r-- | include/chardev/char.h | 3 | ||||
| -rw-r--r-- | include/monitor/monitor.h | 3 | ||||
| -rw-r--r-- | monitor.c | 139 | ||||
| -rw-r--r-- | net/colo-compare.c | 6 | ||||
| -rw-r--r-- | qapi/misc.json | 40 | ||||
| -rw-r--r-- | tests/libqtest.c | 9 | ||||
| -rw-r--r-- | tests/libqtest.h | 4 | ||||
| -rw-r--r-- | tests/qmp-test.c | 6 | ||||
| -rw-r--r-- | tests/test-qmp-cmds.c | 16 | ||||
| -rw-r--r-- | vl.c | 5 |
12 files changed, 118 insertions, 129 deletions
diff --git a/chardev/char.c b/chardev/char.c index 152dde5327..ccba36bafb 100644 --- a/chardev/char.c +++ b/chardev/char.c @@ -193,6 +193,8 @@ void qemu_chr_be_update_read_handlers(Chardev *s, { ChardevClass *cc = CHARDEV_GET_CLASS(s); + assert(qemu_chr_has_feature(s, QEMU_CHAR_FEATURE_GCONTEXT) + || !context); s->gcontext = context; if (cc->chr_update_read_handler) { cc->chr_update_read_handler(s); @@ -240,6 +242,15 @@ static void char_init(Object *obj) chr->logfd = -1; qemu_mutex_init(&chr->chr_write_lock); + + /* + * Assume if chr_update_read_handler is implemented it will + * take the updated gcontext into account. + */ + if (CHARDEV_GET_CLASS(chr)->chr_update_read_handler) { + qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT); + } + } static int null_chr_write(Chardev *chr, const uint8_t *buf, int len) diff --git a/docs/interop/qmp-spec.txt b/docs/interop/qmp-spec.txt index 8f7da0245d..67e44a8120 100644 --- a/docs/interop/qmp-spec.txt +++ b/docs/interop/qmp-spec.txt @@ -130,8 +130,9 @@ to pass "id" with out-of-band commands. Passing it with all commands is recommended for clients that accept capability "oob". If the client sends in-band commands faster than the server can -execute them, the server will eventually drop commands to limit the -queue length. The sever sends event COMMAND_DROPPED then. +execute them, the server will stop reading the requests from the QMP +channel until the request queue length is reduced to an acceptable +range. Only a few commands support out-of-band execution. The ones that do have "allow-oob": true in output of query-qmp-schema. diff --git a/include/chardev/char.h b/include/chardev/char.h index 7becd8c80c..014566c3de 100644 --- a/include/chardev/char.h +++ b/include/chardev/char.h @@ -47,6 +47,9 @@ typedef enum { QEMU_CHAR_FEATURE_FD_PASS, /* Whether replay or record mode is enabled */ QEMU_CHAR_FEATURE_REPLAY, + /* Whether the gcontext can be changed after calling + * qemu_chr_be_update_read_handlers() */ + QEMU_CHAR_FEATURE_GCONTEXT, QEMU_CHAR_FEATURE_LAST, } ChardevFeature; diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h index 6fd2c53b09..c1b40a9cac 100644 --- a/include/monitor/monitor.h +++ b/include/monitor/monitor.h @@ -13,7 +13,8 @@ extern __thread Monitor *cur_mon; #define MONITOR_USE_READLINE 0x02 #define MONITOR_USE_CONTROL 0x04 #define MONITOR_USE_PRETTY 0x08 -#define MONITOR_USE_OOB 0x10 + +#define QMP_REQ_QUEUE_LEN_MAX 8 bool monitor_cur_is_qmp(void); diff --git a/monitor.c b/monitor.c index d39390c2f2..6e81b09294 100644 --- a/monitor.c +++ b/monitor.c @@ -263,10 +263,11 @@ typedef struct QMPRequest QMPRequest; /* QMP checker flags */ #define QMP_ACCEPT_UNKNOWNS 1 -/* Protects mon_list, monitor_qapi_event_state. */ +/* Protects mon_list, monitor_qapi_event_state, monitor_destroyed. */ static QemuMutex monitor_lock; static GHashTable *monitor_qapi_event_state; static QTAILQ_HEAD(mon_list, Monitor) mon_list; +static bool monitor_destroyed; /* Protects mon_fdsets */ static QemuMutex mon_fdsets_lock; @@ -4109,8 +4110,12 @@ static void monitor_qmp_dispatch(Monitor *mon, QObject *req, QObject *id) * processing commands only on a very busy monitor. To achieve that, * when we process one request on a specific monitor, we put that * monitor to the end of mon_list queue. + * + * Note: if the function returned with non-NULL, then the caller will + * be with mon->qmp.qmp_queue_lock held, and the caller is responsible + * to release it. */ -static QMPRequest *monitor_qmp_requests_pop_any(void) +static QMPRequest *monitor_qmp_requests_pop_any_with_lock(void) { QMPRequest *req_obj = NULL; Monitor *mon; @@ -4120,10 +4125,11 @@ static QMPRequest *monitor_qmp_requests_pop_any(void) QTAILQ_FOREACH(mon, &mon_list, entry) { qemu_mutex_lock(&mon->qmp.qmp_queue_lock); req_obj = g_queue_pop_head(mon->qmp.qmp_requests); - qemu_mutex_unlock(&mon->qmp.qmp_queue_lock); if (req_obj) { + /* With the lock of corresponding queue held */ break; } + qemu_mutex_unlock(&mon->qmp.qmp_queue_lock); } if (req_obj) { @@ -4142,30 +4148,34 @@ static QMPRequest *monitor_qmp_requests_pop_any(void) static void monitor_qmp_bh_dispatcher(void *data) { - QMPRequest *req_obj = monitor_qmp_requests_pop_any(); + QMPRequest *req_obj = monitor_qmp_requests_pop_any_with_lock(); QDict *rsp; bool need_resume; + Monitor *mon; if (!req_obj) { return; } + mon = req_obj->mon; /* qmp_oob_enabled() might change after "qmp_capabilities" */ - need_resume = !qmp_oob_enabled(req_obj->mon); + need_resume = !qmp_oob_enabled(mon) || + 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(req_obj->mon, req_obj->req, req_obj->id); + monitor_qmp_dispatch(mon, req_obj->req, req_obj->id); } else { assert(req_obj->err); rsp = qmp_error_response(req_obj->err); req_obj->err = NULL; - monitor_qmp_respond(req_obj->mon, rsp, NULL); + monitor_qmp_respond(mon, rsp, NULL); qobject_unref(rsp); } if (need_resume) { /* Pairs with the monitor_suspend() in handle_qmp_command() */ - monitor_resume(req_obj->mon); + monitor_resume(mon); } qmp_request_free(req_obj); @@ -4173,8 +4183,6 @@ static void monitor_qmp_bh_dispatcher(void *data) qemu_bh_schedule(qmp_dispatcher_bh); } -#define QMP_REQ_QUEUE_LEN_MAX (8) - static void handle_qmp_command(void *opaque, QObject *req, Error *err) { Monitor *mon = opaque; @@ -4216,28 +4224,14 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) qemu_mutex_lock(&mon->qmp.qmp_queue_lock); /* - * If OOB is not enabled on the current monitor, we'll emulate the - * old behavior that we won't process the current monitor any more - * until it has responded. This helps make sure that as long as - * OOB is not enabled, the server will never drop any command. + * Suspend the monitor when we can't queue more requests after + * this one. Dequeuing in monitor_qmp_bh_dispatcher() will resume + * it. Note that when OOB is disabled, we queue at most one + * command, for backward compatibility. */ - if (!qmp_oob_enabled(mon)) { + if (!qmp_oob_enabled(mon) || + mon->qmp.qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1) { monitor_suspend(mon); - } else { - /* Drop the request if queue is full. */ - if (mon->qmp.qmp_requests->length >= QMP_REQ_QUEUE_LEN_MAX) { - qemu_mutex_unlock(&mon->qmp.qmp_queue_lock); - /* - * FIXME @id's scope is just @mon, and broadcasting it is - * wrong. If another monitor's client has a command with - * the same ID in flight, the event will incorrectly claim - * that command was dropped. - */ - qapi_event_send_command_dropped(id, - COMMAND_DROP_REASON_QUEUE_FULL); - qmp_request_free(req_obj); - return; - } } /* @@ -4245,6 +4239,7 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) * handled in time order. Ownership for req_obj, req, id, * etc. will be delivered to the handler side. */ + assert(mon->qmp.qmp_requests->length < QMP_REQ_QUEUE_LEN_MAX); g_queue_push_tail(mon->qmp.qmp_requests, req_obj); qemu_mutex_unlock(&mon->qmp.qmp_queue_lock); @@ -4297,7 +4292,7 @@ int monitor_suspend(Monitor *mon) atomic_inc(&mon->suspend_cnt); - if (monitor_is_qmp(mon) && mon->use_io_thread) { + if (mon->use_io_thread) { /* * Kick I/O thread to make sure this takes effect. It'll be * evaluated again in prepare() of the watch object. @@ -4309,6 +4304,13 @@ int monitor_suspend(Monitor *mon) return 0; } +static void monitor_accept_input(void *opaque) +{ + Monitor *mon = opaque; + + qemu_chr_fe_accept_input(&mon->chr); +} + void monitor_resume(Monitor *mon) { if (monitor_is_hmp_non_interactive(mon)) { @@ -4316,20 +4318,22 @@ void monitor_resume(Monitor *mon) } if (atomic_dec_fetch(&mon->suspend_cnt) == 0) { - if (monitor_is_qmp(mon)) { - /* - * For QMP monitors that are running in the I/O thread, - * let's kick the thread in case it's sleeping. - */ - if (mon->use_io_thread) { - aio_notify(iothread_get_aio_context(mon_iothread)); - } + AioContext *ctx; + + if (mon->use_io_thread) { + ctx = iothread_get_aio_context(mon_iothread); } else { + ctx = qemu_get_aio_context(); + } + + if (!monitor_is_qmp(mon)) { assert(mon->rs); readline_show_prompt(mon->rs); } - qemu_chr_fe_accept_input(&mon->chr); + + aio_bh_schedule_oneshot(ctx, monitor_accept_input, mon); } + trace_monitor_suspend(mon, -1); } @@ -4453,16 +4457,6 @@ static void sortcmdlist(void) qsort((void *)info_cmds, array_num, elem_size, compare_mon_cmd); } -static GMainContext *monitor_get_io_context(void) -{ - return iothread_get_g_main_context(mon_iothread); -} - -static AioContext *monitor_get_aio_context(void) -{ - return iothread_get_aio_context(mon_iothread); -} - static void monitor_iothread_init(void) { mon_iothread = iothread_create("mon_iothread", &error_abort); @@ -4539,8 +4533,21 @@ void error_vprintf_unless_qmp(const char *fmt, va_list ap) static void monitor_list_append(Monitor *mon) { qemu_mutex_lock(&monitor_lock); - QTAILQ_INSERT_HEAD(&mon_list, mon, entry); + /* + * This prevents inserting new monitors during monitor_cleanup(). + * A cleaner solution would involve the main thread telling other + * threads to terminate, waiting for their termination. + */ + if (!monitor_destroyed) { + QTAILQ_INSERT_HEAD(&mon_list, mon, entry); + mon = NULL; + } qemu_mutex_unlock(&monitor_lock); + + if (mon) { + monitor_data_destroy(mon); + g_free(mon); + } } static void monitor_qmp_setup_handlers_bh(void *opaque) @@ -4549,7 +4556,7 @@ static void monitor_qmp_setup_handlers_bh(void *opaque) GMainContext *context; assert(mon->use_io_thread); - context = monitor_get_io_context(); + context = iothread_get_g_main_context(mon_iothread); assert(context); qemu_chr_fe_set_handlers(&mon->chr, monitor_can_read, monitor_qmp_read, monitor_qmp_event, NULL, mon, context, true); @@ -4560,21 +4567,12 @@ void monitor_init(Chardev *chr, int flags) { Monitor *mon = g_malloc(sizeof(*mon)); bool use_readline = flags & MONITOR_USE_READLINE; - bool use_oob = flags & MONITOR_USE_OOB; - if (use_oob) { - if (CHARDEV_IS_MUX(chr)) { - error_report("Monitor out-of-band is not supported with " - "MUX typed chardev backend"); - exit(1); - } - if (use_readline) { - error_report("Monitor out-of-band is only supported by QMP"); - exit(1); - } - } - - monitor_data_init(mon, false, use_oob); + /* Note: we run QMP monitor in I/O thread when @chr supports that */ + monitor_data_init(mon, false, + (flags & MONITOR_USE_CONTROL) + && qemu_chr_has_feature(chr, + QEMU_CHAR_FEATURE_GCONTEXT)); qemu_chr_fe_init(&mon->chr, chr, &error_abort); mon->flags = flags; @@ -4601,7 +4599,7 @@ void monitor_init(Chardev *chr, int flags) * since chardev might be running in the monitor I/O * thread. Schedule a bottom half. */ - aio_bh_schedule_oneshot(monitor_get_aio_context(), + aio_bh_schedule_oneshot(iothread_get_aio_context(mon_iothread), monitor_qmp_setup_handlers_bh, mon); /* The bottom half will add @mon to @mon_list */ return; @@ -4634,10 +4632,14 @@ void monitor_cleanup(void) /* Flush output buffers and destroy monitors */ qemu_mutex_lock(&monitor_lock); + monitor_destroyed = true; QTAILQ_FOREACH_SAFE(mon, &mon_list, entry, next) { QTAILQ_REMOVE(&mon_list, mon, entry); + /* Permit QAPI event emission from character frontend release */ + qemu_mutex_unlock(&monitor_lock); monitor_flush(mon); monitor_data_destroy(mon); + qemu_mutex_lock(&monitor_lock); g_free(mon); } qemu_mutex_unlock(&monitor_lock); @@ -4665,9 +4667,6 @@ QemuOptsList qemu_mon_opts = { },{ .name = "pretty", .type = QEMU_OPT_BOOL, - },{ - .name = "x-oob", - .type = QEMU_OPT_BOOL, }, { /* end of list */ } }, diff --git a/net/colo-compare.c b/net/colo-compare.c index a39191d522..9156ab3349 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -957,6 +957,12 @@ static int find_and_check_chardev(Chardev **chr, return 1; } + if (!qemu_chr_has_feature(*chr, QEMU_CHAR_FEATURE_GCONTEXT)) { + error_setg(errp, "chardev \"%s\" cannot switch context", + chr_name); + return 1; + } + return 0; } diff --git a/qapi/misc.json b/qapi/misc.json index 45121492dd..4211a732f3 100644 --- a/qapi/misc.json +++ b/qapi/misc.json @@ -3445,46 +3445,6 @@ { 'command': 'query-sev-capabilities', 'returns': 'SevCapability' } ## -# @CommandDropReason: -# -# Reasons that caused one command to be dropped. -# -# @queue-full: the command queue is full. This can only occur when -# the client sends a new non-oob command before the -# response to the previous non-oob command has been -# received. -# -# Since: 2.12 -## -{ 'enum': 'CommandDropReason', - 'data': [ 'queue-full' ] } - -## -# @COMMAND_DROPPED: -# -# Emitted when a command is dropped due to some reason. Commands can -# only be dropped when the oob capability is enabled. -# -# @id: The dropped command's "id" field. -# FIXME Broken by design. Events are broadcast to all monitors. If -# another monitor's client has a command with the same ID in flight, -# the event will incorrectly claim that command was dropped. -# -# @reason: The reason why the command is dropped. -# -# Since: 2.12 -# -# Example: -# -# { "event": "COMMAND_DROPPED", -# "data": {"result": {"id": "libvirt-102", -# "reason": "queue-full" } } } -# -## -{ 'event': 'COMMAND_DROPPED' , - 'data': { 'id': 'any', 'reason': 'CommandDropReason' } } - -## # @set-numa-node: # # Runtime equivalent of '-numa' CLI option, available at diff --git a/tests/libqtest.c b/tests/libqtest.c index 75e07e16e7..43be078518 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -187,8 +187,7 @@ static const char *qtest_qemu_binary(void) return qemu_bin; } -QTestState *qtest_init_without_qmp_handshake(bool use_oob, - const char *extra_args) +QTestState *qtest_init_without_qmp_handshake(const char *extra_args) { QTestState *s; int sock, qmpsock, i; @@ -219,12 +218,12 @@ QTestState *qtest_init_without_qmp_handshake(bool use_oob, "-qtest unix:%s,nowait " "-qtest-log %s " "-chardev socket,path=%s,nowait,id=char0 " - "-mon chardev=char0,mode=control%s " + "-mon chardev=char0,mode=control " "-machine accel=qtest " "-display none " "%s", qemu_binary, socket_path, getenv("QTEST_LOG") ? "/dev/fd/2" : "/dev/null", - qmp_socket_path, use_oob ? ",x-oob=on" : "", + qmp_socket_path, extra_args ?: ""); g_test_message("starting QEMU: %s", command); @@ -266,7 +265,7 @@ QTestState *qtest_init_without_qmp_handshake(bool use_oob, QTestState *qtest_init(const char *extra_args) { - QTestState *s = qtest_init_without_qmp_handshake(false, extra_args); + QTestState *s = qtest_init_without_qmp_handshake(extra_args); QDict *greeting; /* Read the QMP greeting and then do the handshake */ diff --git a/tests/libqtest.h b/tests/libqtest.h index ed88ff99d5..96a6c01352 100644 --- a/tests/libqtest.h +++ b/tests/libqtest.h @@ -55,14 +55,12 @@ QTestState *qtest_init(const char *extra_args); /** * qtest_init_without_qmp_handshake: - * @use_oob: true to have the server advertise OOB support * @extra_args: other arguments to pass to QEMU. CAUTION: these * arguments are subject to word splitting and shell evaluation. * * Returns: #QTestState instance. */ -QTestState *qtest_init_without_qmp_handshake(bool use_oob, - const char *extra_args); +QTestState *qtest_init_without_qmp_handshake(const char *extra_args); /** * qtest_quit: diff --git a/tests/qmp-test.c b/tests/qmp-test.c index 7517be4654..48a4fa791a 100644 --- a/tests/qmp-test.c +++ b/tests/qmp-test.c @@ -108,7 +108,7 @@ static void test_qmp_protocol(void) QList *capabilities; QTestState *qts; - qts = qtest_init_without_qmp_handshake(false, common_args); + qts = qtest_init_without_qmp_handshake(common_args); /* Test greeting */ resp = qtest_qmp_receive(qts); @@ -116,7 +116,7 @@ static void test_qmp_protocol(void) g_assert(q); test_version(qdict_get(q, "version")); capabilities = qdict_get_qlist(q, "capabilities"); - g_assert(capabilities && qlist_empty(capabilities)); + g_assert(capabilities); qobject_unref(resp); /* Test valid command before handshake */ @@ -219,7 +219,7 @@ static void test_qmp_oob(void) QList *capabilities; QString *qstr; - qts = qtest_init_without_qmp_handshake(true, common_args); + qts = qtest_init_without_qmp_handshake(common_args); /* Check the greeting message. */ resp = qtest_qmp_receive(qts); diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c index 4ab2b6e5ce..481cb069ca 100644 --- a/tests/test-qmp-cmds.c +++ b/tests/test-qmp-cmds.c @@ -126,6 +126,21 @@ static void test_dispatch_cmd(void) qobject_unref(req); } +static void test_dispatch_cmd_oob(void) +{ + QDict *req = qdict_new(); + QDict *resp; + + qdict_put_str(req, "exec-oob", "test-flags-command"); + + resp = qmp_dispatch(&qmp_commands, QOBJECT(req), true); + assert(resp != NULL); + assert(!qdict_haskey(resp, "error")); + + qobject_unref(resp); + qobject_unref(req); +} + /* test commands that return an error due to invalid parameters */ static void test_dispatch_cmd_failure(void) { @@ -302,6 +317,7 @@ int main(int argc, char **argv) g_test_init(&argc, &argv, NULL); g_test_add_func("/qmp/dispatch_cmd", test_dispatch_cmd); + g_test_add_func("/qmp/dispatch_cmd_oob", test_dispatch_cmd_oob); g_test_add_func("/qmp/dispatch_cmd_failure", test_dispatch_cmd_failure); g_test_add_func("/qmp/dispatch_cmd_io", test_dispatch_cmd_io); g_test_add_func("/qmp/dispatch_cmd_success_response", diff --git a/vl.c b/vl.c index a5ae5f23d2..2a8b2ee16d 100644 --- a/vl.c +++ b/vl.c @@ -2322,11 +2322,6 @@ static int mon_init_func(void *opaque, QemuOpts *opts, Error **errp) if (qemu_opt_get_bool(opts, "pretty", 0)) flags |= MONITOR_USE_PRETTY; - /* OOB is off by default */ - if (qemu_opt_get_bool(opts, "x-oob", 0)) { - flags |= MONITOR_USE_OOB; - } - chardev = qemu_opt_get(opts, "chardev"); if (!chardev) { error_report("chardev is required"); |