On Tue, Sep 15, 2026 at 6:49 PM Markus Armbruster <[email protected]> wrote:
>
> Zhang Chen <[email protected]> writes:
>
> > Monitors that require an IOThread share the internal mon_iothread.
> > Account for each monitor explicitly in IOThread holder tracking, using
> > the monitor's canonical QOM path to distinguish holders.
> >
> > Acquire the shared monitor IOThread AioContext once and store the
> > IOThread and canonical QOM path so the holder can be released after the
> > monitor has been removed from the QOM tree. Use the stored IOThread for
> > per-monitor checks and accesses after initialization, and reuse the
> > stored context for later operations.
> >
> > Release the holder from monitor_finalize(), after resources associated
> > with the AioContext have been destroyed. This pairs the reference taken
> > in complete() with instance finalization and keeps the IOThread alive
> > throughout monitor teardown.
> >
> > The internal mon_iothread remains hidden from query-iothreads, as before.
> > Management applications therefore cannot use it to determine which
> > monitors are using the IOThread. Nevertheless, track the monitor
> > holders so that they follow the same explicit holder and
> > IOThread reference lifecycle as other AioContext users.
> >
> > Signed-off-by: Zhang Chen <[email protected]>
> > ---
> > monitor/monitor-internal.h | 7 +++++--
> > monitor/monitor.c | 36 ++++++++++++++++++++++++++----------
> > monitor/qmp.c | 14 +++++++-------
> > 3 files changed, 38 insertions(+), 19 deletions(-)
> >
> > diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> > index 822a66d06d..020afdaebf 100644
> > --- a/monitor/monitor-internal.h
> > +++ b/monitor/monitor-internal.h
> > @@ -77,6 +77,11 @@ struct Monitor {
> > GString *outbuf;
> > guint out_watch;
> > int mux_out;
> > +
> > + /* iothread context and holder identity */
> > + IOThread *iothread;
> > + char *iothread_qom_path;
> > + AioContext *ctx;
> > };
> >
> > struct MonitorQMPClass {
> > @@ -109,14 +114,12 @@ struct MonitorQMP {
> > };
> >
> > typedef QTAILQ_HEAD(MonitorList, Monitor) MonitorList;
> > -extern IOThread *mon_iothread;
> > extern Coroutine *qmp_dispatcher_co;
> > extern bool qmp_dispatcher_co_shutdown;
> > extern QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
> > extern QemuMutex monitor_lock;
> > extern MonitorList mon_list;
> >
> > -bool monitor_requires_iothread(const Monitor *mon);
> > int monitor_can_read(void *opaque);
> > void monitor_cancel_out_watch(Monitor *mon);
> > void monitor_list_append(Monitor *mon);
> > diff --git a/monitor/monitor.c b/monitor/monitor.c
> > index 2654b81890..a80d8dbf45 100644
> > --- a/monitor/monitor.c
> > +++ b/monitor/monitor.c
> > @@ -55,7 +55,7 @@ typedef struct {
> > } MonitorQAPIEventConf;
> >
> > /* Shared monitor I/O thread */
> > -IOThread *mon_iothread;
> > +static IOThread *mon_iothread;
> >
> > /* Coroutine to dispatch the requests received from I/O thread */
> > Coroutine *qmp_dispatcher_co;
> > @@ -93,6 +93,16 @@ static void monitor_finalize(Object *obj)
> > qemu_chr_fe_deinit(&mon->chr, false);
> > g_string_free(mon->outbuf, true);
> > qemu_mutex_destroy(&mon->mon_lock);
> > +
> > + if (mon->iothread) {
> > + const IOThreadHolder io_holder = {
> > + .type = IO_THREAD_HOLDER_KIND_QOM_OBJECT,
> > + .u.qom_object.qom_path = mon->iothread_qom_path,
> > + };
> > +
> > + iothread_unref_and_put_aio_context(mon->iothread, &io_holder);
> > + }
> > + g_free(mon->iothread_qom_path);
> > }
> >
> > static char *monitor_get_chardev_id(Object *obj, Error **errp)
> > @@ -164,7 +174,7 @@ Monitor *monitor_set_cur(Coroutine *co, Monitor *mon)
> > return old_monitor;
> > }
> >
> > -bool monitor_requires_iothread(const Monitor *mon)
> > +static bool monitor_requires_iothread(const Monitor *mon)
> > {
> > MonitorClass *cls = MONITOR_GET_CLASS(mon);
> > return cls->requires_iothread && cls->requires_iothread(mon);
> > @@ -188,8 +198,8 @@ void monitor_cancel_out_watch(Monitor *mon)
> > GMainContext *ctx = NULL;
> > GSource *src;
> >
> > - if (monitor_requires_iothread(mon)) {
> > - ctx = iothread_get_g_main_context(mon_iothread);
> > + if (mon->iothread) {
> > + ctx = iothread_get_g_main_context(mon->iothread);
> > }
> > src = g_main_context_find_source_by_id(ctx, mon->out_watch);
> > if (!src && ctx) {
> > @@ -516,12 +526,12 @@ void monitor_suspend(Monitor *mon)
> > {
> > qatomic_inc(&mon->suspend_cnt);
> >
> > - if (monitor_requires_iothread(mon)) {
> > + if (mon->iothread) {
> > /*
> > * Kick I/O thread to make sure this takes effect. It'll be
> > * evaluated again in prepare() of the watch object.
> > */
> > - aio_notify(iothread_get_aio_context(mon_iothread));
> > + aio_notify(mon->ctx);
> > }
> >
> > trace_monitor_suspend(mon, 1);
> > @@ -661,7 +671,6 @@ char *monitor_compat_id(void)
> > static void monitor_complete(UserCreatable *uc, Error **errp)
> > {
> > Monitor *mon = MONITOR(uc);
> > - AioContext *ctx;
> >
> > if (mon->chardev_id) {
> > Chardev *chr = qemu_chr_find(mon->chardev_id);
> > @@ -680,11 +689,18 @@ static void monitor_complete(UserCreatable *uc, Error
> > **errp)
> > mon_iothread = iothread_create("mon_iothread", &error_abort);
> > }
> >
> > - ctx = iothread_get_aio_context(mon_iothread);
> > + mon->iothread = mon_iothread;
> > + mon->iothread_qom_path = object_get_canonical_path(OBJECT(mon));
>
> Hmm, this is the monitor's QOM path, not the I/O threads. Let's rename
> Monitor member iothread_qom_path to monitor_qom_path or just qom_path.
OK.
>
> > + const IOThreadHolder io_holder = {
> > + .type = IO_THREAD_HOLDER_KIND_QOM_OBJECT,
> > + .u.qom_object.qom_path = mon->iothread_qom_path,
> > + };
> > +
> > + mon->ctx = iothread_ref_and_get_aio_context(mon->iothread,
> > &io_holder);
> > } else {
> > - ctx = qemu_get_aio_context();
> > + mon->ctx = qemu_get_aio_context();
> > }
> > - mon->accept_input_bh = aio_bh_new(ctx, monitor_accept_input, mon);
> > + mon->accept_input_bh = aio_bh_new(mon->ctx, monitor_accept_input, mon);
> > }
> >
> > int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp)
> > diff --git a/monitor/qmp.c b/monitor/qmp.c
> > index aec0315775..e235302fb3 100644
> > --- a/monitor/qmp.c
> > +++ b/monitor/qmp.c
> > @@ -187,7 +187,7 @@ static void monitor_qmp_caps_reset(MonitorQMP *mon)
> > memset(mon->capab_offered, 0, sizeof(mon->capab_offered));
> > memset(mon->capab, 0, sizeof(mon->capab));
> > mon->capab_offered[QMP_CAPABILITY_OOB] =
> > - monitor_requires_iothread(MONITOR(mon));
> > + MONITOR(mon)->iothread != NULL;
> > }
> >
> > static void qmp_request_free(QMPRequest *req)
> > @@ -678,8 +678,8 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
> > MonitorQMP *mon = opaque;
> > GMainContext *context;
> >
> > - assert(monitor_requires_iothread(MONITOR(mon)));
> > - context = iothread_get_g_main_context(mon_iothread);
> > + assert(MONITOR(mon)->iothread);
> > + context = iothread_get_g_main_context(MONITOR(mon)->iothread);
> > assert(context);
> > qemu_chr_fe_set_handlers(&mon->parent_obj.chr, monitor_can_read,
> > monitor_qmp_read, monitor_qmp_event,
> > @@ -717,7 +717,7 @@ static void monitor_qmp_complete(UserCreatable *uc,
> > Error **errp)
> >
> > qemu_chr_fe_set_echo(&mon->parent_obj.chr, true);
> >
> > - if (monitor_requires_iothread(MONITOR(mon))) {
> > + if (MONITOR(mon)->iothread) {
> > /*
> > * Make sure the old iowatch is gone. It's possible when
> > * e.g. the chardev is in client mode, with wait=on.
> > @@ -734,7 +734,7 @@ static void monitor_qmp_complete(UserCreatable *uc,
> > Error **errp)
> > * thread. Schedule a bottom half.
> > */
> > mon->setup_pending = true;
> > - aio_bh_schedule_oneshot(iothread_get_aio_context(mon_iothread),
> > + aio_bh_schedule_oneshot(MONITOR(mon)->ctx,
> > monitor_qmp_setup_handlers_bh, mon);
> > /* The bottom half will add @mon to @mon_list */
> > } else {
> > @@ -788,8 +788,8 @@ static bool monitor_qmp_prepare_delete(UserCreatable
> > *uc, Error **errp)
> > }
> >
> > /* Synchronize with in-flight iothread callbacks. */
> > - if (monitor_requires_iothread(mon)) {
> > - aio_wait_bh_oneshot(iothread_get_aio_context(mon_iothread),
> > + if (mon->iothread) {
> > + aio_wait_bh_oneshot(mon->ctx,
> > monitor_qmp_iothread_quiesce, NULL);
> > }
>
> With the rename
> Reviewed-by: Markus Armbruster <[email protected]>
Thanks
Chen
>
