Re: [PATCH V5 02/13] iothread: introduce iothread_ref/unref to track attached devices

[Stefan Hajnoczi <stefanha@redhat.com>]

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

On Thu, Mar 05, 2026 at 10:24:48PM +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 QOM path to a list.
>   Note: The IOThread takes ownership of the passed 'holder' string.

This is outdated, the patch duplicates the string and the caller still
owns the holder argument:

  iothread->holders = g_list_prepend(iothread->holders, g_strdup(holder));

> - iothread_unref(): Searches for the device path using a custom
>   string comparison (g_strcmp0), releases the associated memory
>   upon a successful match.
> - holders: A GList storing the QOM paths of attached devices
>   for runtime introspection.
> 
> This infrastructure allows management tools and QMP commands to
> query the attachment status of IOThreads.
> 
> Signed-off-by: Zhang Chen <zhangckid@gmail.com>
> ---
>  include/system/iothread.h |  1 +
>  iothread.c                | 28 ++++++++++++++++++++++++++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/include/system/iothread.h b/include/system/iothread.h
> index e26d13c6c7..21a76bd70d 100644
> --- a/include/system/iothread.h
> +++ b/include/system/iothread.h
> @@ -33,6 +33,7 @@ struct IOThread {
>      bool stopping;              /* has iothread_stop() been called? */
>      bool running;               /* should iothread_run() continue? */
>      int thread_id;
> +    GList *holders;             /* an array of QOM paths for attached devices */
>  
>      /* AioContext poll parameters */
>      int64_t poll_max_ns;
> diff --git a/iothread.c b/iothread.c
> index caf68e0764..80a8cf4b32 100644
> --- a/iothread.c
> +++ b/iothread.c
> @@ -36,6 +36,34 @@
>  #define IOTHREAD_POLL_MAX_NS_DEFAULT 0ULL
>  #endif
>  
> +/*
> + * Add holder device path to the list.
> + */
> +static void iothread_ref(IOThread *iothread, const char *holder)
> +{
> +    iothread->holders = g_list_prepend(iothread->holders, g_strdup(holder));
> +}
> +
> +/*
> + * Delete holder device path from the list.
> + */
> +static void iothread_unref(IOThread *iothread, const char *holder)
> +{
> +    if (iothread->holders) {

g_list_find_customer(NULL, ...) returns NULL rather than crashing, so
there is no need for if (iothread->holders).

> +        GList *link = g_list_find_custom(iothread->holders, holder,
> +                                         (GCompareFunc)g_strcmp0);
> +
> +        if (link) {

assert(link) is usually used in QEMU instead. If the reference has
already been released then the program is in an invalid state and it's
not safe to continue running.

Please also add assert(iothread->holders == NULL) to
iothread_instance_finalize() so that leaked references are caught.

> +            g_free(link->data);
> +            iothread->holders = g_list_delete_link(iothread->holders, link);
> +        } else {
> +            error_report("iothread_unref can't find the holder %s", holder);
> +        }
> +    } else {
> +        error_report("iohtread_unref iothread is not held by any devices");

iohtread -> iothread

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