Re: [PATCH V10 06/15] monitor: Update tracking iothread users with holder

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Jul 16, 2026 at 10:40:31PM +0800, Zhang Chen wrote:
> Based on monitor QOM path tracking iothread users with holder.
> Introduce the AioContext in the Monitor struct to avoid repeated calls to
> iothread_get_aio_context() and ensure symmetrical ref/unref during
> monitor lifecycle.
> 
> Signed-off-by: Zhang Chen <zhangckid@gmail.com>
> ---
>  monitor/monitor-internal.h |  3 +++
>  monitor/monitor.c          | 22 ++++++++++++++++++++--
>  monitor/qmp.c              |  5 +++--
>  3 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> index 23829f32f9..caecceec93 100644
> --- a/monitor/monitor-internal.h
> +++ b/monitor/monitor-internal.h
> @@ -153,6 +153,9 @@ struct Monitor {
>      guint out_watch;
>      int mux_out;
>      int reset_seen;
> +
> +    /* iothread context */
> +    AioContext *ctx;
>  };
>  
>  struct MonitorHMPClass {
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index ed195fd97b..688bb85c81 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -573,7 +573,7 @@ void monitor_suspend(Monitor *mon)
>           * 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);
> @@ -668,6 +668,17 @@ void monitor_cleanup(void)
>          qemu_mutex_unlock(&monitor_lock);
>          monitor_flush(mon);
>          qemu_mutex_lock(&monitor_lock);
> +
> +        if (mon_iothread) {
> +            g_autofree char *path = object_get_canonical_path(OBJECT(mon));
> +            const IOThreadHolder io_holder = {
> +                .type = IO_THREAD_HOLDER_KIND_QOM_OBJECT,
> +                .u.qom_object.qom_path = path,
> +            };
> +
> +            iothread_put_aio_context(mon_iothread, &io_holder);
> +            mon->ctx = NULL;
> +        }
>          object_unparent(OBJECT(mon));
>      }
>      qemu_mutex_unlock(&monitor_lock);
> @@ -732,7 +743,14 @@ static void monitor_complete(UserCreatable *uc, Error **errp)
>              mon_iothread = iothread_create("mon_iothread", &error_abort);
>          }
>  
> -        ctx = iothread_get_aio_context(mon_iothread);
> +        g_autofree char *path = object_get_canonical_path(OBJECT(mon));
> +        const IOThreadHolder io_holder = {
> +            .type = IO_THREAD_HOLDER_KIND_QOM_OBJECT,
> +            .u.qom_object.qom_path = path,
> +        };
> +
> +        mon->ctx = iothread_ref_and_get_aio_context(mon_iothread, &io_holder);
> +        ctx = mon->ctx;
>      } else {
>          ctx = qemu_get_aio_context();
>      }

Even though QMP only takes the first branch, I'd be more
comfortable if we set mon->ctx in both branches of this
condition, so we can always assume that mon->ctx is
non-NULL.

> diff --git a/monitor/qmp.c b/monitor/qmp.c
> index 223e0643c2..b16bd77688 100644
> --- a/monitor/qmp.c
> +++ b/monitor/qmp.c
> @@ -732,7 +732,8 @@ 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 {
> @@ -787,7 +788,7 @@ 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),
> +        aio_wait_bh_oneshot(MONITOR(mon)->ctx,
>                              monitor_qmp_iothread_quiesce, NULL);
>      }

...this unconditionally accesses mon->ctx, and so will surely hit
a NULL

>  
> -- 
> 2.49.0
> 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|