[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH 08/20] block: .bdrv_open is non-coroutine and unlocked
From: |
Kevin Wolf |
Subject: |
[PATCH 08/20] block: .bdrv_open is non-coroutine and unlocked |
Date: |
Tue, 25 Apr 2023 19:31:46 +0200 |
Drivers were a bit confused about whether .bdrv_open can run in a
coroutine and whether or not it holds a graph lock.
It cannot keep a graph lock from the caller across the whole function
because it both changes the graph (requires a writer lock) and does I/O
(requires a reader lock). Therefore, it should take these locks
internally as needed.
The functions used to be called in coroutine context during image
creation. This was buggy for other reasons, and as of commit 32192301,
all block drivers go through no_co_wrappers. So it is not called in
coroutine context any more.
Fix qcow2 and qed to work with the correct assumptions: The graph lock
needs to be taken internally instead of just assuming it's already
there, and the coroutine path is dead code that can be removed.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/block/block_int-common.h | 8 ++++----
block.c | 6 +++---
block/qcow2.c | 15 ++++++---------
block/qed.c | 18 ++++++++----------
4 files changed, 21 insertions(+), 26 deletions(-)
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 013d419444..6fb28cd8fa 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -236,12 +236,12 @@ struct BlockDriver {
void (*bdrv_reopen_abort)(BDRVReopenState *reopen_state);
void (*bdrv_join_options)(QDict *options, QDict *old_options);
- int (*bdrv_open)(BlockDriverState *bs, QDict *options, int flags,
- Error **errp);
+ int GRAPH_UNLOCKED_PTR (*bdrv_open)(
+ BlockDriverState *bs, QDict *options, int flags, Error **errp);
/* Protocol drivers should implement this instead of bdrv_open */
- int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags,
- Error **errp);
+ int GRAPH_UNLOCKED_PTR (*bdrv_file_open)(
+ BlockDriverState *bs, QDict *options, int flags, Error **errp);
void (*bdrv_close)(BlockDriverState *bs);
int coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_create)(
diff --git a/block.c b/block.c
index 20d5ee0959..abec940867 100644
--- a/block.c
+++ b/block.c
@@ -1610,9 +1610,9 @@ out:
* bdrv_refresh_total_sectors() which polls when called from non-coroutine
* context.
*/
-static int bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv,
- const char *node_name, QDict *options,
- int open_flags, Error **errp)
+static int no_coroutine_fn GRAPH_UNLOCKED
+bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv, const char *node_name,
+ QDict *options, int open_flags, Error **errp)
{
Error *local_err = NULL;
int i, ret;
diff --git a/block/qcow2.c b/block/qcow2.c
index 01742b3ebe..5bde3b8401 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1891,7 +1891,7 @@ static void coroutine_fn qcow2_open_entry(void *opaque)
QCow2OpenCo *qoc = opaque;
BDRVQcow2State *s = qoc->bs->opaque;
- assume_graph_lock(); /* FIXME */
+ GRAPH_RDLOCK_GUARD();
qemu_co_mutex_lock(&s->lock);
qoc->ret = qcow2_do_open(qoc->bs, qoc->options, qoc->flags, true,
@@ -1920,14 +1920,11 @@ static int qcow2_open(BlockDriverState *bs, QDict
*options, int flags,
/* Initialise locks */
qemu_co_mutex_init(&s->lock);
- if (qemu_in_coroutine()) {
- /* From bdrv_co_create. */
- qcow2_open_entry(&qoc);
- } else {
- assert(qemu_get_current_aio_context() == qemu_get_aio_context());
- qemu_coroutine_enter(qemu_coroutine_create(qcow2_open_entry, &qoc));
- BDRV_POLL_WHILE(bs, qoc.ret == -EINPROGRESS);
- }
+ assert(!qemu_in_coroutine());
+ assert(qemu_get_current_aio_context() == qemu_get_aio_context());
+ qemu_coroutine_enter(qemu_coroutine_create(qcow2_open_entry, &qoc));
+ BDRV_POLL_WHILE(bs, qoc.ret == -EINPROGRESS);
+
return qoc.ret;
}
diff --git a/block/qed.c b/block/qed.c
index aff2a2076e..be9ff0fb34 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -557,11 +557,13 @@ typedef struct QEDOpenCo {
int ret;
} QEDOpenCo;
-static void coroutine_fn GRAPH_RDLOCK bdrv_qed_open_entry(void *opaque)
+static void coroutine_fn bdrv_qed_open_entry(void *opaque)
{
QEDOpenCo *qoc = opaque;
BDRVQEDState *s = qoc->bs->opaque;
+ GRAPH_RDLOCK_GUARD();
+
qemu_co_mutex_lock(&s->table_lock);
qoc->ret = bdrv_qed_do_open(qoc->bs, qoc->options, qoc->flags, qoc->errp);
qemu_co_mutex_unlock(&s->table_lock);
@@ -579,21 +581,17 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict
*options, int flags,
};
int ret;
- assume_graph_lock(); /* FIXME */
-
ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
if (ret < 0) {
return ret;
}
bdrv_qed_init_state(bs);
- if (qemu_in_coroutine()) {
- bdrv_qed_open_entry(&qoc);
- } else {
- assert(qemu_get_current_aio_context() == qemu_get_aio_context());
- qemu_coroutine_enter(qemu_coroutine_create(bdrv_qed_open_entry, &qoc));
- BDRV_POLL_WHILE(bs, qoc.ret == -EINPROGRESS);
- }
+ assert(!qemu_in_coroutine());
+ assert(qemu_get_current_aio_context() == qemu_get_aio_context());
+ qemu_coroutine_enter(qemu_coroutine_create(bdrv_qed_open_entry, &qoc));
+ BDRV_POLL_WHILE(bs, qoc.ret == -EINPROGRESS);
+
return qoc.ret;
}
--
2.40.0
- [PATCH 07/20] graph-lock: Fix GRAPH_RDLOCK_GUARD*() to be reader lock, (continued)
- [PATCH 07/20] graph-lock: Fix GRAPH_RDLOCK_GUARD*() to be reader lock, Kevin Wolf, 2023/04/25
- [PATCH 15/20] block: Mark bdrv_co_debug_event() GRAPH_RDLOCK, Kevin Wolf, 2023/04/25
- [PATCH 11/20] vhdx: Take graph lock for accessing a node's parent list, Kevin Wolf, 2023/04/25
- [PATCH 13/20] block: Mark bdrv_co_get_allocated_file_size() and callers GRAPH_RDLOCK, Kevin Wolf, 2023/04/25
- [PATCH 14/20] block: Mark bdrv_co_get_info() and callers GRAPH_RDLOCK, Kevin Wolf, 2023/04/25
- [PATCH 08/20] block: .bdrv_open is non-coroutine and unlocked,
Kevin Wolf <=
- [PATCH 09/20] nbd: Remove nbd_co_flush() wrapper function, Kevin Wolf, 2023/04/25
- [PATCH 12/20] mirror: Take graph lock for accessing a node's parent list, Kevin Wolf, 2023/04/25
- [PATCH 18/20] block: Mark bdrv_query_block_graph_info() and callers GRAPH_RDLOCK, Kevin Wolf, 2023/04/25
- [PATCH 19/20] block: Mark bdrv_recurse_can_replace() and callers GRAPH_RDLOCK, Kevin Wolf, 2023/04/25
- [PATCH 20/20] block: Mark bdrv_refresh_limits() and callers GRAPH_RDLOCK, Kevin Wolf, 2023/04/25