Re: [PATCH v4 10/11] block: convert bdrv_create to generated_co_wrapper_simple

[Kevin Wolf]

Am 16.11.2022 um 13:22 hat Emanuele Giuseppe Esposito geschrieben:
> This function is never called in coroutine context, therefore
> instead of manually creating a new coroutine, delegate it to the
> block-coroutine-wrapper script, defining it as g_c_w_simple.
> 
> Signed-off-by: Emanuele Giuseppe Esposito 

At first I thought that "never called in coroutine context" was not
entirely true because of paths like this:

qcow2_co_create() -> bdrv_open_blockdev_ref() -> bdrv_open_inherit() ->
bdrv_append_temp_snapshot() -> bdrv_create().

The only reason why it doesn't happen is that with a BlockdevRef, it's
impossible to get BDRV_O_SNAPSHOT set, so bdrv_append_temp_snapshot()
can't actually be called when you come from bdrv_open_blockdev_ref().

Of course, opening images in a coroutine is likely to already do similar
things. We should probably drop out of coroutine context for bdrv_open
to be safe. In practice, I suspect it might work anyway because nothing
is going to wait on the current coroutine, but maybe better not to rely
on it.

Anyway, not a problem of your patch, it's just something it made me
think of.

> diff --git a/block.c b/block.c
> index 7a4c3eb540..d3e168408a 100644
> --- a/block.c
> +++ b/block.c
> @@ -528,7 +528,7 @@ typedef struct CreateCo {
>  Error *err;
>  } CreateCo;
>  
> -static int coroutine_fn bdrv_co_create(BlockDriver *drv, const char 
> *filename,
> +int coroutine_fn bdrv_co_create(BlockDriver *drv, const char *filename,
> QemuOpts *opts, Error **errp)

The indentation is off now. With this fixed:

Reviewed-by: Kevin Wolf 

[PATCH v4 10/11] block: convert bdrv_create to generated_co_wrapper_simple

[Emanuele Giuseppe Esposito]

This function is never called in coroutine context, therefore
instead of manually creating a new coroutine, delegate it to the
block-coroutine-wrapper script, defining it as g_c_w_simple.

Signed-off-by: Emanuele Giuseppe Esposito 
---
 block.c| 38 +-
 include/block/block-global-state.h | 10 ++--
 2 files changed, 9 insertions(+), 39 deletions(-)

diff --git a/block.c b/block.c
index 7a4c3eb540..d3e168408a 100644
--- a/block.c
+++ b/block.c
@@ -528,7 +528,7 @@ typedef struct CreateCo {
 Error *err;
 } CreateCo;
 
-static int coroutine_fn bdrv_co_create(BlockDriver *drv, const char *filename,
+int coroutine_fn bdrv_co_create(BlockDriver *drv, const char *filename,
QemuOpts *opts, Error **errp)
 {
 int ret;
@@ -555,42 +555,6 @@ static int coroutine_fn bdrv_co_create(BlockDriver *drv, 
const char *filename,
 return ret;
 }
 
-static void coroutine_fn bdrv_create_co_entry(void *opaque)
-{
-CreateCo *cco = opaque;
-GLOBAL_STATE_CODE();
-
-cco->ret = bdrv_co_create(cco->drv, cco->filename, cco->opts, &cco->err);
-}
-
-int bdrv_create(BlockDriver *drv, const char* filename,
-QemuOpts *opts, Error **errp)
-{
-GLOBAL_STATE_CODE();
-
-if (qemu_in_coroutine()) {
-/* Fast-path if already in coroutine context */
-return bdrv_co_create(drv, filename, opts, errp);
-} else {
-Coroutine *co;
-CreateCo cco = {
-.drv = drv,
-.filename = filename,
-.opts = opts,
-.ret = NOT_DONE,
-.err = NULL,
-};
-
-co = qemu_coroutine_create(bdrv_create_co_entry, &cco);
-qemu_coroutine_enter(co);
-while (cco.ret == NOT_DONE) {
-aio_poll(qemu_get_aio_context(), true);
-}
-error_propagate(errp, cco.err);
-return cco.ret;
-}
-}
-
 /**
  * Helper function for bdrv_create_file_fallback(): Resize @blk to at
  * least the given @minimum_size.
diff --git a/include/block/block-global-state.h 
b/include/block/block-global-state.h
index 6f35ed99e3..305336bdb9 100644
--- a/include/block/block-global-state.h
+++ b/include/block/block-global-state.h
@@ -55,8 +55,14 @@ BlockDriver *bdrv_find_protocol(const char *filename,
 bool allow_protocol_prefix,
 Error **errp);
 BlockDriver *bdrv_find_format(const char *format_name);
-int bdrv_create(BlockDriver *drv, const char* filename,
-QemuOpts *opts, Error **errp);
+
+int coroutine_fn bdrv_co_create(BlockDriver *drv, const char* filename,
+QemuOpts *opts, Error **errp);
+int generated_co_wrapper_simple bdrv_create(BlockDriver *drv,
+const char* filename,
+QemuOpts *opts,
+Error **errp);
+
 int coroutine_fn bdrv_create_file(const char *filename, QemuOpts *opts,
   Error **errp);
 
-- 
2.31.1