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()

 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(-)

-- 
2.38.1

Am 26.01.2023 um 18:24 hat Kevin Wolf geschrieben:
> 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.

Thanks for the review, fixed up the missing coroutine_fn in patch 3 (as
pointed out by Emanuele) and applied to the block branch.

Kevin

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(-)
>

On 26.01.23 18:24, Kevin Wolf wrote:
> 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.

Reviewed-by: Hanna Czenczek <hreitz@redhat.com>