* [PATCH 01/13] block-coroutine-wrapper: Introduce no_co_wrapper
2023-01-26 17:24 [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context Kevin Wolf
@ 2023-01-26 17:24 ` Kevin Wolf
2023-01-26 17:24 ` [PATCH 02/13] block: Create no_co_wrappers for open functions Kevin Wolf
` (14 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2023-01-26 17:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, hreitz, eesposit, pbonzini, qemu-devel
Some functions must not be called from coroutine context. The common
pattern to use them anyway from a coroutine is running them in a BH and
letting the calling coroutine yield to be woken up when the BH is
completed.
Instead of manually writing such wrappers, add support for generating
them to block-coroutine-wrapper.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/block/block-common.h | 14 +++++
scripts/block-coroutine-wrapper.py | 83 ++++++++++++++++++++++++++----
2 files changed, 86 insertions(+), 11 deletions(-)
diff --git a/include/block/block-common.h b/include/block/block-common.h
index 469300fe8d..b5122ef8ab 100644
--- a/include/block/block-common.h
+++ b/include/block/block-common.h
@@ -54,6 +54,20 @@
#define co_wrapper_bdrv_rdlock no_coroutine_fn
#define co_wrapper_mixed_bdrv_rdlock no_coroutine_fn coroutine_mixed_fn
+/*
+ * no_co_wrapper: Function specifier used by block-coroutine-wrapper.py
+ *
+ * Function specifier which does nothing but mark functions to be generated by
+ * scripts/block-coroutine-wrapper.py.
+ *
+ * A no_co_wrapper function declaration creates a coroutine_fn wrapper around
+ * functions that must not be called in coroutine context. It achieves this by
+ * scheduling a BH in the bottom half that runs the respective non-coroutine
+ * function. The coroutine yields after scheduling the BH and is reentered when
+ * the wrapped function returns.
+ */
+#define no_co_wrapper
+
#include "block/blockjob.h"
/* block.c */
diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py
index e82b648127..60e9b3107c 100644
--- a/scripts/block-coroutine-wrapper.py
+++ b/scripts/block-coroutine-wrapper.py
@@ -63,8 +63,8 @@ def __init__(self, param_decl: str) -> None:
class FuncDecl:
- def __init__(self, return_type: str, name: str, args: str,
- variant: str) -> None:
+ def __init__(self, wrapper_type: str, return_type: str, name: str,
+ args: str, variant: str) -> None:
self.return_type = return_type.strip()
self.name = name.strip()
self.struct_name = snake_to_camel(self.name)
@@ -72,8 +72,21 @@ def __init__(self, return_type: str, name: str, args: str,
self.create_only_co = 'mixed' not in variant
self.graph_rdlock = 'bdrv_rdlock' in variant
- subsystem, subname = self.name.split('_', 1)
- self.co_name = f'{subsystem}_co_{subname}'
+ self.wrapper_type = wrapper_type
+
+ if wrapper_type == 'co':
+ subsystem, subname = self.name.split('_', 1)
+ self.target_name = f'{subsystem}_co_{subname}'
+ else:
+ assert wrapper_type == 'no_co'
+ subsystem, co_infix, subname = self.name.split('_', 2)
+ if co_infix != 'co':
+ raise ValueError(f"Invalid no_co function name: {self.name}")
+ if not self.create_only_co:
+ raise ValueError(f"no_co function can't be mixed: {self.name}")
+ if self.graph_rdlock:
+ raise ValueError(f"no_co function can't be rdlock: {self.name}")
+ self.target_name = f'{subsystem}_{subname}'
t = self.args[0].type
if t == 'BlockDriverState *':
@@ -105,7 +118,8 @@ def gen_block(self, format: str) -> str:
# Match wrappers declared with a co_wrapper mark
func_decl_re = re.compile(r'^(?P<return_type>[a-zA-Z][a-zA-Z0-9_]* [\*]?)'
- r'\s*co_wrapper'
+ r'(\s*coroutine_fn)?'
+ r'\s*(?P<wrapper_type>(no_)?co)_wrapper'
r'(?P<variant>(_[a-z][a-z0-9_]*)?)\s*'
r'(?P<wrapper_name>[a-z][a-z0-9_]*)'
r'\((?P<args>[^)]*)\);$', re.MULTILINE)
@@ -113,7 +127,8 @@ def gen_block(self, format: str) -> str:
def func_decl_iter(text: str) -> Iterator:
for m in func_decl_re.finditer(text):
- yield FuncDecl(return_type=m.group('return_type'),
+ yield FuncDecl(wrapper_type=m.group('wrapper_type'),
+ return_type=m.group('return_type'),
name=m.group('wrapper_name'),
args=m.group('args'),
variant=m.group('variant'))
@@ -133,7 +148,7 @@ def create_mixed_wrapper(func: FuncDecl) -> str:
"""
Checks if we are already in coroutine
"""
- name = func.co_name
+ name = func.target_name
struct_name = func.struct_name
graph_assume_lock = 'assume_graph_lock();' if func.graph_rdlock else ''
@@ -163,7 +178,7 @@ def create_co_wrapper(func: FuncDecl) -> str:
"""
Assumes we are not in coroutine, and creates one
"""
- name = func.co_name
+ name = func.target_name
struct_name = func.struct_name
return f"""\
{func.return_type} {func.name}({ func.gen_list('{decl}') })
@@ -183,10 +198,11 @@ def create_co_wrapper(func: FuncDecl) -> str:
}}"""
-def gen_wrapper(func: FuncDecl) -> str:
+def gen_co_wrapper(func: FuncDecl) -> str:
assert not '_co_' in func.name
+ assert func.wrapper_type == 'co'
- name = func.co_name
+ name = func.target_name
struct_name = func.struct_name
graph_lock=''
@@ -225,11 +241,56 @@ def gen_wrapper(func: FuncDecl) -> str:
{creation_function(func)}"""
+def gen_no_co_wrapper(func: FuncDecl) -> str:
+ assert '_co_' in func.name
+ assert func.wrapper_type == 'no_co'
+
+ name = func.target_name
+ struct_name = func.struct_name
+
+ return f"""\
+/*
+ * Wrappers for {name}
+ */
+
+typedef struct {struct_name} {{
+ Coroutine *co;
+ {func.return_field}
+{ func.gen_block(' {decl};') }
+}} {struct_name};
+
+static void {name}_bh(void *opaque)
+{{
+ {struct_name} *s = opaque;
+
+ {func.get_result}{name}({ func.gen_list('s->{name}') });
+
+ aio_co_wake(s->co);
+}}
+
+{func.return_type} coroutine_fn {func.name}({ func.gen_list('{decl}') })
+{{
+ {struct_name} s = {{
+ .co = qemu_coroutine_self(),
+{ func.gen_block(' .{name} = {name},') }
+ }};
+ assert(qemu_in_coroutine());
+
+ aio_bh_schedule_oneshot(qemu_get_aio_context(), {name}_bh, &s);
+ qemu_coroutine_yield();
+
+ {func.ret}
+}}"""
+
+
def gen_wrappers(input_code: str) -> str:
res = ''
for func in func_decl_iter(input_code):
res += '\n\n\n'
- res += gen_wrapper(func)
+ if func.wrapper_type == 'co':
+ res += gen_co_wrapper(func)
+ else:
+ res += gen_no_co_wrapper(func)
return res
--
2.38.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 02/13] block: Create no_co_wrappers for open functions
2023-01-26 17:24 [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context Kevin Wolf
2023-01-26 17:24 ` [PATCH 01/13] block-coroutine-wrapper: Introduce no_co_wrapper Kevin Wolf
@ 2023-01-26 17:24 ` Kevin Wolf
2023-01-26 17:24 ` [PATCH 03/13] luks: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper Kevin Wolf
` (13 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2023-01-26 17:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, hreitz, eesposit, pbonzini, qemu-devel
Images can't be opened in coroutine context because opening needs to
change the block graph. Add no_co_wrappers so that coroutines have a
simple way of opening images in a BH instead.
At the same time, mark the wrapped functions as no_coroutine_fn.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/block/block-global-state.h | 35 +++++++++++++++------
include/sysemu/block-backend-global-state.h | 21 ++++++++++---
block/meson.build | 1 +
3 files changed, 44 insertions(+), 13 deletions(-)
diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h
index a38f86dc15..447176414e 100644
--- a/include/block/block-global-state.h
+++ b/include/block/block-global-state.h
@@ -77,16 +77,26 @@ BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *node_options,
int flags, Error **errp);
int bdrv_drop_filter(BlockDriverState *bs, Error **errp);
-BdrvChild *bdrv_open_child(const char *filename,
- QDict *options, const char *bdref_key,
- BlockDriverState *parent,
- const BdrvChildClass *child_class,
- BdrvChildRole child_role,
- bool allow_none, Error **errp);
+BdrvChild * no_coroutine_fn
+bdrv_open_child(const char *filename, QDict *options, const char *bdref_key,
+ BlockDriverState *parent, const BdrvChildClass *child_class,
+ BdrvChildRole child_role, bool allow_none, Error **errp);
+
+BdrvChild * coroutine_fn no_co_wrapper
+bdrv_co_open_child(const char *filename, QDict *options, const char *bdref_key,
+ BlockDriverState *parent, const BdrvChildClass *child_class,
+ BdrvChildRole child_role, bool allow_none, Error **errp);
+
int bdrv_open_file_child(const char *filename,
QDict *options, const char *bdref_key,
BlockDriverState *parent, Error **errp);
-BlockDriverState *bdrv_open_blockdev_ref(BlockdevRef *ref, Error **errp);
+
+BlockDriverState * no_coroutine_fn
+bdrv_open_blockdev_ref(BlockdevRef *ref, Error **errp);
+
+BlockDriverState * coroutine_fn no_co_wrapper
+bdrv_co_open_blockdev_ref(BlockdevRef *ref, Error **errp);
+
int bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
Error **errp);
int bdrv_set_backing_hd_drained(BlockDriverState *bs,
@@ -94,8 +104,15 @@ int bdrv_set_backing_hd_drained(BlockDriverState *bs,
Error **errp);
int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
const char *bdref_key, Error **errp);
-BlockDriverState *bdrv_open(const char *filename, const char *reference,
- QDict *options, int flags, Error **errp);
+
+BlockDriverState * no_coroutine_fn
+bdrv_open(const char *filename, const char *reference, QDict *options,
+ int flags, Error **errp);
+
+BlockDriverState * coroutine_fn no_co_wrapper
+bdrv_co_open(const char *filename, const char *reference,
+ QDict *options, int flags, Error **errp);
+
BlockDriverState *bdrv_new_open_driver_opts(BlockDriver *drv,
const char *node_name,
QDict *options, int flags,
diff --git a/include/sysemu/block-backend-global-state.h b/include/sysemu/block-backend-global-state.h
index 6858e39cb6..2b6d27db7c 100644
--- a/include/sysemu/block-backend-global-state.h
+++ b/include/sysemu/block-backend-global-state.h
@@ -23,10 +23,23 @@
*/
BlockBackend *blk_new(AioContext *ctx, uint64_t perm, uint64_t shared_perm);
-BlockBackend *blk_new_with_bs(BlockDriverState *bs, uint64_t perm,
- uint64_t shared_perm, Error **errp);
-BlockBackend *blk_new_open(const char *filename, const char *reference,
- QDict *options, int flags, Error **errp);
+
+BlockBackend * no_coroutine_fn
+blk_new_with_bs(BlockDriverState *bs, uint64_t perm, uint64_t shared_perm,
+ Error **errp);
+
+BlockBackend * coroutine_fn no_co_wrapper
+blk_co_new_with_bs(BlockDriverState *bs, uint64_t perm, uint64_t shared_perm,
+ Error **errp);
+
+BlockBackend * no_coroutine_fn
+blk_new_open(const char *filename, const char *reference, QDict *options,
+ int flags, Error **errp);
+
+BlockBackend * coroutine_fn no_co_wrapper
+blk_co_new_open(const char *filename, const char *reference, QDict *options,
+ int flags, Error **errp);
+
int blk_get_refcnt(BlockBackend *blk);
void blk_ref(BlockBackend *blk);
void blk_unref(BlockBackend *blk);
diff --git a/block/meson.build b/block/meson.build
index 3662852dc2..382bec0e7d 100644
--- a/block/meson.build
+++ b/block/meson.build
@@ -141,6 +141,7 @@ block_gen_c = custom_target('block-gen.c',
'../include/block/dirty-bitmap.h',
'../include/block/block_int-io.h',
'../include/block/block-global-state.h',
+ '../include/sysemu/block-backend-global-state.h',
'../include/sysemu/block-backend-io.h',
'coroutines.h'
),
--
2.38.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 03/13] luks: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper
2023-01-26 17:24 [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context Kevin Wolf
2023-01-26 17:24 ` [PATCH 01/13] block-coroutine-wrapper: Introduce no_co_wrapper Kevin Wolf
2023-01-26 17:24 ` [PATCH 02/13] block: Create no_co_wrappers for open functions Kevin Wolf
@ 2023-01-26 17:24 ` Kevin Wolf
2023-01-26 17:24 ` [PATCH 04/13] parallels: " Kevin Wolf
` (12 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2023-01-26 17:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, hreitz, eesposit, pbonzini, qemu-devel
.bdrv_co_create implementations run in a coroutine. Therefore they are
not allowed to open images directly. Fix the calls to use the
corresponding no_co_wrappers instead.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/crypto.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/block/crypto.c b/block/crypto.c
index b70cec97c7..9c477397c7 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -325,8 +325,8 @@ static int block_crypto_co_create_generic(BlockDriverState *bs,
QCryptoBlock *crypto = NULL;
struct BlockCryptoCreateData data;
- blk = blk_new_with_bs(bs, BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL,
- errp);
+ blk = blk_co_new_with_bs(bs, BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL,
+ errp);
if (!blk) {
ret = -EPERM;
goto cleanup;
@@ -639,7 +639,7 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
assert(create_options->driver == BLOCKDEV_DRIVER_LUKS);
luks_opts = &create_options->u.luks;
- bs = bdrv_open_blockdev_ref(luks_opts->file, errp);
+ bs = bdrv_co_open_blockdev_ref(luks_opts->file, errp);
if (bs == NULL) {
return -EIO;
}
@@ -708,8 +708,8 @@ static int coroutine_fn block_crypto_co_create_opts_luks(BlockDriver *drv,
goto fail;
}
- bs = bdrv_open(filename, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
+ bs = bdrv_co_open(filename, NULL, NULL,
+ BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
if (!bs) {
ret = -EINVAL;
goto fail;
--
2.38.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 04/13] parallels: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper
2023-01-26 17:24 [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context Kevin Wolf
` (2 preceding siblings ...)
2023-01-26 17:24 ` [PATCH 03/13] luks: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper Kevin Wolf
@ 2023-01-26 17:24 ` Kevin Wolf
2023-01-26 17:24 ` [PATCH 05/13] qcow: " Kevin Wolf
` (11 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2023-01-26 17:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, hreitz, eesposit, pbonzini, qemu-devel
.bdrv_co_create implementations run in a coroutine. Therefore they are
not allowed to open images directly. Fix the calls to use the
corresponding no_co_wrappers instead.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/parallels.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index bbea2f2221..d4378e09de 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -565,13 +565,13 @@ static int coroutine_fn parallels_co_create(BlockdevCreateOptions* opts,
}
/* Create BlockBackend to write to the image */
- bs = bdrv_open_blockdev_ref(parallels_opts->file, errp);
+ bs = bdrv_co_open_blockdev_ref(parallels_opts->file, errp);
if (bs == NULL) {
return -EIO;
}
- blk = blk_new_with_bs(bs, BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL,
- errp);
+ blk = blk_co_new_with_bs(bs, BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL,
+ errp);
if (!blk) {
ret = -EPERM;
goto out;
@@ -651,8 +651,8 @@ static int coroutine_fn parallels_co_create_opts(BlockDriver *drv,
goto done;
}
- bs = bdrv_open(filename, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
+ bs = bdrv_co_open(filename, NULL, NULL,
+ BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
if (bs == NULL) {
ret = -EIO;
goto done;
--
2.38.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 05/13] qcow: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper
2023-01-26 17:24 [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context Kevin Wolf
` (3 preceding siblings ...)
2023-01-26 17:24 ` [PATCH 04/13] parallels: " Kevin Wolf
@ 2023-01-26 17:24 ` Kevin Wolf
2023-01-26 17:24 ` [PATCH 06/13] qcow2: Fix open/create " Kevin Wolf
` (10 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2023-01-26 17:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, hreitz, eesposit, pbonzini, qemu-devel
.bdrv_co_create implementations run in a coroutine. Therefore they are
not allowed to open images directly. Fix the calls to use the
corresponding no_co_wrappers instead.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/qcow.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/block/qcow.c b/block/qcow.c
index 5f0801f545..20c53b447b 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -833,13 +833,13 @@ static int coroutine_fn qcow_co_create(BlockdevCreateOptions *opts,
}
/* Create BlockBackend to write to the image */
- bs = bdrv_open_blockdev_ref(qcow_opts->file, errp);
+ bs = bdrv_co_open_blockdev_ref(qcow_opts->file, errp);
if (bs == NULL) {
return -EIO;
}
- qcow_blk = blk_new_with_bs(bs, BLK_PERM_WRITE | BLK_PERM_RESIZE,
- BLK_PERM_ALL, errp);
+ qcow_blk = blk_co_new_with_bs(bs, BLK_PERM_WRITE | BLK_PERM_RESIZE,
+ BLK_PERM_ALL, errp);
if (!qcow_blk) {
ret = -EPERM;
goto exit;
@@ -978,8 +978,8 @@ static int coroutine_fn qcow_co_create_opts(BlockDriver *drv,
goto fail;
}
- bs = bdrv_open(filename, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
+ bs = bdrv_co_open(filename, NULL, NULL,
+ BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
if (bs == NULL) {
ret = -EIO;
goto fail;
--
2.38.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 06/13] qcow2: Fix open/create to open images with no_co_wrapper
2023-01-26 17:24 [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context Kevin Wolf
` (4 preceding siblings ...)
2023-01-26 17:24 ` [PATCH 05/13] qcow: " Kevin Wolf
@ 2023-01-26 17:24 ` Kevin Wolf
2023-01-26 17:24 ` [PATCH 07/13] qed: Fix .bdrv_co_create(_opts) " Kevin Wolf
` (9 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2023-01-26 17:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, hreitz, eesposit, pbonzini, qemu-devel
.bdrv_co_create implementations run in a coroutine, as does
qcow2_do_open(). Therefore they are not allowed to open images directly.
Fix the calls to use the corresponding no_co_wrappers instead.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/qcow2.c | 43 ++++++++++++++++++++++---------------------
1 file changed, 22 insertions(+), 21 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 21aa4c6b7a..ee0e5b45cc 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1617,9 +1617,9 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
if (open_data_file) {
/* Open external data file */
- s->data_file = bdrv_open_child(NULL, options, "data-file", bs,
- &child_of_bds, BDRV_CHILD_DATA,
- true, errp);
+ s->data_file = bdrv_co_open_child(NULL, options, "data-file", bs,
+ &child_of_bds, BDRV_CHILD_DATA,
+ true, errp);
if (*errp) {
ret = -EINVAL;
goto fail;
@@ -1627,9 +1627,10 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
if (s->incompatible_features & QCOW2_INCOMPAT_DATA_FILE) {
if (!s->data_file && s->image_data_file) {
- s->data_file = bdrv_open_child(s->image_data_file, options,
- "data-file", bs, &child_of_bds,
- BDRV_CHILD_DATA, false, errp);
+ s->data_file = bdrv_co_open_child(s->image_data_file, options,
+ "data-file", bs,
+ &child_of_bds,
+ BDRV_CHILD_DATA, false, errp);
if (!s->data_file) {
ret = -EINVAL;
goto fail;
@@ -3454,7 +3455,7 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
assert(create_options->driver == BLOCKDEV_DRIVER_QCOW2);
qcow2_opts = &create_options->u.qcow2;
- bs = bdrv_open_blockdev_ref(qcow2_opts->file, errp);
+ bs = bdrv_co_open_blockdev_ref(qcow2_opts->file, errp);
if (bs == NULL) {
return -EIO;
}
@@ -3596,7 +3597,7 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
ret = -EINVAL;
goto out;
}
- data_bs = bdrv_open_blockdev_ref(qcow2_opts->data_file, errp);
+ data_bs = bdrv_co_open_blockdev_ref(qcow2_opts->data_file, errp);
if (data_bs == NULL) {
ret = -EIO;
goto out;
@@ -3629,8 +3630,8 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
}
/* Create BlockBackend to write to the image */
- blk = blk_new_with_bs(bs, BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL,
- errp);
+ blk = blk_co_new_with_bs(bs, BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL,
+ errp);
if (!blk) {
ret = -EPERM;
goto out;
@@ -3712,9 +3713,9 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
if (data_bs) {
qdict_put_str(options, "data-file", data_bs->node_name);
}
- blk = blk_new_open(NULL, NULL, options,
- BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_NO_FLUSH,
- errp);
+ blk = blk_co_new_open(NULL, NULL, options,
+ BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_NO_FLUSH,
+ errp);
if (blk == NULL) {
ret = -EIO;
goto out;
@@ -3793,9 +3794,9 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
if (data_bs) {
qdict_put_str(options, "data-file", data_bs->node_name);
}
- blk = blk_new_open(NULL, NULL, options,
- BDRV_O_RDWR | BDRV_O_NO_BACKING | BDRV_O_NO_IO,
- errp);
+ blk = blk_co_new_open(NULL, NULL, options,
+ BDRV_O_RDWR | BDRV_O_NO_BACKING | BDRV_O_NO_IO,
+ errp);
if (blk == NULL) {
ret = -EIO;
goto out;
@@ -3877,8 +3878,8 @@ static int coroutine_fn qcow2_co_create_opts(BlockDriver *drv,
goto finish;
}
- bs = bdrv_open(filename, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
+ bs = bdrv_co_open(filename, NULL, NULL,
+ BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
if (bs == NULL) {
ret = -EIO;
goto finish;
@@ -3892,9 +3893,9 @@ static int coroutine_fn qcow2_co_create_opts(BlockDriver *drv,
goto finish;
}
- data_bs = bdrv_open(val, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL,
- errp);
+ data_bs = bdrv_co_open(val, NULL, NULL,
+ BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL,
+ errp);
if (data_bs == NULL) {
ret = -EIO;
goto finish;
--
2.38.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 07/13] qed: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper
2023-01-26 17:24 [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context Kevin Wolf
` (5 preceding siblings ...)
2023-01-26 17:24 ` [PATCH 06/13] qcow2: Fix open/create " Kevin Wolf
@ 2023-01-26 17:24 ` Kevin Wolf
2023-01-26 17:24 ` [PATCH 08/13] vdi: " Kevin Wolf
` (8 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2023-01-26 17:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, hreitz, eesposit, pbonzini, qemu-devel
.bdrv_co_create implementations run in a coroutine. Therefore they are
not allowed to open images directly. Fix the calls to use the
corresponding no_co_wrappers instead.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/qed.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/block/qed.c b/block/qed.c
index 4473465bba..175a46c67b 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -676,13 +676,13 @@ static int coroutine_fn bdrv_qed_co_create(BlockdevCreateOptions *opts,
}
/* Create BlockBackend to write to the image */
- bs = bdrv_open_blockdev_ref(qed_opts->file, errp);
+ bs = bdrv_co_open_blockdev_ref(qed_opts->file, errp);
if (bs == NULL) {
return -EIO;
}
- blk = blk_new_with_bs(bs, BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL,
- errp);
+ blk = blk_co_new_with_bs(bs, BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL,
+ errp);
if (!blk) {
ret = -EPERM;
goto out;
@@ -783,8 +783,8 @@ static int coroutine_fn bdrv_qed_co_create_opts(BlockDriver *drv,
goto fail;
}
- bs = bdrv_open(filename, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
+ bs = bdrv_co_open(filename, NULL, NULL,
+ BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
if (bs == NULL) {
ret = -EIO;
goto fail;
--
2.38.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 08/13] vdi: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper
2023-01-26 17:24 [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context Kevin Wolf
` (6 preceding siblings ...)
2023-01-26 17:24 ` [PATCH 07/13] qed: Fix .bdrv_co_create(_opts) " Kevin Wolf
@ 2023-01-26 17:24 ` Kevin Wolf
2023-01-26 17:24 ` [PATCH 09/13] vhdx: " Kevin Wolf
` (7 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2023-01-26 17:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, hreitz, eesposit, pbonzini, qemu-devel
.bdrv_co_create implementations run in a coroutine. Therefore they are
not allowed to open images directly. Fix the calls to use the
corresponding no_co_wrappers instead.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/vdi.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/block/vdi.c b/block/vdi.c
index 9c8736b26f..27db67d493 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -800,14 +800,14 @@ static int coroutine_fn vdi_co_do_create(BlockdevCreateOptions *create_options,
}
/* Create BlockBackend to write to the image */
- bs_file = bdrv_open_blockdev_ref(vdi_opts->file, errp);
+ bs_file = bdrv_co_open_blockdev_ref(vdi_opts->file, errp);
if (!bs_file) {
ret = -EIO;
goto exit;
}
- blk = blk_new_with_bs(bs_file, BLK_PERM_WRITE | BLK_PERM_RESIZE,
- BLK_PERM_ALL, errp);
+ blk = blk_co_new_with_bs(bs_file, BLK_PERM_WRITE | BLK_PERM_RESIZE,
+ BLK_PERM_ALL, errp);
if (!blk) {
ret = -EPERM;
goto exit;
@@ -940,8 +940,8 @@ static int coroutine_fn vdi_co_create_opts(BlockDriver *drv,
goto done;
}
- bs_file = bdrv_open(filename, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
+ bs_file = bdrv_co_open(filename, NULL, NULL,
+ BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
if (!bs_file) {
ret = -EIO;
goto done;
--
2.38.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 09/13] vhdx: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper
2023-01-26 17:24 [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context Kevin Wolf
` (7 preceding siblings ...)
2023-01-26 17:24 ` [PATCH 08/13] vdi: " Kevin Wolf
@ 2023-01-26 17:24 ` Kevin Wolf
2023-01-26 17:24 ` [PATCH 10/13] vmdk: " Kevin Wolf
` (6 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2023-01-26 17:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, hreitz, eesposit, pbonzini, qemu-devel
.bdrv_co_create implementations run in a coroutine. Therefore they are
not allowed to open images directly. Fix the calls to use the
corresponding no_co_wrappers instead.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/vhdx.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/block/vhdx.c b/block/vhdx.c
index ef1f65d917..59fbdb413b 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1991,13 +1991,13 @@ static int coroutine_fn vhdx_co_create(BlockdevCreateOptions *opts,
}
/* Create BlockBackend to write to the image */
- bs = bdrv_open_blockdev_ref(vhdx_opts->file, errp);
+ bs = bdrv_co_open_blockdev_ref(vhdx_opts->file, errp);
if (bs == NULL) {
return -EIO;
}
- blk = blk_new_with_bs(bs, BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL,
- errp);
+ blk = blk_co_new_with_bs(bs, BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL,
+ errp);
if (!blk) {
ret = -EPERM;
goto delete_and_exit;
@@ -2090,8 +2090,8 @@ static int coroutine_fn vhdx_co_create_opts(BlockDriver *drv,
goto fail;
}
- bs = bdrv_open(filename, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
+ bs = bdrv_co_open(filename, NULL, NULL,
+ BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
if (bs == NULL) {
ret = -EIO;
goto fail;
--
2.38.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 10/13] vmdk: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper
2023-01-26 17:24 [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context Kevin Wolf
` (8 preceding siblings ...)
2023-01-26 17:24 ` [PATCH 09/13] vhdx: " Kevin Wolf
@ 2023-01-26 17:24 ` Kevin Wolf
2023-01-26 17:24 ` [PATCH 11/13] vpc: " Kevin Wolf
` (5 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2023-01-26 17:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, hreitz, eesposit, pbonzini, qemu-devel
.bdrv_co_create implementations run in a coroutine. Therefore they are
not allowed to open images directly. Fix the calls to use the
corresponding no_co_wrappers instead.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/vmdk.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 5b0eae877e..171c9272ca 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2299,9 +2299,9 @@ static int coroutine_fn vmdk_create_extent(const char *filename,
goto exit;
}
- blk = blk_new_open(filename, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL,
- errp);
+ blk = blk_co_new_open(filename, NULL, NULL,
+ BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL,
+ errp);
if (blk == NULL) {
ret = -EIO;
goto exit;
@@ -2518,8 +2518,8 @@ static int coroutine_fn vmdk_co_do_create(int64_t size,
}
assert(full_backing);
- backing = blk_new_open(full_backing, NULL, NULL,
- BDRV_O_NO_BACKING, errp);
+ backing = blk_co_new_open(full_backing, NULL, NULL,
+ BDRV_O_NO_BACKING, errp);
g_free(full_backing);
if (backing == NULL) {
ret = -EIO;
@@ -2781,7 +2781,7 @@ static BlockBackend * coroutine_fn vmdk_co_create_cb(int64_t size, int idx,
BlockdevCreateOptionsVmdk *opts = opaque;
if (idx == 0) {
- bs = bdrv_open_blockdev_ref(opts->file, errp);
+ bs = bdrv_co_open_blockdev_ref(opts->file, errp);
} else {
int i;
BlockdevRefList *list = opts->extents;
@@ -2796,14 +2796,16 @@ static BlockBackend * coroutine_fn vmdk_co_create_cb(int64_t size, int idx,
error_setg(errp, "Extent [%d] not specified", idx - 1);
return NULL;
}
- bs = bdrv_open_blockdev_ref(list->value, errp);
+ bs = bdrv_co_open_blockdev_ref(list->value, errp);
}
if (!bs) {
return NULL;
}
- blk = blk_new_with_bs(bs,
- BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE | BLK_PERM_RESIZE,
- BLK_PERM_ALL, errp);
+ blk = blk_co_new_with_bs(bs,
+ BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE |
+ BLK_PERM_RESIZE,
+ BLK_PERM_ALL,
+ errp);
if (!blk) {
return NULL;
}
--
2.38.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 11/13] vpc: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper
2023-01-26 17:24 [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context Kevin Wolf
` (9 preceding siblings ...)
2023-01-26 17:24 ` [PATCH 10/13] vmdk: " Kevin Wolf
@ 2023-01-26 17:24 ` Kevin Wolf
2023-01-26 17:24 ` [PATCH 12/13] block: Fix bdrv_co_create_opts_simple() " Kevin Wolf
` (4 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2023-01-26 17:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, hreitz, eesposit, pbonzini, qemu-devel
.bdrv_co_create implementations run in a coroutine. Therefore they are
not allowed to open images directly. Fix the calls to use the
corresponding no_co_wrappers instead.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/vpc.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/block/vpc.c b/block/vpc.c
index cfdea7db80..3c256fc5a4 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -1005,13 +1005,13 @@ static int coroutine_fn vpc_co_create(BlockdevCreateOptions *opts,
}
/* Create BlockBackend to write to the image */
- bs = bdrv_open_blockdev_ref(vpc_opts->file, errp);
+ bs = bdrv_co_open_blockdev_ref(vpc_opts->file, errp);
if (bs == NULL) {
return -EIO;
}
- blk = blk_new_with_bs(bs, BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL,
- errp);
+ blk = blk_co_new_with_bs(bs, BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL,
+ errp);
if (!blk) {
ret = -EPERM;
goto out;
@@ -1117,8 +1117,8 @@ static int coroutine_fn vpc_co_create_opts(BlockDriver *drv,
goto fail;
}
- bs = bdrv_open(filename, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
+ bs = bdrv_co_open(filename, NULL, NULL,
+ BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
if (bs == NULL) {
ret = -EIO;
goto fail;
--
2.38.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 12/13] block: Fix bdrv_co_create_opts_simple() to open images with no_co_wrapper
2023-01-26 17:24 [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context Kevin Wolf
` (10 preceding siblings ...)
2023-01-26 17:24 ` [PATCH 11/13] vpc: " Kevin Wolf
@ 2023-01-26 17:24 ` Kevin Wolf
2023-02-03 17:04 ` Eric Blake
2023-01-26 17:24 ` [PATCH 13/13] block: Assert non-coroutine context for bdrv_open_inherit() Kevin Wolf
` (3 subsequent siblings)
15 siblings, 1 reply; 18+ messages in thread
From: Kevin Wolf @ 2023-01-26 17:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, hreitz, eesposit, pbonzini, qemu-devel
bdrv_co_create_opts_simpl() runs in a coroutine. Therefore it is not
allowed to open images directly. Fix the call to use the corresponding
no_co_wrapper instead.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block.c b/block.c
index aa9062f2c1..6eac16eac5 100644
--- a/block.c
+++ b/block.c
@@ -657,8 +657,8 @@ int coroutine_fn bdrv_co_create_opts_simple(BlockDriver *drv,
options = qdict_new();
qdict_put_str(options, "driver", drv->format_name);
- blk = blk_new_open(filename, NULL, options,
- BDRV_O_RDWR | BDRV_O_RESIZE, errp);
+ blk = blk_co_new_open(filename, NULL, options,
+ BDRV_O_RDWR | BDRV_O_RESIZE, errp);
if (!blk) {
error_prepend(errp, "Protocol driver '%s' does not support image "
"creation, and opening the image failed: ",
--
2.38.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 12/13] block: Fix bdrv_co_create_opts_simple() to open images with no_co_wrapper
2023-01-26 17:24 ` [PATCH 12/13] block: Fix bdrv_co_create_opts_simple() " Kevin Wolf
@ 2023-02-03 17:04 ` Eric Blake
0 siblings, 0 replies; 18+ messages in thread
From: Eric Blake @ 2023-02-03 17:04 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, hreitz, eesposit, pbonzini, qemu-devel
On Thu, Jan 26, 2023 at 06:24:31PM +0100, Kevin Wolf wrote:
> bdrv_co_create_opts_simpl() runs in a coroutine. Therefore it is not
simple
> allowed to open images directly. Fix the call to use the corresponding
> no_co_wrapper instead.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/block.c b/block.c
> index aa9062f2c1..6eac16eac5 100644
> --- a/block.c
> +++ b/block.c
> @@ -657,8 +657,8 @@ int coroutine_fn bdrv_co_create_opts_simple(BlockDriver *drv,
> options = qdict_new();
> qdict_put_str(options, "driver", drv->format_name);
>
> - blk = blk_new_open(filename, NULL, options,
> - BDRV_O_RDWR | BDRV_O_RESIZE, errp);
> + blk = blk_co_new_open(filename, NULL, options,
> + BDRV_O_RDWR | BDRV_O_RESIZE, errp);
> if (!blk) {
> error_prepend(errp, "Protocol driver '%s' does not support image "
> "creation, and opening the image failed: ",
> --
> 2.38.1
>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 13/13] block: Assert non-coroutine context for bdrv_open_inherit()
2023-01-26 17:24 [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context Kevin Wolf
` (11 preceding siblings ...)
2023-01-26 17:24 ` [PATCH 12/13] block: Fix bdrv_co_create_opts_simple() " Kevin Wolf
@ 2023-01-26 17:24 ` Kevin Wolf
2023-01-27 8:30 ` [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context Emanuele Giuseppe Esposito
` (2 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2023-01-26 17:24 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, hreitz, eesposit, pbonzini, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/block.c b/block.c
index 6eac16eac5..122aa9a9ac 100644
--- a/block.c
+++ b/block.c
@@ -3807,13 +3807,11 @@ out:
* function eventually calls bdrv_refresh_total_sectors() which polls
* when called from non-coroutine context.
*/
-static BlockDriverState *bdrv_open_inherit(const char *filename,
- const char *reference,
- QDict *options, int flags,
- BlockDriverState *parent,
- const BdrvChildClass *child_class,
- BdrvChildRole child_role,
- Error **errp)
+static BlockDriverState * no_coroutine_fn
+bdrv_open_inherit(const char *filename, const char *reference, QDict *options,
+ int flags, BlockDriverState *parent,
+ const BdrvChildClass *child_class, BdrvChildRole child_role,
+ Error **errp)
{
int ret;
BlockBackend *file = NULL;
@@ -3829,6 +3827,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
assert(!child_class || !flags);
assert(!child_class == !parent);
GLOBAL_STATE_CODE();
+ assert(!qemu_in_coroutine());
if (reference) {
bool options_non_empty = options ? qdict_size(options) : false;
--
2.38.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context
2023-01-26 17:24 [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context Kevin Wolf
` (12 preceding siblings ...)
2023-01-26 17:24 ` [PATCH 13/13] block: Assert non-coroutine context for bdrv_open_inherit() Kevin Wolf
@ 2023-01-27 8:30 ` Emanuele Giuseppe Esposito
2023-02-07 15:07 ` Hanna Czenczek
2023-02-13 11:26 ` Kevin Wolf
15 siblings, 0 replies; 18+ messages in thread
From: Emanuele Giuseppe Esposito @ 2023-01-27 8:30 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: hreitz, pbonzini, qemu-devel
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(-)
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context
2023-01-26 17:24 [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context Kevin Wolf
` (13 preceding siblings ...)
2023-01-27 8:30 ` [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context Emanuele Giuseppe Esposito
@ 2023-02-07 15:07 ` Hanna Czenczek
2023-02-13 11:26 ` Kevin Wolf
15 siblings, 0 replies; 18+ messages in thread
From: Hanna Czenczek @ 2023-02-07 15:07 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: eesposit, pbonzini, qemu-devel
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>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context
2023-01-26 17:24 [PATCH 00/13] block: Fix bdrv_open*() calls from coroutine context Kevin Wolf
` (14 preceding siblings ...)
2023-02-07 15:07 ` Hanna Czenczek
@ 2023-02-13 11:26 ` Kevin Wolf
15 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2023-02-13 11:26 UTC (permalink / raw)
To: qemu-block; +Cc: hreitz, eesposit, pbonzini, qemu-devel
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
^ permalink raw reply [flat|nested] 18+ messages in thread