aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPeter Maydell <peter.maydell@linaro.org>2018-06-19 15:19:07 +0100
committerPeter Maydell <peter.maydell@linaro.org>2018-06-19 15:19:07 +0100
commita01fba4687b9a2464ed9b0eee60bb42b8f81f61b (patch)
tree35f36b0261a77da65157dc7de424c35785a0397c
parente8729c154b3fb97232e640a43c832122f0fdd32d (diff)
parent474514668b47c9d2148b526e379ffbb6764d9a9e (diff)
Merge remote-tracking branch 'remotes/armbru/tags/pull-monitor-2018-06-18' into staging
Monitor patches for 2018-06-18 # gpg: Signature made Mon 18 Jun 2018 14:50:29 BST # 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-06-18: monitor: add lock to protect mon_fdsets monitor: move init global earlier monitor: remove event_clock_type monitor: fix comment for monitor_lock monitor: more comments on lock-free elements monitor: protect mon->fds with mon_lock monitor: rename out_lock to mon_lock Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
-rw-r--r--monitor.c168
-rw-r--r--stubs/fdset.c2
-rw-r--r--util/osdep.c3
-rw-r--r--vl.c7
4 files changed, 121 insertions, 59 deletions
diff --git a/monitor.c b/monitor.c
index 6d0cec552e..885e000f9b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -207,22 +207,35 @@ struct Monitor {
bool skip_flush;
bool use_io_thr;
- /* We can't access guest memory when holding the lock */
- QemuMutex out_lock;
- QString *outbuf;
- guint out_watch;
-
- /* Read under either BQL or out_lock, written with BQL+out_lock. */
- int mux_out;
-
+ /*
+ * State used only in the thread "owning" the monitor.
+ * If @use_io_thr, this is mon_global.mon_iothread.
+ * Else, it's the main thread.
+ * These members can be safely accessed without locks.
+ */
ReadLineState *rs;
+
MonitorQMP qmp;
gchar *mon_cpu_path;
BlockCompletionFunc *password_completion_cb;
void *password_opaque;
mon_cmd_t *cmd_table;
- QLIST_HEAD(,mon_fd_t) fds;
QTAILQ_ENTRY(Monitor) entry;
+
+ /*
+ * The per-monitor lock. We can't access guest memory when holding
+ * the lock.
+ */
+ QemuMutex mon_lock;
+
+ /*
+ * Fields that are protected by the per-monitor lock.
+ */
+ QLIST_HEAD(, mon_fd_t) fds;
+ QString *outbuf;
+ guint out_watch;
+ /* Read under either BQL or mon_lock, written with BQL+mon_lock. */
+ int mux_out;
};
/* Let's add monitor global variables to this struct. */
@@ -253,11 +266,15 @@ typedef struct QMPRequest QMPRequest;
/* QMP checker flags */
#define QMP_ACCEPT_UNKNOWNS 1
-/* Protects mon_list, monitor_event_state. */
+/* Protects mon_list, monitor_qapi_event_state. */
static QemuMutex monitor_lock;
-
+static GHashTable *monitor_qapi_event_state;
static QTAILQ_HEAD(mon_list, Monitor) mon_list;
+
+/* Protects mon_fdsets */
+static QemuMutex mon_fdsets_lock;
static QLIST_HEAD(mon_fdsets, MonFdset) mon_fdsets;
+
static int mon_refcount;
static mon_cmd_t mon_cmds[];
@@ -267,8 +284,6 @@ QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
Monitor *cur_mon;
-static QEMUClockType event_clock_type = QEMU_CLOCK_REALTIME;
-
static void monitor_command_cb(void *opaque, const char *cmdline,
void *readline_opaque);
@@ -295,6 +310,19 @@ static inline bool monitor_is_hmp_non_interactive(const Monitor *mon)
return !monitor_is_qmp(mon) && !monitor_uses_readline(mon);
}
+/*
+ * Return the clock to use for recording an event's time.
+ * Beware: result is invalid before configure_accelerator().
+ */
+static inline QEMUClockType monitor_get_event_clock(void)
+{
+ /*
+ * This allows us to perform tests on the monitor queues to verify
+ * that the rate limits are enforced.
+ */
+ return qtest_enabled() ? QEMU_CLOCK_VIRTUAL : QEMU_CLOCK_REALTIME;
+}
+
/**
* Is the current monitor, if any, a QMP monitor?
*/
@@ -365,14 +393,14 @@ static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
{
Monitor *mon = opaque;
- qemu_mutex_lock(&mon->out_lock);
+ qemu_mutex_lock(&mon->mon_lock);
mon->out_watch = 0;
monitor_flush_locked(mon);
- qemu_mutex_unlock(&mon->out_lock);
+ qemu_mutex_unlock(&mon->mon_lock);
return FALSE;
}
-/* Called with mon->out_lock held. */
+/* Called with mon->mon_lock held. */
static void monitor_flush_locked(Monitor *mon)
{
int rc;
@@ -410,9 +438,9 @@ static void monitor_flush_locked(Monitor *mon)
void monitor_flush(Monitor *mon)
{
- qemu_mutex_lock(&mon->out_lock);
+ qemu_mutex_lock(&mon->mon_lock);
monitor_flush_locked(mon);
- qemu_mutex_unlock(&mon->out_lock);
+ qemu_mutex_unlock(&mon->mon_lock);
}
/* flush at every end of line */
@@ -420,7 +448,7 @@ static void monitor_puts(Monitor *mon, const char *str)
{
char c;
- qemu_mutex_lock(&mon->out_lock);
+ qemu_mutex_lock(&mon->mon_lock);
for(;;) {
c = *str++;
if (c == '\0')
@@ -433,7 +461,7 @@ static void monitor_puts(Monitor *mon, const char *str)
monitor_flush_locked(mon);
}
}
- qemu_mutex_unlock(&mon->out_lock);
+ qemu_mutex_unlock(&mon->mon_lock);
}
void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
@@ -558,8 +586,6 @@ static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = {
[QAPI_EVENT_VSERPORT_CHANGE] = { 1000 * SCALE_MS },
};
-GHashTable *monitor_qapi_event_state;
-
/*
* Emits the event to every monitor instance, @event is only used for trace
* Called with monitor_lock held.
@@ -620,7 +646,7 @@ monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
* monitor_qapi_event_handler() in evconf->rate ns. Any
* events arriving before then will be delayed until then.
*/
- int64_t now = qemu_clock_get_ns(event_clock_type);
+ int64_t now = qemu_clock_get_ns(monitor_get_event_clock());
monitor_qapi_event_emit(event, qdict);
@@ -628,7 +654,7 @@ monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
evstate->event = event;
evstate->data = qobject_ref(data);
evstate->qdict = NULL;
- evstate->timer = timer_new_ns(event_clock_type,
+ evstate->timer = timer_new_ns(monitor_get_event_clock(),
monitor_qapi_event_handler,
evstate);
g_hash_table_add(monitor_qapi_event_state, evstate);
@@ -653,7 +679,7 @@ static void monitor_qapi_event_handler(void *opaque)
qemu_mutex_lock(&monitor_lock);
if (evstate->qdict) {
- int64_t now = qemu_clock_get_ns(event_clock_type);
+ int64_t now = qemu_clock_get_ns(monitor_get_event_clock());
monitor_qapi_event_emit(evstate->event, evstate->qdict);
qobject_unref(evstate->qdict);
@@ -709,10 +735,6 @@ static gboolean qapi_event_throttle_equal(const void *a, const void *b)
static void monitor_qapi_event_init(void)
{
- if (qtest_enabled()) {
- event_clock_type = QEMU_CLOCK_VIRTUAL;
- }
-
monitor_qapi_event_state = g_hash_table_new(qapi_event_throttle_hash,
qapi_event_throttle_equal);
qmp_event_set_func_emit(monitor_qapi_event_queue);
@@ -724,7 +746,7 @@ static void monitor_data_init(Monitor *mon, bool skip_flush,
bool use_io_thr)
{
memset(mon, 0, sizeof(Monitor));
- qemu_mutex_init(&mon->out_lock);
+ qemu_mutex_init(&mon->mon_lock);
qemu_mutex_init(&mon->qmp.qmp_queue_lock);
mon->outbuf = qstring_new();
/* Use *mon_cmds by default. */
@@ -744,7 +766,7 @@ static void monitor_data_destroy(Monitor *mon)
}
readline_free(mon->rs);
qobject_unref(mon->outbuf);
- qemu_mutex_destroy(&mon->out_lock);
+ qemu_mutex_destroy(&mon->mon_lock);
qemu_mutex_destroy(&mon->qmp.qmp_queue_lock);
monitor_qmp_cleanup_req_queue_locked(mon);
monitor_qmp_cleanup_resp_queue_locked(mon);
@@ -776,13 +798,13 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
handle_hmp_command(&hmp, command_line);
cur_mon = old_mon;
- qemu_mutex_lock(&hmp.out_lock);
+ qemu_mutex_lock(&hmp.mon_lock);
if (qstring_get_length(hmp.outbuf) > 0) {
output = g_strdup(qstring_get_str(hmp.outbuf));
} else {
output = g_strdup("");
}
- qemu_mutex_unlock(&hmp.out_lock);
+ qemu_mutex_unlock(&hmp.mon_lock);
out:
monitor_data_destroy(&hmp);
@@ -1306,7 +1328,7 @@ void qmp_qmp_capabilities(bool has_enable, QMPCapabilityList *enable,
cur_mon->qmp.commands = &qmp_commands;
}
-/* set the current CPU defined by the user */
+/* Set the current CPU defined by the user. Callers must hold BQL. */
int monitor_set_cpu(int cpu_index)
{
CPUState *cpu;
@@ -1320,6 +1342,7 @@ int monitor_set_cpu(int cpu_index)
return 0;
}
+/* Callers must hold BQL. */
static CPUState *mon_get_cpu_sync(bool synchronize)
{
CPUState *cpu;
@@ -2182,7 +2205,7 @@ static void hmp_acl_remove(Monitor *mon, const QDict *qdict)
void qmp_getfd(const char *fdname, Error **errp)
{
mon_fd_t *monfd;
- int fd;
+ int fd, tmp_fd;
fd = qemu_chr_fe_get_msgfd(&cur_mon->chr);
if (fd == -1) {
@@ -2197,13 +2220,17 @@ void qmp_getfd(const char *fdname, Error **errp)
return;
}
+ qemu_mutex_lock(&cur_mon->mon_lock);
QLIST_FOREACH(monfd, &cur_mon->fds, next) {
if (strcmp(monfd->name, fdname) != 0) {
continue;
}
- close(monfd->fd);
+ tmp_fd = monfd->fd;
monfd->fd = fd;
+ qemu_mutex_unlock(&cur_mon->mon_lock);
+ /* Make sure close() is out of critical section */
+ close(tmp_fd);
return;
}
@@ -2212,24 +2239,31 @@ void qmp_getfd(const char *fdname, Error **errp)
monfd->fd = fd;
QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
+ qemu_mutex_unlock(&cur_mon->mon_lock);
}
void qmp_closefd(const char *fdname, Error **errp)
{
mon_fd_t *monfd;
+ int tmp_fd;
+ qemu_mutex_lock(&cur_mon->mon_lock);
QLIST_FOREACH(monfd, &cur_mon->fds, next) {
if (strcmp(monfd->name, fdname) != 0) {
continue;
}
QLIST_REMOVE(monfd, next);
- close(monfd->fd);
+ tmp_fd = monfd->fd;
g_free(monfd->name);
g_free(monfd);
+ qemu_mutex_unlock(&cur_mon->mon_lock);
+ /* Make sure close() is out of critical section */
+ close(tmp_fd);
return;
}
+ qemu_mutex_unlock(&cur_mon->mon_lock);
error_setg(errp, QERR_FD_NOT_FOUND, fdname);
}
@@ -2237,6 +2271,7 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
{
mon_fd_t *monfd;
+ qemu_mutex_lock(&mon->mon_lock);
QLIST_FOREACH(monfd, &mon->fds, next) {
int fd;
@@ -2250,10 +2285,12 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
QLIST_REMOVE(monfd, next);
g_free(monfd->name);
g_free(monfd);
+ qemu_mutex_unlock(&mon->mon_lock);
return fd;
}
+ qemu_mutex_unlock(&mon->mon_lock);
error_setg(errp, "File descriptor named '%s' has not been found", fdname);
return -1;
}
@@ -2285,9 +2322,11 @@ static void monitor_fdsets_cleanup(void)
MonFdset *mon_fdset;
MonFdset *mon_fdset_next;
+ qemu_mutex_lock(&mon_fdsets_lock);
QLIST_FOREACH_SAFE(mon_fdset, &mon_fdsets, next, mon_fdset_next) {
monitor_fdset_cleanup(mon_fdset);
}
+ qemu_mutex_unlock(&mon_fdsets_lock);
}
AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, bool has_opaque,
@@ -2322,6 +2361,7 @@ void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
MonFdsetFd *mon_fdset_fd;
char fd_str[60];
+ qemu_mutex_lock(&mon_fdsets_lock);
QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
if (mon_fdset->id != fdset_id) {
continue;
@@ -2341,10 +2381,12 @@ void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
goto error;
}
monitor_fdset_cleanup(mon_fdset);
+ qemu_mutex_unlock(&mon_fdsets_lock);
return;
}
error:
+ qemu_mutex_unlock(&mon_fdsets_lock);
if (has_fd) {
snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId64 ", fd:%" PRId64,
fdset_id, fd);
@@ -2360,6 +2402,7 @@ FdsetInfoList *qmp_query_fdsets(Error **errp)
MonFdsetFd *mon_fdset_fd;
FdsetInfoList *fdset_list = NULL;
+ qemu_mutex_lock(&mon_fdsets_lock);
QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
FdsetInfoList *fdset_info = g_malloc0(sizeof(*fdset_info));
FdsetFdInfoList *fdsetfd_list = NULL;
@@ -2389,6 +2432,7 @@ FdsetInfoList *qmp_query_fdsets(Error **errp)
fdset_info->next = fdset_list;
fdset_list = fdset_info;
}
+ qemu_mutex_unlock(&mon_fdsets_lock);
return fdset_list;
}
@@ -2401,6 +2445,7 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
MonFdsetFd *mon_fdset_fd;
AddfdInfo *fdinfo;
+ qemu_mutex_lock(&mon_fdsets_lock);
if (has_fdset_id) {
QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
/* Break if match found or match impossible due to ordering by ID */
@@ -2421,6 +2466,7 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
if (fdset_id < 0) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "fdset-id",
"a non-negative value");
+ qemu_mutex_unlock(&mon_fdsets_lock);
return NULL;
}
/* Use specified fdset ID */
@@ -2471,16 +2517,21 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
fdinfo->fdset_id = mon_fdset->id;
fdinfo->fd = mon_fdset_fd->fd;
+ qemu_mutex_unlock(&mon_fdsets_lock);
return fdinfo;
}
int monitor_fdset_get_fd(int64_t fdset_id, int flags)
{
-#ifndef _WIN32
+#ifdef _WIN32
+ return -ENOENT;
+#else
MonFdset *mon_fdset;
MonFdsetFd *mon_fdset_fd;
int mon_fd_flags;
+ int ret;
+ qemu_mutex_lock(&mon_fdsets_lock);
QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
if (mon_fdset->id != fdset_id) {
continue;
@@ -2488,20 +2539,24 @@ int monitor_fdset_get_fd(int64_t fdset_id, int flags)
QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) {
mon_fd_flags = fcntl(mon_fdset_fd->fd, F_GETFL);
if (mon_fd_flags == -1) {
- return -1;
+ ret = -errno;
+ goto out;
}
if ((flags & O_ACCMODE) == (mon_fd_flags & O_ACCMODE)) {
- return mon_fdset_fd->fd;
+ ret = mon_fdset_fd->fd;
+ goto out;
}
}
- errno = EACCES;
- return -1;
+ ret = -EACCES;
+ goto out;
}
-#endif
+ ret = -ENOENT;
- errno = ENOENT;
- return -1;
+out:
+ qemu_mutex_unlock(&mon_fdsets_lock);
+ return ret;
+#endif
}
int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
@@ -2509,20 +2564,25 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
MonFdset *mon_fdset;
MonFdsetFd *mon_fdset_fd_dup;
+ qemu_mutex_lock(&mon_fdsets_lock);
QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
if (mon_fdset->id != fdset_id) {
continue;
}
QLIST_FOREACH(mon_fdset_fd_dup, &mon_fdset->dup_fds, next) {
if (mon_fdset_fd_dup->fd == dup_fd) {
- return -1;
+ goto err;
}
}
mon_fdset_fd_dup = g_malloc0(sizeof(*mon_fdset_fd_dup));
mon_fdset_fd_dup->fd = dup_fd;
QLIST_INSERT_HEAD(&mon_fdset->dup_fds, mon_fdset_fd_dup, next);
+ qemu_mutex_unlock(&mon_fdsets_lock);
return 0;
}
+
+err:
+ qemu_mutex_unlock(&mon_fdsets_lock);
return -1;
}
@@ -2531,6 +2591,7 @@ static int monitor_fdset_dup_fd_find_remove(int dup_fd, bool remove)
MonFdset *mon_fdset;
MonFdsetFd *mon_fdset_fd_dup;
+ qemu_mutex_lock(&mon_fdsets_lock);
QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
QLIST_FOREACH(mon_fdset_fd_dup, &mon_fdset->dup_fds, next) {
if (mon_fdset_fd_dup->fd == dup_fd) {
@@ -2539,13 +2600,17 @@ static int monitor_fdset_dup_fd_find_remove(int dup_fd, bool remove)
if (QLIST_EMPTY(&mon_fdset->dup_fds)) {
monitor_fdset_cleanup(mon_fdset);
}
- return -1;
+ goto err;
} else {
+ qemu_mutex_unlock(&mon_fdsets_lock);
return mon_fdset->id;
}
}
}
}
+
+err:
+ qemu_mutex_unlock(&mon_fdsets_lock);
return -1;
}
@@ -4381,9 +4446,9 @@ static void monitor_event(void *opaque, int event)
switch (event) {
case CHR_EVENT_MUX_IN:
- qemu_mutex_lock(&mon->out_lock);
+ qemu_mutex_lock(&mon->mon_lock);
mon->mux_out = 0;
- qemu_mutex_unlock(&mon->out_lock);
+ qemu_mutex_unlock(&mon->mon_lock);
if (mon->reset_seen) {
readline_restart(mon->rs);
monitor_resume(mon);
@@ -4403,9 +4468,9 @@ static void monitor_event(void *opaque, int event)
} else {
atomic_inc(&mon->suspend_cnt);
}
- qemu_mutex_lock(&mon->out_lock);
+ qemu_mutex_lock(&mon->mon_lock);
mon->mux_out = 1;
- qemu_mutex_unlock(&mon->out_lock);
+ qemu_mutex_unlock(&mon->mon_lock);
break;
case CHR_EVENT_OPENED:
@@ -4485,6 +4550,7 @@ void monitor_init_globals(void)
monitor_qapi_event_init();
sortcmdlist();
qemu_mutex_init(&monitor_lock);
+ qemu_mutex_init(&mon_fdsets_lock);
monitor_iothread_init();
}
diff --git a/stubs/fdset.c b/stubs/fdset.c
index 6020cf28c8..4f3edf2ea4 100644
--- a/stubs/fdset.c
+++ b/stubs/fdset.c
@@ -14,7 +14,7 @@ int monitor_fdset_dup_fd_find(int dup_fd)
int monitor_fdset_get_fd(int64_t fdset_id, int flags)
{
- return -1;
+ return -ENOENT;
}
void monitor_fdset_dup_fd_remove(int dupfd)
diff --git a/util/osdep.c b/util/osdep.c
index a73de0e1ba..ea51d500b6 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -302,7 +302,8 @@ int qemu_open(const char *name, int flags, ...)
}
fd = monitor_fdset_get_fd(fdset_id, flags);
- if (fd == -1) {
+ if (fd < 0) {
+ errno = -fd;
return -1;
}
diff --git a/vl.c b/vl.c
index 6e34fb348d..b3426e03d0 100644
--- a/vl.c
+++ b/vl.c
@@ -2978,6 +2978,7 @@ int main(int argc, char **argv, char **envp)
runstate_init();
postcopy_infrastructure_init();
+ monitor_init_globals();
if (qcrypto_init(&err) < 0) {
error_reportf_err(err, "cannot initialize crypto: ");
@@ -4412,12 +4413,6 @@ int main(int argc, char **argv, char **envp)
default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
- /*
- * Note: qtest_enabled() (which is used in monitor_qapi_event_init())
- * depends on configure_accelerator() above.
- */
- monitor_init_globals();
-
if (qemu_opts_foreach(qemu_find_opts("mon"),
mon_init_func, NULL, NULL)) {
exit(1);