Re: [PATCH V14 06/15] monitor: track IOThread users with QOM paths

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3365 bytes --]

On Thu, Sep 03, 2026 at 08:05:14AM +0200, Markus Armbruster wrote:
> Stefan Hajnoczi <stefanha@redhat.com> writes:
> 
> > On Tue, Sep 01, 2026 at 12:00:27AM +0800, Zhang Chen wrote:
> >> 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, using the monitor
> >> QOM path as the holder, and reuse the stored context for later
> >> operations.
> >> 
> >> Release the holder from the Monitor ObjectClass::unparent callback,
> >> while the canonical QOM path is still available. This covers every
> >> object_unparent() path and avoids duplicating the cleanup in
> >> monitor_cleanup() and monitor_qmp_prepare_delete().
> >> 
> >> 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 <zhangckid@gmail.com>
> >> ---
> >>  monitor/monitor-internal.h |  3 +++
> >>  monitor/monitor.c          | 33 ++++++++++++++++++++++++++++-----
> >>  monitor/qmp.c              |  5 +++--
> >>  3 files changed, 34 insertions(+), 7 deletions(-)
> >> 
> >> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> >> index e0a124a317..74ea15c85f 100644
> >> --- a/monitor/monitor-internal.h
> >> +++ b/monitor/monitor-internal.h
> >> @@ -154,6 +154,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 6af09f2f6e..9e3b639eff 100644
> >> --- a/monitor/monitor.c
> >> +++ b/monitor/monitor.c
> >> @@ -81,6 +81,22 @@ int monitor_device_index;
> >>  OBJECT_DEFINE_TYPE_EXTENDED(Monitor, monitor, MONITOR, OBJECT, true,
> >>                              { TYPE_USER_CREATABLE }, {});
> >>  
> >> +static void monitor_unparent(Object *obj)
> >> +{
> >> +    Monitor *mon = MONITOR(obj);
> >> +
> >> +    if (mon->ctx && monitor_requires_iothread(mon)) {
> >> +        g_autofree char *path = object_get_canonical_path(obj);
> >> +        const IOThreadHolder io_holder = {
> >> +            .type = IO_THREAD_HOLDER_KIND_QOM_OBJECT,
> >> +            .u.qom_object.qom_path = path,
> >> +        };
> >> +
> >> +        iothread_unref_and_put_aio_context(mon_iothread, &io_holder);
> >> +        mon->ctx = NULL;
> >
> > mon->accept_input_bh still exits and uses the AioContext at this point

exits -> exists

> > (it is freed in monitor_finalize()). Strictly speaking, this violates
> > the idea of not using the AioContext anymore after calling
> > iothread_unref_and_put_aio_context().
> >
> > This could be a source of bugs, but mon->accept_input_bh won't be
> > scheduled anymore in this code path, so I think we can live with this.
> 
> Worth a comment?

Yes.

> 
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]