Re: [PATCH v2 5/9] block: distinguish between bdrv_create running in coroutine and not

[Vladimir Sementsov-Ogievskiy]

On 11/4/22 12:56, Emanuele Giuseppe Esposito wrote:
> Call two different functions depending on whether bdrv_create
> is in coroutine or not, following the same pattern as
> generated_co_wrapper functions.
>
> This allows to also call the coroutine function directly,
> without using CreateCo or relying in bdrv_create().

Can we move to auto-generation of bdrv_create(), like for bdrv_check() and 
friends?

> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
>   block.c | 74 ++++++++++++++++++++++++++++-----------------------------
>   1 file changed, 36 insertions(+), 38 deletions(-)
>
> diff --git a/block.c b/block.c
> index d2b2800039..0823563e4d 100644
> --- a/block.c
> +++ b/block.c
> @@ -522,66 +522,64 @@ typedef struct CreateCo {
>       Error *err;
>   } CreateCo;
>   
> -static void coroutine_fn bdrv_create_co_entry(void *opaque)
> +static int coroutine_fn bdrv_co_create(BlockDriver *drv, const char *filename,
> +                                       QemuOpts *opts, Error **errp)
>   {
> -    Error *local_err = NULL;
>       int ret;
> +    GLOBAL_STATE_CODE();
> +    assert(*errp == NULL);
> +
> +    if (!drv->bdrv_co_create_opts) {
> +        error_setg(errp, "Driver '%s' does not support image creation",
> +                   drv->format_name);
> +        return -ENOTSUP;
> +    }
> +
> +    ret = drv->bdrv_co_create_opts(drv, filename, opts, errp);
>   
> +    if (ret < 0 && !*errp) {
> +        error_setg_errno(errp, -ret, "Could not create image");
> +    }
> +
> +    return ret;
> +}
> +
> +static void coroutine_fn bdrv_create_co_entry(void *opaque)
> +{
>       CreateCo *cco = opaque;
> -    assert(cco->drv);
>       GLOBAL_STATE_CODE();
> +    assert(cco->drv);
>   
> -    ret = cco->drv->bdrv_co_create_opts(cco->drv,
> -                                        cco->filename, cco->opts, &local_err);
> -    error_propagate(&cco->err, local_err);
> -    cco->ret = ret;
> +    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)
>   {
> -    int ret;
> -
>       GLOBAL_STATE_CODE();
>   
> -    Coroutine *co;
> -    CreateCo cco = {
> -        .drv = drv,
> -        .filename = g_strdup(filename),
> -        .opts = opts,
> -        .ret = NOT_DONE,
> -        .err = NULL,
> -    };
> -
> -    if (!drv->bdrv_co_create_opts) {
> -        error_setg(errp, "Driver '%s' does not support image creation", 
> drv->format_name);
> -        ret = -ENOTSUP;
> -        goto out;
> -    }
> -
>       if (qemu_in_coroutine()) {
>           /* Fast-path if already in coroutine context */
> -        bdrv_create_co_entry(&cco);
> +        return bdrv_co_create(drv, filename, opts, errp);
>       } else {
> +        Coroutine *co;
> +        CreateCo cco = {
> +            .drv = drv,
> +            .filename = g_strdup(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);
> +        g_free(cco.filename);
> +        return cco.ret;
>       }
> -
> -    ret = cco.ret;
> -    if (ret < 0) {
> -        if (cco.err) {
> -            error_propagate(errp, cco.err);
> -        } else {
> -            error_setg_errno(errp, -ret, "Could not create image");
> -        }
> -    }
> -
> -out:
> -    g_free(cco.filename);
> -    return ret;
>   }
>   
>   /**

--
Best regards,
Vladimir