Monitor patches for 2018-12-12

-----BEGIN PGP SIGNATURE-----
 
 iQIcBAABAgAGBQJcEN4PAAoJEDhwtADrkYZT9IsP/1B1sWdX4vakOnSMXeTgqHE6
 8G6jRlwOzHF4vehqDHrx7VLril5G3t9An7ZsGB95L54SXRdiaGEyn9ScK+PbHQzD
 Qak0RycT1fX2GP7wJG/yxyXKG8jAMoHpFwFzHnfM4kW+UegLOVOBxPqVfKQmTlz2
 rPqVVUNFp6oRpTPUJ6GDpOtyJ4/Y2YKx5kncctkV/SWpT1tXRxue9gOyx1qesqAK
 i2FAZPmpw3SEacZZvR9Zzl5s5mPjCK4qfVfaAaBNSd/HMt2OIoixN+q+vQ7hfXCk
 yuPnvf0dWVxBkIltbs225qTx9M8Km5ElSuiuXuwzVyUBJEVUmV2UgAL8VMuDi1lQ
 M1iOGpO4nsaD3Lbg3vZnRaJbXkWnZshrYntrwbi6sP5G88TkzTknpSJrP1UXOJSx
 YAhnKl6B0hulgl1eRW+Zq5qKKlz5K7huaexHtgN5hd3x+kljF1Gh9qSdUxLunwdB
 TMW2x01xjKwv84XAPRibNMSsxMgP7nMCrx3U9sxf7w/kfZYQSaqv//cCn8mqRWgc
 9PAtj1DQuPG9n4vOqjHF6D2kQ81oFkJWLsdvYM1TXXuSHDc6fZRrGEUc1OhNcYdK
 XPoA69RP/NShoME1070vsrlzMndwg6zuKwJAOX5vja0rngZk25IzDbt9DE7USook
 72dmKIiFDBFD1TECj5Yo
 =/D6l
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/armbru/tags/pull-monitor-2018-12-12' into staging

Monitor patches for 2018-12-12

# gpg: Signature made Wed 12 Dec 2018 10:08:15 GMT
# gpg:                using RSA key 3870B400EB918653
# gpg: Good signature from "Markus Armbruster <armbru@redhat.com>"
# gpg:                 aka "Markus Armbruster <armbru@pond.sub.org>"
# Primary key fingerprint: 354B C8B3 D7EB 2A6B 6867  4E5F 3870 B400 EB91 8653

* remotes/armbru/tags/pull-monitor-2018-12-12:
  tests: add oob functional test for test-qmp-cmds
  Revert "tests: Add parameter to qtest_init_without_qmp_handshake"
  monitor: Remove "x-oob", offer capability "oob" unconditionally
  monitor: Suspend monitor instead dropping commands
  monitor: avoid potential dead-lock when cleaning up
  monitor: prevent inserting new monitors after cleanup
  colo: check chardev can switch context
  monitor: check if chardev can switch gcontext for OOB
  char: add a QEMU_CHAR_FEATURE_GCONTEXT flag
  monitor: accept chardev input from iothread
  monitor: inline ambiguous helper functions

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
This commit is contained in:
Peter Maydell 2018-12-13 13:41:44 +00:00
commit c3ec0fa1a8
12 changed files with 118 additions and 129 deletions

View File

@ -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)

View File

@ -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.

View File

@ -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;

View File

@ -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);

139
monitor.c
View File

@ -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 */ }
},

View File

@ -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;
}

View File

@ -3444,46 +3444,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:
#

View File

@ -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 */

View File

@ -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:

View File

@ -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);

View File

@ -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",

5
vl.c
View File

@ -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");