Commit a01fba46 authored by Peter Maydell's avatar Peter Maydell
Browse files

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: default avatarPeter Maydell <peter.maydell@linaro.org>
parents e8729c15 47451466
Loading
Loading
Loading
Loading
+117 −51
Original line number Diff line number Diff line
@@ -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();
}

+1 −1
Original line number Diff line number Diff line
@@ -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)
+2 −1
Original line number Diff line number Diff line
@@ -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;
        }

+1 −6
Original line number Diff line number Diff line
@@ -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);