Re: [PATCH V9 02/17] iothread: introduce iothread_ref/unref to track attached devices

[Markus Armbruster <armbru@redhat.com>]

Zhang Chen <zhangckid@gmail.com> writes:

> On Fri, Jun 26, 2026 at 4:46 PM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> Looks like you missed my review of v8.  I'm replaying my review
>> comments.
>
> Oh, my fault, very sorry about that, focused on other patch's comments fixes.

Happens :)

> Thank you for your detailed explanation.
>
> I will address your comments in next version.
> Expect the:
>   IOThreadHolder *h = QAPI_CLONE(IOThreadHolder, holder);
>
> Same as your estimate, this doesn't seem worthwhile.

Replacing hand-written code by QAPI_CLONE() is worth a try.

I additionally described how to avoid the copy entirely instead, but
advised against it.

> And the same issue in the [ PATCH V8 14/15] comments:
> static IOThreadHolderList *iothread_get_holders_list(IOThread *iothread)
>
>> If iothread->holders was a IOThreadHolderList instead of a GList, this
>> loop could be QAPI_CLONE(IOThreadHolderList, iothread->holders) instead.
>
> If we change the GList *holders to the IOThreadHolderList *holders,
> This means that we have to reimplement the  GList related API,
> for example the "g_list_prepend", "g_list_find_custom" and the
> "g_list_delete_link"
>
> This doesn't seem worthwhile.

The replacement for g_list_prepend() would be trivial:

    h->next = iothread->holders;
    iothread->holders = h;

iothread_unref() would turn into simple loop to find and delete the
matching list element.  iothread_get_holders_list()'s similarly simple
loop around QAPI_CLONE() would turn into a single QAPI_CLONE().
Basically the same amount of code, I think.

Your solution already exists, which is a valid argument.

Mine stays within QAPI instead of mixing GList and QAPI.  Also a valid
argument.

Use your judgement.  I'd try to avoid the mixing.