Re: [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context

[Emanuele Giuseppe Esposito]

Am 26/01/2023 um 18:24 schrieb Kevin Wolf:
> bdrv_open*() must not be called from coroutine context, amongst others
> because it modifies the block graph. However, some functions - in
> particular all .bdrv_co_create* implementations of image formats - do
> call it from coroutine context. This is already wrong today, but when we
> add locking, it actually becomes visible.
> 
> This series adds no_co_wrapper functions, which are automatically
> generated wrappers that run in coroutine context and use a BH to call
> the wrapped function outside of coroutine context. It then uses these
> wrappers to fix the problematic bdrv_open*() calls.
> 
> Kevin Wolf (13):
>   block-coroutine-wrapper: Introduce no_co_wrapper
>   block: Create no_co_wrappers for open functions
>   luks: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper
>   parallels: Fix .bdrv_co_create(_opts) to open images with
>     no_co_wrapper
>   qcow: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper
>   qcow2: Fix open/create to open images with no_co_wrapper
>   qed: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper
>   vdi: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper
>   vhdx: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper
>   vmdk: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper
>   vpc: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper
>   block: Fix bdrv_co_create_opts_simple() to open images with
>     no_co_wrapper
>   block: Assert non-coroutine context for bdrv_open_inherit()
> 

Apart from a small nitpick in patch 3 where the functions are not marked
as coroutine_fn (but I think this is because BDS callbacks usually don't
have such annotations), looks good to me.

Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

>  include/block/block-common.h                | 14 ++++
>  include/block/block-global-state.h          | 35 ++++++---
>  include/sysemu/block-backend-global-state.h | 21 +++++-
>  block.c                                     | 17 ++---
>  block/crypto.c                              | 10 +--
>  block/parallels.c                           | 10 +--
>  block/qcow.c                                | 10 +--
>  block/qcow2.c                               | 43 +++++------
>  block/qed.c                                 | 10 +--
>  block/vdi.c                                 | 10 +--
>  block/vhdx.c                                | 10 +--
>  block/vmdk.c                                | 22 +++---
>  block/vpc.c                                 | 10 +--
>  scripts/block-coroutine-wrapper.py          | 83 ++++++++++++++++++---
>  block/meson.build                           |  1 +
>  15 files changed, 207 insertions(+), 99 deletions(-)
>