On Mon, Jun 29, 2026 at 6:42 PM Markus Armbruster <[email protected]> wrote:
>
> Zhang Chen <[email protected]> writes:
>
> > On Fri, Jun 26, 2026 at 4:46 PM Markus Armbruster <[email protected]> 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.

Make sense, I will change the GList code to QAPI code in the next version.

By the way, do you think it's OK to make this series based on Daniel's
V5 patches
for the monitor QOM part.
https://lists.gnu.org/archive/html/qemu-devel/2026-06/msg06427.html

This allows us to delete some of the code and directly use the monitor as QOM.
https://lists.gnu.org/archive/html/qemu-devel/2026-06/msg07323.html

Thanks
Chen

>

Reply via email to