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 <eesposit@redhat.com>

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 <kwolf@redhat.com>