Re: [RFC 07/11] aio: add errp argument to aio_context_setup()

2025-05-28 Thread Eric Blake
On Wed, May 28, 2025 at 03:09:12PM -0400, Stefan Hajnoczi wrote:
> When aio_context_new() -> aio_context_setup() fails at startup it
> doesn't really matter whether errors are returned to the caller or the
> process terminates immediately.
> 
> However, it is not acceptable to terminate when hotplugging --object
> iothread at runtime. Refactor aio_context_setup() so that errors can be
> propagated. The next commit will set errp when fdmon_io_uring_setup()
> fails.
> 
> Suggested-by: Kevin Wolf 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  include/block/aio.h | 3 ++-
>  util/aio-posix.c| 2 +-
>  util/aio-win32.c| 2 +-
>  util/async.c| 7 ++-
>  4 files changed, 10 insertions(+), 4 deletions(-)
>

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org




[RFC 07/11] aio: add errp argument to aio_context_setup()

2025-05-28 Thread Stefan Hajnoczi
When aio_context_new() -> aio_context_setup() fails at startup it
doesn't really matter whether errors are returned to the caller or the
process terminates immediately.

However, it is not acceptable to terminate when hotplugging --object
iothread at runtime. Refactor aio_context_setup() so that errors can be
propagated. The next commit will set errp when fdmon_io_uring_setup()
fails.

Suggested-by: Kevin Wolf 
Signed-off-by: Stefan Hajnoczi 
---
 include/block/aio.h | 3 ++-
 util/aio-posix.c| 2 +-
 util/aio-win32.c| 2 +-
 util/async.c| 7 ++-
 4 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/block/aio.h b/include/block/aio.h
index 2760f308f5..d919d7c8f4 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -718,10 +718,11 @@ void qemu_set_current_aio_context(AioContext *ctx);
 /**
  * aio_context_setup:
  * @ctx: the aio context
+ * @errp: error pointer
  *
  * Initialize the aio context.
  */
-void aio_context_setup(AioContext *ctx);
+void aio_context_setup(AioContext *ctx, Error **errp);
 
 /**
  * aio_context_destroy:
diff --git a/util/aio-posix.c b/util/aio-posix.c
index 5634da7d27..fa047fc7ad 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -711,7 +711,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
 return progress;
 }
 
-void aio_context_setup(AioContext *ctx)
+void aio_context_setup(AioContext *ctx, Error **errp)
 {
 ctx->fdmon_ops = &fdmon_poll_ops;
 ctx->epollfd = -1;
diff --git a/util/aio-win32.c b/util/aio-win32.c
index 34c4074133..4ba401ff92 100644
--- a/util/aio-win32.c
+++ b/util/aio-win32.c
@@ -419,7 +419,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
 return progress;
 }
 
-void aio_context_setup(AioContext *ctx)
+void aio_context_setup(AioContext *ctx, Error **errp)
 {
 }
 
diff --git a/util/async.c b/util/async.c
index bc841eeb4f..bba9622e97 100644
--- a/util/async.c
+++ b/util/async.c
@@ -577,6 +577,7 @@ static void co_schedule_bh_cb(void *opaque)
 
 AioContext *aio_context_new(Error **errp)
 {
+ERRP_GUARD();
 int ret;
 AioContext *ctx;
 
@@ -590,7 +591,11 @@ AioContext *aio_context_new(Error **errp)
 goto fail;
 }
 
-aio_context_setup(ctx);
+aio_context_setup(ctx, errp);
+if (*errp) {
+event_notifier_cleanup(&ctx->notifier);
+goto fail;
+}
 
 g_source_set_can_recurse(&ctx->source, true);
 qemu_lockcnt_init(&ctx->list_lock);
-- 
2.49.0