On Wed, Jun 24, 2026 at 11:51 PM Stefan Hajnoczi <[email protected]> wrote:
>
> On Wed, Jun 24, 2026 at 03:08:36PM +0800, Zhang Chen wrote:
> > Currently, IOThreads do not maintain a record of which devices are
> > associated with them. This makes it difficult to monitor the
> > workload distribution of IOThreads, especially in complex
> > hotplug scenarios involving multiple virtio-blk or virtio-scsi devices.
> >
> > This patch introduces a reference counting and tracking mechanism
> > within the IOThread object:
> >
> > - iothread_ref(): Prepends the device's IOThreadHolder to a list.
> > - iothread_unref(): Searches for the IOThreadHolder using a custom
> > string comparison (g_strcmp0), releases the associated memory
> > upon a successful match.
> > - holders: A GList storing the IOThreadHolder of attached devices
> > for runtime introspection.
> >
> > A later commit will add QMP commands to let management applications
> > query the attachment status of IOThreads.
> >
> > Signed-off-by: Zhang Chen <[email protected]>
> > ---
> > include/system/iothread.h | 5 +++
> > iothread.c | 94 +++++++++++++++++++++++++++++++++++++++
> > qapi/misc.json | 61 +++++++++++++++++++++++++
> > 3 files changed, 160 insertions(+)
>
> Minor comments below. This patch looks good overall:
>
> Reviewed-by: Stefan Hajnoczi <[email protected]>
Thanks~
>
> > diff --git a/include/system/iothread.h b/include/system/iothread.h
> > index a1ef7696cb..b9207ad829 100644
> > --- a/include/system/iothread.h
> > +++ b/include/system/iothread.h
> > @@ -50,6 +50,11 @@ struct IOThread {
> > bool stopping; /* has iothread_stop() been called? */
> > bool running; /* should iothread_run() continue? */
> > int thread_id;
> > + /*
> > + * The list elements are of type IOThreadHolder, which can
> > + * represent either a QOM path or a block node name.
>
> This comment duplicates the IOThreadHolder types and will become
> outdated easily. It might already be outdated (there are 3 types, not
> just the 2 mentioned in the comment). I suggest replacing it with
> something like:
>
> /* List of iothread_ref() holders (elements are of type IOThreadHolder) */
>
OK, Sorry, I forgot to update the comments with the patch version updates.
> > +static int iothread_holder_compare(gconstpointer a, gconstpointer b)
> > +{
> > + const IOThreadHolder *holder_a = a;
> > + const IOThreadHolder *holder_b = b;
> > + const char *name_a, *name_b;
> > +
> > + if (holder_a->type != holder_b->type) {
> > + return -1;
> > + }
> > +
> > + switch (holder_a->type) {
> > + case IO_THREAD_HOLDER_KIND_QOM_OBJECT:
> > + name_a = holder_a->u.qom_object.qom_path;
> > + name_b = holder_b->u.qom_object.qom_path;
> > + break;
> > + case IO_THREAD_HOLDER_KIND_BLOCK_NODE:
> > + name_a = holder_a->u.block_node.node_name;
> > + name_b = holder_b->u.block_node.node_name;
> > + break;
> > + case IO_THREAD_HOLDER_KIND_MONITOR_NAME:
> > + name_a = holder_a->u.monitor_name.monitor_name;
> > + name_b = holder_b->u.monitor_name.monitor_name;
> > + break;
> > + default:
> > + /*
> > + * This should not happen. If it does, name_a/b remains
> > + * NULL and g_strcmp0 will handle it safely.
> > + */
> > + name_a = NULL;
> > + name_b = NULL;
>
> return -1 would be simpler and safer. g_strcmp0(NULL, NULL) returns 0,
> so callers will think they have found a match and proceed with their
> operation - it will likely result in undefined behavior since memory is
> probably corrupted if we got here. It's safer to fail the comparison so
> the caller does not proceed.
OK.
Thanks
Chen
