Re: [PATCH V13 02/15] iothread: introduce holder tracking

[Markus Armbruster <armbru@redhat.com>]

Zhang Chen <zhangckid@gmail.com> writes:

> IOThreads do not record the QOM objects and block exports that use
> them, making runtime introspection and hotplug debugging difficult.
>
> Add:
>
> - holders: a list describing what is holding an iothread
> - iothread_ref(): add a holder to the list
> - iothread_unref(): remove a holder from the list
>
> A holder can identify either a QOM object or a block export. A later
> patch will expose this information through query-iothreads.
>
> Signed-off-by: Zhang Chen <zhangckid@gmail.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

I'm suspending my R-by, see below why.

[...]

> diff --git a/iothread.c b/iothread.c
> index 3558535b40..66dfde626e 100644
> --- a/iothread.c
> +++ b/iothread.c
> @@ -21,10 +21,71 @@
>  #include "system/iothread.h"
>  #include "qapi/error.h"
>  #include "qapi/qapi-commands-misc.h"
> +#include "qapi/clone-visitor.h"
> +#include "qapi/qapi-visit-misc.h"
>  #include "qemu/error-report.h"
>  #include "qemu/rcu.h"
>  #include "qemu/main-loop.h"
>  
> +/* Add a deep copy of @holder to @iothread's list of holders. */
> +void iothread_ref(IOThread *iothread, const IOThreadHolder *holder)
> +{
> +    assert(holder);
> +
> +    QAPI_LIST_PREPEND(iothread->holders, QAPI_CLONE(IOThreadHolder, holder));
> +}
> +
> +static int iothread_holder_compare(const IOThreadHolder *holder_a,
> +                                   const IOThreadHolder *holder_b)
> +{
> +    const char *name_a, *name_b;
> +
> +    if (holder_a->type != holder_b->type) {
> +        return holder_a->type - holder_b->type;

Why did you swap the operands in v13?  Not mentioned in the cover
letter's change log.  I requested the other order in review of v8[*] and
v9, and you complied in v10.  Unless you have a compelling reason, swap
them right back, please.

In general, rocking the boat like this when you're at v13 is
inadvisable.  It risks delaying the series even more and annoying
reviewers.

> +    }
> +
> +    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_EXPORT:
> +        name_a = holder_a->u.block_export.export_id;
> +        name_b = holder_b->u.block_export.export_id;
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +
> +    return strcmp(name_a, name_b);
> +}

[...]

[*] Here's my review of v8:

    Date: Tue, 02 Jun 2026 13:29:19 +0200
    Message-ID: <87tsrl2nao.fsf@pond.sub.org>
    https://lore.kernel.org/qemu-devel/87tsrl2nao.fsf@pond.sub.org