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]> > 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) */ > +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.
signature.asc
Description: PGP signature
