Re: [RFC 06/11] aio: free AioContext when aio_context_new() fails
On Wed, May 28, 2025 at 04:06:13PM -0500, Eric Blake wrote: > On Wed, May 28, 2025 at 03:09:11PM -0400, Stefan Hajnoczi wrote: > > g_source_destroy() only removes the GSource from the GMainContext it's > > attached to, if any. It does not free it. > > > > Use g_source_unref() instead so that the AioContext (which embeds a > > GSource) is freed. There is no need to call g_source_destroy() in > > aio_context_new() because the GSource isn't attached to a GMainContext > > yet. > > > > aio_ctx_finalize() expects everything to be set up already, so introduce > > the new ctx->initialized boolean and do nothing when called with > > !initialized. This also requires moving aio_context_setup() down after > > event_notifier_init() since aio_ctx_finalize() won't release any > > resources that aio_context_setup() acquired. > > > > Signed-off-by: Stefan Hajnoczi > > --- > > include/block/aio.h | 3 +++ > > util/async.c| 12 ++-- > > 2 files changed, 13 insertions(+), 2 deletions(-) > > > > > +++ b/util/async.c > > @@ -369,6 +369,10 @@ aio_ctx_finalize(GSource *source) > > While you're here, is it worth cleaning up that odd spacing in the > function parameter list? Yes. Stefan signature.asc Description: PGP signature
Re: [RFC 06/11] aio: free AioContext when aio_context_new() fails
On Wed, May 28, 2025 at 03:09:11PM -0400, Stefan Hajnoczi wrote:
> g_source_destroy() only removes the GSource from the GMainContext it's
> attached to, if any. It does not free it.
>
> Use g_source_unref() instead so that the AioContext (which embeds a
> GSource) is freed. There is no need to call g_source_destroy() in
> aio_context_new() because the GSource isn't attached to a GMainContext
> yet.
>
> aio_ctx_finalize() expects everything to be set up already, so introduce
> the new ctx->initialized boolean and do nothing when called with
> !initialized. This also requires moving aio_context_setup() down after
> event_notifier_init() since aio_ctx_finalize() won't release any
> resources that aio_context_setup() acquired.
>
> Signed-off-by: Stefan Hajnoczi
> ---
> include/block/aio.h | 3 +++
> util/async.c| 12 ++--
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> +++ b/util/async.c
> @@ -369,6 +369,10 @@ aio_ctx_finalize(GSource *source)
While you're here, is it worth cleaning up that odd spacing in the
function parameter list?
> QEMUBH *bh;
> unsigned flags;
>
> +if (!ctx->initialized) {
> +return;
> +}
> +
> thread_pool_free_aio(ctx->thread_pool);
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
