Additional nitpick detail on Kevin's request.
Kevin Wolf <kw...@redhat.com> writes:
> This way, a monitor command handler will still be able to access the
> current monitor, but when it yields, all other code code will correctly
> get NULL from monitor_cur().
>
> This uses a hash table to map the coroutine pointer to the current
> monitor of that coroutine. Outside of coroutine context, we associate
> the current monitor with the leader coroutine of the current thread.
>
> Approaches to implement some form of coroutine local storage directly in
> the coroutine core code have been considered and discarded because they
> didn't end up being much more generic than the hash table and their
> performance impact on coroutines not using coroutine local storage was
> unclear. As the block layer uses a coroutine per I/O request, this is a
> fast path and we have to be careful. It's safest to just stay out of
> this path with code only used by the monitor.
>
> Signed-off-by: Kevin Wolf <kw...@redhat.com>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
> ---
> include/monitor/monitor.h | 2 +-
> monitor/hmp.c | 4 ++--
> monitor/monitor.c | 34 +++++++++++++++++++++++++++-------
> qapi/qmp-dispatch.c | 4 ++--
> stubs/monitor-core.c | 2 +-
> 5 files changed, 33 insertions(+), 13 deletions(-)
>
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index 7b0ad1de12..026f8a31b2 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -13,7 +13,7 @@ typedef struct MonitorOptions MonitorOptions;
> extern QemuOptsList qemu_mon_opts;
>
> Monitor *monitor_cur(void);
> -Monitor *monitor_set_cur(Monitor *mon);
> +Monitor *monitor_set_cur(Coroutine *co, Monitor *mon);
> bool monitor_cur_is_qmp(void);
>
> void monitor_init_globals(void);
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> index 896c670183..4b66ca1cd6 100644
> --- a/monitor/hmp.c
> +++ b/monitor/hmp.c
> @@ -1081,9 +1081,9 @@ void handle_hmp_command(MonitorHMP *mon, const char
> *cmdline)
> }
>
> /* old_mon is non-NULL when called from qmp_human_monitor_command() */
> - old_mon = monitor_set_cur(&mon->common);
> + old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->common);
> cmd->cmd(&mon->common, qdict);
> - monitor_set_cur(old_mon);
> + monitor_set_cur(qemu_coroutine_self(), old_mon);
>
> qobject_unref(qdict);
> }
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index be3839a7aa..629aa073ee 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -58,29 +58,48 @@ IOThread *mon_iothread;
> /* Bottom half to dispatch the requests received from I/O thread */
> QEMUBH *qmp_dispatcher_bh;
>
> -/* Protects mon_list, monitor_qapi_event_state, monitor_destroyed. */
> +/*
> + * Protects mon_list, monitor_qapi_event_state, coroutine_mon,
> + * monitor_destroyed.
> + */
> QemuMutex monitor_lock;
> static GHashTable *monitor_qapi_event_state;
> +static GHashTable *coroutine_mon; /* Maps Coroutine* to Monitor* */
>
> MonitorList mon_list;
> int mon_refcount;
> static bool monitor_destroyed;
>
> -static __thread Monitor *cur_monitor;
> -
> Monitor *monitor_cur(void)
> {
> - return cur_monitor;
> + Monitor *mon;
> +
> + qemu_mutex_lock(&monitor_lock);
> + mon = g_hash_table_lookup(coroutine_mon, qemu_coroutine_self());
> + qemu_mutex_unlock(&monitor_lock);
> +
> + return mon;
> }
>
> /**
> * Sets a new current monitor and returns the old one.
> + *
> + * If a non-NULL monitor is set for a coroutine, another call resetting it to
> + * NULL is required before the coroutine terminates, otherwise a stale entry
> + * would remain in the hash table.
Wrapping the comment around column 70 or so would make it easier to
read.
> */
> -Monitor *monitor_set_cur(Monitor *mon)
> +Monitor *monitor_set_cur(Coroutine *co, Monitor *mon)
> {
> - Monitor *old_monitor = cur_monitor;
> + Monitor *old_monitor = monitor_cur();
> +
> + qemu_mutex_lock(&monitor_lock);
> + if (mon) {
> + g_hash_table_replace(coroutine_mon, co, mon);
> + } else {
> + g_hash_table_remove(coroutine_mon, co);
> + }
> + qemu_mutex_unlock(&monitor_lock);
>
> - cur_monitor = mon;
> return old_monitor;
> }
>
[...]
