From deb9c2ad0b81ac25fa02935f28cabb9c6155f377 Mon Sep 17 00:00:00 2001 From: Emanuele Giuseppe Esposito Date: Tue, 17 Jan 2023 08:52:01 -0500 Subject: [PATCH 01/22] util/qemu-thread-posix: use TSA_NO_TSA to suppress clang TSA warnings in FreeBSD FreeBSD implements pthread headers using TSA (thread safety analysis) annotations, therefore when an application is compiled with -Wthread-safety there are some locking/annotation requirements that the user of the pthread API has to follow. This will also be the case in QEMU, since util/qemu-thread-posix.c uses the pthread API. Therefore when building it with -Wthread-safety, the compiler will throw warnings because the functions are not properly annotated. We need TSA to be enabled because it ensures that the critical sections of an annotated variable are properly locked. In order to make the compiler happy and avoid adding all the necessary macros to all callers (lock functions should use TSA_ACQUIRE, while unlock TSA_RELEASE, and this applies to all users of pthread_mutex_lock and pthread_mutex_unlock), simply use TSA_NO_TSA to supppress such warnings. Signed-off-by: Emanuele Giuseppe Esposito Message-Id: <20230117135203.3049709-2-eesposit@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- include/qemu/thread.h | 14 +++++++++----- util/qemu-thread-posix.c | 2 +- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/include/qemu/thread.h b/include/qemu/thread.h index 7841084199..dd3822d7ce 100644 --- a/include/qemu/thread.h +++ b/include/qemu/thread.h @@ -3,6 +3,7 @@ #include "qemu/processor.h" #include "qemu/atomic.h" +#include "qemu/clang-tsa.h" typedef struct QemuCond QemuCond; typedef struct QemuSemaphore QemuSemaphore; @@ -24,9 +25,12 @@ typedef struct QemuThread QemuThread; void qemu_mutex_init(QemuMutex *mutex); void qemu_mutex_destroy(QemuMutex *mutex); -int qemu_mutex_trylock_impl(QemuMutex *mutex, const char *file, const int line); -void qemu_mutex_lock_impl(QemuMutex *mutex, const char *file, const int line); -void qemu_mutex_unlock_impl(QemuMutex *mutex, const char *file, const int line); +int TSA_NO_TSA qemu_mutex_trylock_impl(QemuMutex *mutex, const char *file, + const int line); +void TSA_NO_TSA qemu_mutex_lock_impl(QemuMutex *mutex, const char *file, + const int line); +void TSA_NO_TSA qemu_mutex_unlock_impl(QemuMutex *mutex, const char *file, + const int line); void qemu_rec_mutex_init(QemuRecMutex *mutex); void qemu_rec_mutex_destroy(QemuRecMutex *mutex); @@ -153,8 +157,8 @@ void qemu_cond_destroy(QemuCond *cond); */ void qemu_cond_signal(QemuCond *cond); void qemu_cond_broadcast(QemuCond *cond); -void qemu_cond_wait_impl(QemuCond *cond, QemuMutex *mutex, - const char *file, const int line); +void TSA_NO_TSA qemu_cond_wait_impl(QemuCond *cond, QemuMutex *mutex, + const char *file, const int line); bool qemu_cond_timedwait_impl(QemuCond *cond, QemuMutex *mutex, int ms, const char *file, const int line); diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c index bae938c670..2dd1069cd3 100644 --- a/util/qemu-thread-posix.c +++ b/util/qemu-thread-posix.c @@ -223,7 +223,7 @@ void qemu_cond_wait_impl(QemuCond *cond, QemuMutex *mutex, const char *file, con error_exit(err, __func__); } -static bool +static bool TSA_NO_TSA qemu_cond_timedwait_ts(QemuCond *cond, QemuMutex *mutex, struct timespec *ts, const char *file, const int line) { From e022d9cab70f02f7a8fb5fd9c619f46ac877dc4e Mon Sep 17 00:00:00 2001 From: Emanuele Giuseppe Esposito Date: Tue, 17 Jan 2023 08:52:02 -0500 Subject: [PATCH 02/22] bsd-user/mmap: use TSA_NO_TSA to suppress clang TSA warnings in FreeBSD FreeBSD implements pthread headers using TSA (thread safety analysis) annotations, therefore when an application is compiled with -Wthread-safety there are some locking/annotation requirements that the user of the pthread API has to follow. This will also be the case in QEMU, since bsd-user/mmap.c uses the pthread API. Therefore when building it with -Wthread-safety the compiler will throw warnings because the functions are not properly annotated. We need TSA to be enabled because it ensures that the critical sections of an annotated variable are properly locked. In order to make the compiler happy and avoid adding all the necessary macros to all callers (lock functions should use TSA_ACQUIRE, while unlock TSA_RELEASE, and this applies to all users of pthread_mutex_lock and pthread_mutex_unlock), simply use TSA_NO_TSA to supppress such warnings. Signed-off-by: Emanuele Giuseppe Esposito Message-Id: <20230117135203.3049709-3-eesposit@redhat.com> Reviewed-by: Warner Losh Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- bsd-user/qemu.h | 5 +++-- include/exec/exec-all.h | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h index 0ceecfb6df..4e7b8b1c06 100644 --- a/bsd-user/qemu.h +++ b/bsd-user/qemu.h @@ -36,6 +36,7 @@ extern char **environ; #include "target_os_signal.h" #include "target.h" #include "exec/gdbstub.h" +#include "qemu/clang-tsa.h" /* * This struct is used to hold certain information about the image. Basically, @@ -234,8 +235,8 @@ int target_msync(abi_ulong start, abi_ulong len, int flags); extern unsigned long last_brk; extern abi_ulong mmap_next_start; abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size); -void mmap_fork_start(void); -void mmap_fork_end(int child); +void TSA_NO_TSA mmap_fork_start(void); +void TSA_NO_TSA mmap_fork_end(int child); /* main.c */ extern char qemu_proc_pathname[]; diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 54585a9954..0e36f4d063 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -25,6 +25,7 @@ #include "exec/cpu_ldst.h" #endif #include "qemu/interval-tree.h" +#include "qemu/clang-tsa.h" /* allow to see translation results - the slowdown should be negligible, so we leave it */ #define DEBUG_DISAS @@ -759,8 +760,8 @@ static inline tb_page_addr_t get_page_addr_code(CPUArchState *env, } #if defined(CONFIG_USER_ONLY) -void mmap_lock(void); -void mmap_unlock(void); +void TSA_NO_TSA mmap_lock(void); +void TSA_NO_TSA mmap_unlock(void); bool have_mmap_lock(void); /** From 3d2d4cc5a23229088528f9451518f12dea9a7285 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 17 Jan 2023 08:52:03 -0500 Subject: [PATCH 03/22] configure: Enable -Wthread-safety if present This enables clang's thread safety analysis (TSA), which we'll use to statically check the block graph locking. Signed-off-by: Kevin Wolf Message-Id: <20221207131838.239125-9-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito Signed-off-by: Kevin Wolf Message-Id: <20230117135203.3049709-4-eesposit@redhat.com> Signed-off-by: Kevin Wolf --- configure | 1 + 1 file changed, 1 insertion(+) diff --git a/configure b/configure index 00415f0b48..cf6db3d551 100755 --- a/configure +++ b/configure @@ -1184,6 +1184,7 @@ add_to warn_flags -Wendif-labels add_to warn_flags -Wexpansion-to-defined add_to warn_flags -Wimplicit-fallthrough=2 add_to warn_flags -Wmissing-format-attribute +add_to warn_flags -Wthread-safety nowarn_flags= add_to nowarn_flags -Wno-initializer-overrides From 1e84cf79573e364075d6e63a4b00f7dc5f8aa924 Mon Sep 17 00:00:00 2001 From: Hanna Czenczek Date: Mon, 6 Feb 2023 14:29:49 +0100 Subject: [PATCH 04/22] curl: Fix error path in curl_open() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit g_hash_table_destroy() and g_hash_table_foreach_remove() (called by curl_drop_all_sockets()) both require the table to be non-NULL, or will print assertion failures (just print, no abort). There are several paths in curl_open() that can lead to the out_noclean label without s->sockets being allocated, so clean it only if it has been allocated. Example reproducer: $ qemu-img info -f http '' qemu-img: GLib: g_hash_table_foreach_remove: assertion 'hash_table != NULL' failed qemu-img: GLib: g_hash_table_destroy: assertion 'hash_table != NULL' failed qemu-img: Could not open '': http curl driver cannot handle the URL '' (does not start with 'http://') Closes: https://gitlab.com/qemu-project/qemu/-/issues/1475 Suggested-by: Daniel P. Berrangé Signed-off-by: Hanna Czenczek Message-Id: <20230206132949.92917-1-hreitz@redhat.com> Reviewed-by: Daniel P. Berrangé Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/curl.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/block/curl.c b/block/curl.c index cbada22e9e..ba9977af5a 100644 --- a/block/curl.c +++ b/block/curl.c @@ -850,8 +850,10 @@ out_noclean: g_free(s->username); g_free(s->proxyusername); g_free(s->proxypassword); - curl_drop_all_sockets(s->sockets); - g_hash_table_destroy(s->sockets); + if (s->sockets) { + curl_drop_all_sockets(s->sockets); + g_hash_table_destroy(s->sockets); + } qemu_opts_del(opts); return -EINVAL; } From d6ee2e324ec26a02776d90125e3a55454f0ca57e Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 26 Jan 2023 18:24:20 +0100 Subject: [PATCH 05/22] block-coroutine-wrapper: Introduce no_co_wrapper 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 Message-Id: <20230126172432.436111-2-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito Reviewed-by: Hanna Czenczek Signed-off-by: Kevin Wolf --- 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 @@ class ParamDecl: 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 @@ class FuncDecl: 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 @@ class FuncDecl: # Match wrappers declared with a co_wrapper mark func_decl_re = re.compile(r'^(?P[a-zA-Z][a-zA-Z0-9_]* [\*]?)' - r'\s*co_wrapper' + r'(\s*coroutine_fn)?' + r'\s*(?P(no_)?co)_wrapper' r'(?P(_[a-z][a-z0-9_]*)?)\s*' r'(?P[a-z][a-z0-9_]*)' r'\((?P[^)]*)\);$', re.MULTILINE) @@ -113,7 +127,8 @@ func_decl_re = re.compile(r'^(?P[a-zA-Z][a-zA-Z0-9_]* [\*]?)' 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 @@ static void coroutine_fn {name}_entry(void *opaque) {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 From 4bee90e9da3c58b09d4df949d9c64043133e4181 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 26 Jan 2023 18:24:21 +0100 Subject: [PATCH 06/22] block: Create no_co_wrappers for open functions 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 Message-Id: <20230126172432.436111-3-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito Reviewed-by: Hanna Czenczek Signed-off-by: Kevin Wolf --- block/meson.build | 1 + include/block/block-global-state.h | 35 +++++++++++++++------ include/sysemu/block-backend-global-state.h | 21 ++++++++++--- 3 files changed, 44 insertions(+), 13 deletions(-) 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' ), 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); From 91817e9c58687f44a4d4d2ee39b88cb778d228a8 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 26 Jan 2023 18:24:22 +0100 Subject: [PATCH 07/22] luks: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper .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 Message-Id: <20230126172432.436111-4-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito Reviewed-by: Hanna Czenczek Signed-off-by: Kevin Wolf --- block/crypto.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/block/crypto.c b/block/crypto.c index b70cec97c7..72ac30568c 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -314,19 +314,18 @@ static int block_crypto_open_generic(QCryptoBlockFormat format, } -static int block_crypto_co_create_generic(BlockDriverState *bs, - int64_t size, - QCryptoBlockCreateOptions *opts, - PreallocMode prealloc, - Error **errp) +static int coroutine_fn +block_crypto_co_create_generic(BlockDriverState *bs, int64_t size, + QCryptoBlockCreateOptions *opts, + PreallocMode prealloc, Error **errp) { int ret; BlockBackend *blk; 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 +638,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 +707,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; From 48a4e92d3c8458cd5ab272790dc6fd884c58e206 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 26 Jan 2023 18:24:23 +0100 Subject: [PATCH 08/22] parallels: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper .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 Message-Id: <20230126172432.436111-5-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito Reviewed-by: Hanna Czenczek Signed-off-by: Kevin Wolf --- 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; From 5b9d79b62dcee57c9f0f0a5b34eea2bbb1f4e8d2 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 26 Jan 2023 18:24:24 +0100 Subject: [PATCH 09/22] qcow: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper .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 Message-Id: <20230126172432.436111-6-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito Reviewed-by: Hanna Czenczek Signed-off-by: Kevin Wolf --- 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; From ecbc57caba986d7ac0d5ecb75cc72cdfe3602f51 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 26 Jan 2023 18:24:25 +0100 Subject: [PATCH 10/22] qcow2: Fix open/create to open images with no_co_wrapper .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 Message-Id: <20230126172432.436111-7-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito Reviewed-by: Hanna Czenczek Signed-off-by: Kevin Wolf --- 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; From 0b1e95cf46f7f43a39b5c4043b399b8ec29bd440 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 26 Jan 2023 18:24:26 +0100 Subject: [PATCH 11/22] qed: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper .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 Message-Id: <20230126172432.436111-8-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito Reviewed-by: Hanna Czenczek Signed-off-by: Kevin Wolf --- 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; From 13dd6327efb565678bcfe14270dc3b6f7859237c Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 26 Jan 2023 18:24:27 +0100 Subject: [PATCH 12/22] vdi: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper .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 Message-Id: <20230126172432.436111-9-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito Reviewed-by: Hanna Czenczek Signed-off-by: Kevin Wolf --- 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; From 41e089cbe9966a9459aabd0f754f65e2619391ef Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 26 Jan 2023 18:24:28 +0100 Subject: [PATCH 13/22] vhdx: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper .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 Message-Id: <20230126172432.436111-10-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito Reviewed-by: Hanna Czenczek Signed-off-by: Kevin Wolf --- 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; From 882f202e9da82369521d4b7a65694571c35e20f1 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 26 Jan 2023 18:24:29 +0100 Subject: [PATCH 14/22] vmdk: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper .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 Message-Id: <20230126172432.436111-11-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito Reviewed-by: Hanna Czenczek Signed-off-by: Kevin Wolf --- 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; } From 6ef028519b02fe64bdd6c87c81fd3ed05054ac55 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 26 Jan 2023 18:24:30 +0100 Subject: [PATCH 15/22] vpc: Fix .bdrv_co_create(_opts) to open images with no_co_wrapper .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 Message-Id: <20230126172432.436111-12-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito Reviewed-by: Hanna Czenczek Signed-off-by: Kevin Wolf --- 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; From be1a732c9a74da9b27884c20e14df608fd100401 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 26 Jan 2023 18:24:31 +0100 Subject: [PATCH 16/22] block: Fix bdrv_co_create_opts_simple() to open images with no_co_wrapper bdrv_co_create_opts_simple() 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 Message-Id: <20230126172432.436111-13-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito Reviewed-by: Hanna Czenczek Signed-off-by: Kevin Wolf --- 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: ", From 321923010d0e46f265c9a16efc340bf0cb66f785 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 26 Jan 2023 18:24:32 +0100 Subject: [PATCH 17/22] block: Assert non-coroutine context for bdrv_open_inherit() Signed-off-by: Kevin Wolf Message-Id: <20230126172432.436111-14-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito Reviewed-by: Hanna Czenczek Signed-off-by: Kevin Wolf --- 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; From e7b8d9d038f313c2b9e601609e7d7c3ca6ad0234 Mon Sep 17 00:00:00 2001 From: Anton Johansson Date: Mon, 23 Jan 2023 21:14:31 +0100 Subject: [PATCH 18/22] block: Handle curl 7.55.0, 7.85.0 version changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 7.55.0 deprecates CURLINFO_CONTENT_LENGTH_DOWNLOAD in favour of a *_T version, which returns curl_off_t instead of a double. * 7.85.0 deprecates CURLOPT_PROTOCOLS and CURLOPT_REDIR_PROTOCOLS in favour of *_STR variants, specifying the desired protocols via a string. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1440 Signed-off-by: Anton Johansson Message-Id: <20230123201431.23118-1-anjo@rev.ng> Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/curl.c | 44 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/block/curl.c b/block/curl.c index ba9977af5a..8bb39a134e 100644 --- a/block/curl.c +++ b/block/curl.c @@ -38,8 +38,15 @@ // #define DEBUG_VERBOSE +/* CURL 7.85.0 switches to a string based API for specifying + * the desired protocols. + */ +#if LIBCURL_VERSION_NUM >= 0x075500 +#define PROTOCOLS "HTTP,HTTPS,FTP,FTPS" +#else #define PROTOCOLS (CURLPROTO_HTTP | CURLPROTO_HTTPS | \ CURLPROTO_FTP | CURLPROTO_FTPS) +#endif #define CURL_NUM_STATES 8 #define CURL_NUM_ACB 8 @@ -510,9 +517,18 @@ static int curl_init_state(BDRVCURLState *s, CURLState *state) * obscure protocols. For example, do not allow POP3/SMTP/IMAP see * CVE-2013-0249. * - * Restricting protocols is only supported from 7.19.4 upwards. + * Restricting protocols is only supported from 7.19.4 upwards. Note: + * version 7.85.0 deprecates CURLOPT_*PROTOCOLS in favour of a string + * based CURLOPT_*PROTOCOLS_STR API. */ -#if LIBCURL_VERSION_NUM >= 0x071304 +#if LIBCURL_VERSION_NUM >= 0x075500 + if (curl_easy_setopt(state->curl, + CURLOPT_PROTOCOLS_STR, PROTOCOLS) || + curl_easy_setopt(state->curl, + CURLOPT_REDIR_PROTOCOLS_STR, PROTOCOLS)) { + goto err; + } +#elif LIBCURL_VERSION_NUM >= 0x071304 if (curl_easy_setopt(state->curl, CURLOPT_PROTOCOLS, PROTOCOLS) || curl_easy_setopt(state->curl, CURLOPT_REDIR_PROTOCOLS, PROTOCOLS)) { goto err; @@ -670,7 +686,12 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags, const char *file; const char *cookie; const char *cookie_secret; - double d; + /* CURL >= 7.55.0 uses curl_off_t for content length instead of a double */ +#if LIBCURL_VERSION_NUM >= 0x073700 + curl_off_t cl; +#else + double cl; +#endif const char *secretid; const char *protocol_delimiter; int ret; @@ -797,27 +818,36 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags, } if (curl_easy_perform(state->curl)) goto out; - if (curl_easy_getinfo(state->curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &d)) { + /* CURL 7.55.0 deprecates CURLINFO_CONTENT_LENGTH_DOWNLOAD in favour of + * the *_T version which returns a more sensible type for content length. + */ +#if LIBCURL_VERSION_NUM >= 0x073700 + if (curl_easy_getinfo(state->curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD_T, &cl)) { goto out; } +#else + if (curl_easy_getinfo(state->curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &cl)) { + goto out; + } +#endif /* Prior CURL 7.19.4 return value of 0 could mean that the file size is not * know or the size is zero. From 7.19.4 CURL returns -1 if size is not * known and zero if it is really zero-length file. */ #if LIBCURL_VERSION_NUM >= 0x071304 - if (d < 0) { + if (cl < 0) { pstrcpy(state->errmsg, CURL_ERROR_SIZE, "Server didn't report file size."); goto out; } #else - if (d <= 0) { + if (cl <= 0) { pstrcpy(state->errmsg, CURL_ERROR_SIZE, "Unknown file size or zero-length file."); goto out; } #endif - s->len = d; + s->len = cl; if ((!strncasecmp(s->url, "http://", strlen("http://")) || !strncasecmp(s->url, "https://", strlen("https://"))) From 60d90bf43c169b9d1dbcb17ed794b7b02c6862b1 Mon Sep 17 00:00:00 2001 From: Stefano Garzarella Date: Tue, 14 Feb 2023 18:16:21 +0100 Subject: [PATCH 19/22] block: temporarily hold the new AioContext of bs_top in bdrv_append() bdrv_append() is called with bs_top AioContext held, but bdrv_attach_child_noperm() could change the AioContext of bs_top. bdrv_replace_node_noperm() calls bdrv_drained_begin() starting from commit 2398747128 ("block: Don't poll in bdrv_replace_child_noperm()"). bdrv_drained_begin() can call BDRV_POLL_WHILE that assumes the new lock is taken, so let's temporarily hold the new AioContext to prevent QEMU from failing in BDRV_POLL_WHILE when it tries to release the wrong AioContext. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2168209 Reported-by: Aihua Liang Signed-off-by: Stefano Garzarella Message-Id: <20230214171621.11574-1-sgarzare@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/block.c b/block.c index 122aa9a9ac..0c807d15cd 100644 --- a/block.c +++ b/block.c @@ -5265,6 +5265,8 @@ int bdrv_drop_filter(BlockDriverState *bs, Error **errp) * child. * * This function does not create any image files. + * + * The caller must hold the AioContext lock for @bs_top. */ int bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top, Error **errp) @@ -5272,11 +5274,14 @@ int bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top, int ret; BdrvChild *child; Transaction *tran = tran_new(); + AioContext *old_context, *new_context = NULL; GLOBAL_STATE_CODE(); assert(!bs_new->backing); + old_context = bdrv_get_aio_context(bs_top); + child = bdrv_attach_child_noperm(bs_new, bs_top, "backing", &child_of_bds, bdrv_backing_role(bs_new), tran, errp); @@ -5285,6 +5290,19 @@ int bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top, goto out; } + /* + * bdrv_attach_child_noperm could change the AioContext of bs_top. + * bdrv_replace_node_noperm calls bdrv_drained_begin, so let's temporarily + * hold the new AioContext, since bdrv_drained_begin calls BDRV_POLL_WHILE + * that assumes the new lock is taken. + */ + new_context = bdrv_get_aio_context(bs_top); + + if (old_context != new_context) { + aio_context_release(old_context); + aio_context_acquire(new_context); + } + ret = bdrv_replace_node_noperm(bs_top, bs_new, true, tran, errp); if (ret < 0) { goto out; @@ -5296,6 +5314,11 @@ out: bdrv_refresh_limits(bs_top, NULL, NULL); + if (new_context && old_context != new_context) { + aio_context_release(new_context); + aio_context_acquire(old_context); + } + return ret; } From 167643ff5e77bdfa3d2867f2e2469741484bd63f Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Tue, 14 Feb 2023 21:28:48 +0300 Subject: [PATCH 20/22] MAINTAINERS: drop Vladimir from parallels block driver I have to admit this is out of my scope now. Still feel free to Cc me directly if my help is needed :) Signed-off-by: Vladimir Sementsov-Ogievskiy Message-Id: <20230214182848.1564714-1-vsementsov@yandex-team.ru> Signed-off-by: Kevin Wolf --- MAINTAINERS | 2 -- 1 file changed, 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index fd54c1f140..65ee4c31b1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3579,13 +3579,11 @@ F: block/dmg.c parallels M: Stefan Hajnoczi M: Denis V. Lunev -M: Vladimir Sementsov-Ogievskiy L: qemu-block@nongnu.org S: Supported F: block/parallels.c F: block/parallels-ext.c F: docs/interop/parallels.txt -T: git https://gitlab.com/vsementsov/qemu.git block qed M: Stefan Hajnoczi From 005ee3cdc79e05b7691c8ce078c147c1f9336814 Mon Sep 17 00:00:00 2001 From: Emanuele Giuseppe Esposito Date: Thu, 9 Feb 2023 10:45:22 -0500 Subject: [PATCH 21/22] block/file-posix: don't use functions calling AIO_WAIT_WHILE in worker threads When calling bdrv_getlength() in handle_aiocb_write_zeroes(), the function creates a new coroutine and then waits that it finishes using AIO_WAIT_WHILE. The problem is that this function could also run in a worker thread, that has a different AioContext from main loop and iothreads, therefore in AIO_WAIT_WHILE we will have in_aio_context_home_thread(ctx) == false and therefore assert(qemu_get_current_aio_context() == qemu_get_aio_context()); in the else branch will fail, crashing QEMU. Aside from that, bdrv_getlength() is wrong also conceptually, because it reads the BDS graph from another thread and is not protected by any lock. Replace it with raw_co_getlength, that doesn't create a coroutine and doesn't read the BDS graph. Reported-by: Ninad Palsule Suggested-by: Kevin Wolf Signed-off-by: Emanuele Giuseppe Esposito Message-Id: <20230209154522.1164401-1-eesposit@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/file-posix.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/file-posix.c b/block/file-posix.c index d3073a7caa..9a99111f45 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -1738,7 +1738,7 @@ static int handle_aiocb_write_zeroes(void *opaque) #ifdef CONFIG_FALLOCATE /* Last resort: we are trying to extend the file with zeroed data. This * can be done via fallocate(fd, 0) */ - len = bdrv_getlength(aiocb->bs); + len = raw_co_getlength(aiocb->bs); if (s->has_fallocate && len >= 0 && aiocb->aio_offset >= len) { int ret = do_fallocate(s->fd, 0, aiocb->aio_offset, aiocb->aio_nbytes); if (ret == 0 || ret != -ENOTSUP) { From a4d5224c2cb650b5a401d626d3f36e42e6987aa7 Mon Sep 17 00:00:00 2001 From: Andrey Zhadchenko Date: Thu, 2 Feb 2023 21:15:23 +0300 Subject: [PATCH 22/22] hbitmap: fix hbitmap_status() return value for first dirty bit case The last return statement should return true, as we already evaluated that start == next_dirty Also, fix hbitmap_status() description in header Cc: qemu-stable@nongnu.org Fixes: a6426475a75 ("block/dirty-bitmap: introduce bdrv_dirty_bitmap_status()") Signed-off-by: Andrey Zhadchenko Message-Id: <20230202181523.423131-1-andrey.zhadchenko@virtuozzo.com> Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- include/qemu/hbitmap.h | 2 +- util/hbitmap.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h index af4e4ab746..8136e33674 100644 --- a/include/qemu/hbitmap.h +++ b/include/qemu/hbitmap.h @@ -330,7 +330,7 @@ bool hbitmap_next_dirty_area(const HBitmap *hb, int64_t start, int64_t end, int64_t *dirty_start, int64_t *dirty_count); /* - * bdrv_dirty_bitmap_status: + * hbitmap_status: * @hb: The HBitmap to operate on * @start: The bit to start from * @count: Number of bits to proceed diff --git a/util/hbitmap.c b/util/hbitmap.c index 297db35fb1..6d6e1b595d 100644 --- a/util/hbitmap.c +++ b/util/hbitmap.c @@ -331,7 +331,7 @@ bool hbitmap_status(const HBitmap *hb, int64_t start, int64_t count, assert(next_zero > start); *pnum = next_zero - start; - return false; + return true; } bool hbitmap_empty(const HBitmap *hb)