On Tue, 3 Jun 2014 18:39:10 +0200
Paolo Bonzini <pbonz...@redhat.com> wrote:
> This lets the block layer emit QMP events from outside the I/O thread.
To be honest I'm starting to forget monitor code, but this looks correct
to me. I have only one comment below that doesn't prevent me from adding:
Reviewed-by: Luiz Capitulino <lcapitul...@redhat.com>
>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> monitor.c | 35 ++++++++++++++++++++++++++++-------
> 1 file changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 342e83b..ebc66fb 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -191,8 +191,11 @@ struct Monitor {
> int flags;
> int suspend_cnt;
> bool skip_flush;
> +
> + QemuMutex out_lock;
> QString *outbuf;
> - guint watch;
> + guint out_watch;
> +
> ReadLineState *rs;
> MonitorControl *mc;
> CPUState *mon_cpu;
> @@ -265,17 +268,22 @@ int monitor_read_password(Monitor *mon, ReadLineFunc
> *readline_func,
> }
> }
>
> +static void do_monitor_flush(Monitor *mon);
> +
> static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
> void *opaque)
> {
> Monitor *mon = opaque;
>
> - mon->watch = 0;
> + qemu_mutex_lock(&mon->out_lock);
> + mon->out_watch = 0;
> monitor_flush(mon);
> + qemu_mutex_unlock(&mon->out_lock);
> return FALSE;
> }
>
> -void monitor_flush(Monitor *mon)
> +/* Called with mon->out_lock held. */
> +static void do_monitor_flush(Monitor *mon)
> {
> int rc;
> size_t len;
> @@ -302,18 +310,26 @@ void monitor_flush(Monitor *mon)
> QDECREF(mon->outbuf);
> mon->outbuf = tmp;
> }
> - if (mon->watch == 0) {
> - mon->watch = qemu_chr_fe_add_watch(mon->chr, G_IO_OUT,
> - monitor_unblocked, mon);
> + if (mon->out_watch == 0) {
> + mon->out_watch = qemu_chr_fe_add_watch(mon->chr, G_IO_OUT,
> + monitor_unblocked, mon);
> }
> }
> }
>
> +void monitor_flush(Monitor *mon)
> +{
> + qemu_mutex_lock(&mon->out_lock);
> + do_monitor_flush(mon);
> + qemu_mutex_unlock(&mon->out_lock);
> +}
> +
> /* flush at every end of line */
> static void monitor_puts(Monitor *mon, const char *str)
> {
> char c;
>
> + qemu_mutex_lock(&mon->out_lock);
> for(;;) {
> c = *str++;
> if (c == '\0')
> @@ -323,9 +339,10 @@ static void monitor_puts(Monitor *mon, const char *str)
> }
> qstring_append_chr(mon->outbuf, c);
> if (c == '\n') {
> - monitor_flush(mon);
> + do_monitor_flush(mon);
> }
> }
> + qemu_mutex_unlock(&mon->out_lock);
> }
>
> void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
> @@ -690,6 +707,7 @@ static void handle_user_command(Monitor *mon, const char
> *cmdline);
> static void monitor_data_init(Monitor *mon)
> {
> memset(mon, 0, sizeof(Monitor));
> + qemu_mutex_init(&mon->out_lock);
> mon->outbuf = qstring_new();
> /* Use *mon_cmds by default. */
> mon->cmd_table = mon_cmds;
> @@ -698,6 +716,7 @@ static void monitor_data_init(Monitor *mon)
> static void monitor_data_destroy(Monitor *mon)
> {
> QDECREF(mon->outbuf);
> + qemu_mutex_destroy(&mon->out_lock);
> }
>
> char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
> @@ -725,11 +744,13 @@ char *qmp_human_monitor_command(const char
> *command_line, bool has_cpu_index,
> handle_user_command(&hmp, command_line);
> cur_mon = old_mon;
>
> + qemu_mutex_lock(&hmp.out_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);
Are you sure we need to lock/unlock in this function? hmp is allocated
in the stack.
>
> out:
> monitor_data_destroy(&hmp);
