Convert colo-compare to use the iothread_get_aio_context() and
iothread_put_aio_context() APIs. This ensures that IOThread
references are tracked using the COLO object's canonical QOM path
as the holder ID.
Changes:
- In colo_compare_iothread(), acquire the AioContext with the object
path and pass the context to colo_compare_timer_init() to avoid
redundant reference counting.
- In colo_compare_finalize(), use the object path to release the
IOThread reference.
- Properly manage the lifecycle of the canonical path string with
g_free() to avoid memory leaks.
This refactoring improves IOThread lifecycle traceability and aligns
the code with modern QEMU iothread reference management patterns.
Signed-off-by: Zhang Chen <zhangckid@gmail.com>
---
net/colo-compare.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index c356419d6a..43addb5f00 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -924,10 +924,8 @@ void colo_notify_compares_event(void *opaque, int event, Error **errp)
qemu_mutex_unlock(&colo_compare_mutex);
}
-static void colo_compare_timer_init(CompareState *s)
+static void colo_compare_timer_init(CompareState *s, AioContext *ctx)
{
- AioContext *ctx = iothread_get_aio_context(s->iothread);
-
s->packet_check_timer = aio_timer_new(ctx, QEMU_CLOCK_HOST,
SCALE_MS, check_old_packet_regular,
s);
@@ -968,8 +966,9 @@ static void colo_compare_handle_event(void *opaque)
static void colo_compare_iothread(CompareState *s)
{
- AioContext *ctx = iothread_get_aio_context(s->iothread);
- object_ref(OBJECT(s->iothread));
+ char *path = object_get_canonical_path(OBJECT(s));
+ AioContext *ctx = iothread_get_aio_context(s->iothread, path);
+
s->worker_context = iothread_get_g_main_context(s->iothread);
qemu_chr_fe_set_handlers(&s->chr_pri_in, compare_chr_can_read,
@@ -984,8 +983,10 @@ static void colo_compare_iothread(CompareState *s)
s, s->worker_context, true);
}
- colo_compare_timer_init(s);
+ colo_compare_timer_init(s, ctx);
s->event_bh = aio_bh_new(ctx, colo_compare_handle_event, s);
+
+ g_free(path);
}
static char *compare_get_pri_indev(Object *obj, Error **errp)
@@ -1408,6 +1409,7 @@ static void colo_compare_finalize(Object *obj)
{
CompareState *s = COLO_COMPARE(obj);
CompareState *tmp = NULL;
+ char *path = object_get_canonical_path(OBJECT(s));
qemu_mutex_lock(&colo_compare_mutex);
QTAILQ_FOREACH(tmp, &net_compares, next) {
@@ -1434,11 +1436,17 @@ static void colo_compare_finalize(Object *obj)
qemu_bh_delete(s->event_bh);
- AioContext *ctx = iothread_get_aio_context(s->iothread);
+ /*
+ * Use the device's canonical path as the holder ID to track IOThread
+ * usage and ensure the AioContext remains valid during the device's
+ * lifecycle.
+ */
+ AioContext *ctx = iothread_get_aio_context(s->iothread, NULL);
AIO_WAIT_WHILE(ctx, !s->out_sendco.done);
if (s->notify_dev) {
AIO_WAIT_WHILE(ctx, !s->notify_sendco.done);
}
+ iothread_put_aio_context(s->iothread, path);
/* Release all unhandled packets after compare thead exited */
g_queue_foreach(&s->conn_list, colo_flush_packets, s);
@@ -1456,6 +1464,7 @@ static void colo_compare_finalize(Object *obj)
object_unref(OBJECT(s->iothread));
+ g_free(path);
g_free(s->pri_indev);
g_free(s->sec_indev);
g_free(s->outdev);
--
2.49.0On Thu, Mar 05, 2026 at 10:24:53PM +0800, Zhang Chen wrote:
> @@ -1434,11 +1436,17 @@ static void colo_compare_finalize(Object *obj)
>
> qemu_bh_delete(s->event_bh);
>
> - AioContext *ctx = iothread_get_aio_context(s->iothread);
> + /*
> + * Use the device's canonical path as the holder ID to track IOThread
> + * usage and ensure the AioContext remains valid during the device's
> + * lifecycle.
> + */
> + AioContext *ctx = iothread_get_aio_context(s->iothread, NULL);
Should NULL be path?
> AIO_WAIT_WHILE(ctx, !s->out_sendco.done);
> if (s->notify_dev) {
> AIO_WAIT_WHILE(ctx, !s->notify_sendco.done);
> }
> + iothread_put_aio_context(s->iothread, path);
On Mon, Mar 9, 2026 at 4:44 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> On Thu, Mar 05, 2026 at 10:24:53PM +0800, Zhang Chen wrote:
> > @@ -1434,11 +1436,17 @@ static void colo_compare_finalize(Object *obj)
> >
> > qemu_bh_delete(s->event_bh);
> >
> > - AioContext *ctx = iothread_get_aio_context(s->iothread);
> > + /*
> > + * Use the device's canonical path as the holder ID to track IOThread
> > + * usage and ensure the AioContext remains valid during the device's
> > + * lifecycle.
> > + */
> > + AioContext *ctx = iothread_get_aio_context(s->iothread, NULL);
>
> Should NULL be path?
>
Yes, here code in the finalize function need to waiting for the
coroutine to complete processing
Or do you have any other suggestion?
Thanks
Chen
> > AIO_WAIT_WHILE(ctx, !s->out_sendco.done);
> > if (s->notify_dev) {
> > AIO_WAIT_WHILE(ctx, !s->notify_sendco.done);
> > }
> > + iothread_put_aio_context(s->iothread, path);
On Tue, Mar 10, 2026 at 06:15:16PM +0800, Zhang Chen wrote:
> On Mon, Mar 9, 2026 at 4:44 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > On Thu, Mar 05, 2026 at 10:24:53PM +0800, Zhang Chen wrote:
> > > @@ -1434,11 +1436,17 @@ static void colo_compare_finalize(Object *obj)
> > >
> > > qemu_bh_delete(s->event_bh);
> > >
> > > - AioContext *ctx = iothread_get_aio_context(s->iothread);
> > > + /*
> > > + * Use the device's canonical path as the holder ID to track IOThread
> > > + * usage and ensure the AioContext remains valid during the device's
> > > + * lifecycle.
> > > + */
> > > + AioContext *ctx = iothread_get_aio_context(s->iothread, NULL);
> >
> > Should NULL be path?
> >
>
> Yes, here code in the finalize function need to waiting for the
> coroutine to complete processing
> Or do you have any other suggestion?
I'm not sure I understand. My question was about:
AioContext *ctx = iothread_get_aio_context(s->iothread, NULL);
^^^^
NULL is passed as the argument but this function has a 'path' local
variable. The path variable should be passed instead of NULL.
Stefan
On Thu, Mar 12, 2026 at 1:36 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> On Tue, Mar 10, 2026 at 06:15:16PM +0800, Zhang Chen wrote:
> > On Mon, Mar 9, 2026 at 4:44 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > >
> > > On Thu, Mar 05, 2026 at 10:24:53PM +0800, Zhang Chen wrote:
> > > > @@ -1434,11 +1436,17 @@ static void colo_compare_finalize(Object *obj)
> > > >
> > > > qemu_bh_delete(s->event_bh);
> > > >
> > > > - AioContext *ctx = iothread_get_aio_context(s->iothread);
> > > > + /*
> > > > + * Use the device's canonical path as the holder ID to track IOThread
> > > > + * usage and ensure the AioContext remains valid during the device's
> > > > + * lifecycle.
> > > > + */
> > > > + AioContext *ctx = iothread_get_aio_context(s->iothread, NULL);
> > >
> > > Should NULL be path?
> > >
> >
> > Yes, here code in the finalize function need to waiting for the
> > coroutine to complete processing
> > Or do you have any other suggestion?
>
> I'm not sure I understand. My question was about:
>
> AioContext *ctx = iothread_get_aio_context(s->iothread, NULL);
> ^^^^
>
> NULL is passed as the argument but this function has a 'path' local
> variable. The path variable should be passed instead of NULL.
Let me explain in detail.
colo-compare have two real holders for the iothread,
1. colo_compare_timer_init.
2. worker_context for chardev handlers.
I try to make them use one pair of iothread_get/put.
The real iothread_get with "path" is here:
static void colo_compare_iothread(CompareState *s)
{
- AioContext *ctx = iothread_get_aio_context(s->iothread);
- object_ref(OBJECT(s->iothread));
+ char *path = object_get_canonical_path(OBJECT(s));
+ AioContext *ctx = iothread_get_aio_context(s->iothread, path);
+
The real pairing iothread_put with the "path" is here:
AIO_WAIT_WHILE(ctx, !s->out_sendco.done);
if (s->notify_dev) {
AIO_WAIT_WHILE(ctx, !s->notify_sendco.done);
}
+ iothread_put_aio_context(s->iothread, path);
/* Release all unhandled packets after compare thead exited */
g_queue_foreach(&s->conn_list, colo_flush_packets, s);
Thanks
Chen
>
> Stefan
On Thu, Mar 12, 2026 at 02:31:32PM +0800, Zhang Chen wrote: > On Thu, Mar 12, 2026 at 1:36 PM Stefan Hajnoczi <stefanha@redhat.com> wrote: > > > > On Tue, Mar 10, 2026 at 06:15:16PM +0800, Zhang Chen wrote: > > > On Mon, Mar 9, 2026 at 4:44 PM Stefan Hajnoczi <stefanha@redhat.com> wrote: > > > > > > > > On Thu, Mar 05, 2026 at 10:24:53PM +0800, Zhang Chen wrote: > > > > > @@ -1434,11 +1436,17 @@ static void colo_compare_finalize(Object *obj) > > > > > > > > > > qemu_bh_delete(s->event_bh); > > > > > > > > > > - AioContext *ctx = iothread_get_aio_context(s->iothread); > > > > > + /* > > > > > + * Use the device's canonical path as the holder ID to track IOThread > > > > > + * usage and ensure the AioContext remains valid during the device's > > > > > + * lifecycle. > > > > > + */ > > > > > + AioContext *ctx = iothread_get_aio_context(s->iothread, NULL); > > > > > > > > Should NULL be path? > > > > > > > > > > Yes, here code in the finalize function need to waiting for the > > > coroutine to complete processing > > > Or do you have any other suggestion? > > > > I'm not sure I understand. My question was about: > > > > AioContext *ctx = iothread_get_aio_context(s->iothread, NULL); > > ^^^^ > > > > NULL is passed as the argument but this function has a 'path' local > > variable. The path variable should be passed instead of NULL. > > Let me explain in detail. > colo-compare have two real holders for the iothread, > 1. colo_compare_timer_init. > 2. worker_context for chardev handlers. > > I try to make them use one pair of iothread_get/put. I suggest storing the AioContext pointer in a new CompareState field in colo_compare_iothread() and and then using it in colo_compare_finalize() instead of calling iothread_get_aio_context(s->iothread, NULL). Stefan
On Thu, Mar 12, 2026 at 3:36 PM Stefan Hajnoczi <stefanha@redhat.com> wrote: > > On Thu, Mar 12, 2026 at 02:31:32PM +0800, Zhang Chen wrote: > > On Thu, Mar 12, 2026 at 1:36 PM Stefan Hajnoczi <stefanha@redhat.com> wrote: > > > > > > On Tue, Mar 10, 2026 at 06:15:16PM +0800, Zhang Chen wrote: > > > > On Mon, Mar 9, 2026 at 4:44 PM Stefan Hajnoczi <stefanha@redhat.com> wrote: > > > > > > > > > > On Thu, Mar 05, 2026 at 10:24:53PM +0800, Zhang Chen wrote: > > > > > > @@ -1434,11 +1436,17 @@ static void colo_compare_finalize(Object *obj) > > > > > > > > > > > > qemu_bh_delete(s->event_bh); > > > > > > > > > > > > - AioContext *ctx = iothread_get_aio_context(s->iothread); > > > > > > + /* > > > > > > + * Use the device's canonical path as the holder ID to track IOThread > > > > > > + * usage and ensure the AioContext remains valid during the device's > > > > > > + * lifecycle. > > > > > > + */ > > > > > > + AioContext *ctx = iothread_get_aio_context(s->iothread, NULL); > > > > > > > > > > Should NULL be path? > > > > > > > > > > > > > Yes, here code in the finalize function need to waiting for the > > > > coroutine to complete processing > > > > Or do you have any other suggestion? > > > > > > I'm not sure I understand. My question was about: > > > > > > AioContext *ctx = iothread_get_aio_context(s->iothread, NULL); > > > ^^^^ > > > > > > NULL is passed as the argument but this function has a 'path' local > > > variable. The path variable should be passed instead of NULL. > > > > Let me explain in detail. > > colo-compare have two real holders for the iothread, > > 1. colo_compare_timer_init. > > 2. worker_context for chardev handlers. > > > > I try to make them use one pair of iothread_get/put. > > I suggest storing the AioContext pointer in a new CompareState field in > colo_compare_iothread() and and then using it in colo_compare_finalize() > instead of calling iothread_get_aio_context(s->iothread, NULL). OK, it works and fewer misunderstandings. Thanks Chen > > Stefan
© 2016 - 2026 Red Hat, Inc.