[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 21/28] block: Mark bdrv_add/del_child() and caller GRAPH_WRLOCK
From: |
Kevin Wolf |
Subject: |
[PULL 21/28] block: Mark bdrv_add/del_child() and caller GRAPH_WRLOCK |
Date: |
Fri, 15 Sep 2023 16:43:37 +0200 |
The functions read the parents list in the generic block layer, so we
need to hold the graph lock already there. The BlockDriver
implementations actually modify the graph, so it has to be a writer
lock.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20230911094620.45040-22-kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/block/block-global-state.h | 8 +++++---
include/block/block_int-common.h | 9 +++++----
block/quorum.c | 23 ++++++-----------------
blockdev.c | 17 +++++++++++------
4 files changed, 27 insertions(+), 30 deletions(-)
diff --git a/include/block/block-global-state.h
b/include/block/block-global-state.h
index 0f6df8f1a2..f31660c7b1 100644
--- a/include/block/block-global-state.h
+++ b/include/block/block-global-state.h
@@ -276,9 +276,11 @@ int bdrv_try_change_aio_context(BlockDriverState *bs,
AioContext *ctx,
int bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz);
int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo);
-void bdrv_add_child(BlockDriverState *parent, BlockDriverState *child,
- Error **errp);
-void bdrv_del_child(BlockDriverState *parent, BdrvChild *child, Error **errp);
+void GRAPH_WRLOCK
+bdrv_add_child(BlockDriverState *parent, BlockDriverState *child, Error
**errp);
+
+void GRAPH_WRLOCK
+bdrv_del_child(BlockDriverState *parent, BdrvChild *child, Error **errp);
/**
*
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 3feb67ec4a..2ca3758cb8 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -393,10 +393,11 @@ struct BlockDriver {
*/
int (*bdrv_probe_geometry)(BlockDriverState *bs, HDGeometry *geo);
- void (*bdrv_add_child)(BlockDriverState *parent, BlockDriverState *child,
- Error **errp);
- void (*bdrv_del_child)(BlockDriverState *parent, BdrvChild *child,
- Error **errp);
+ void GRAPH_WRLOCK_PTR (*bdrv_add_child)(
+ BlockDriverState *parent, BlockDriverState *child, Error **errp);
+
+ void GRAPH_WRLOCK_PTR (*bdrv_del_child)(
+ BlockDriverState *parent, BdrvChild *child, Error **errp);
/**
* Informs the block driver that a permission change is intended. The
diff --git a/block/quorum.c b/block/quorum.c
index 620a50ba2c..05220cab7f 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -1066,8 +1066,8 @@ static void quorum_close(BlockDriverState *bs)
g_free(s->children);
}
-static void quorum_add_child(BlockDriverState *bs, BlockDriverState *child_bs,
- Error **errp)
+static void GRAPH_WRLOCK
+quorum_add_child(BlockDriverState *bs, BlockDriverState *child_bs, Error
**errp)
{
BDRVQuorumState *s = bs->opaque;
BdrvChild *child;
@@ -1093,29 +1093,22 @@ static void quorum_add_child(BlockDriverState *bs,
BlockDriverState *child_bs,
}
s->next_child_index++;
- bdrv_drained_begin(bs);
-
/* We can safely add the child now */
bdrv_ref(child_bs);
- bdrv_graph_wrlock(child_bs);
child = bdrv_attach_child(bs, child_bs, indexstr, &child_of_bds,
BDRV_CHILD_DATA, errp);
- bdrv_graph_wrunlock();
if (child == NULL) {
s->next_child_index--;
- goto out;
+ return;
}
s->children = g_renew(BdrvChild *, s->children, s->num_children + 1);
s->children[s->num_children++] = child;
quorum_refresh_flags(bs);
-
-out:
- bdrv_drained_end(bs);
}
-static void quorum_del_child(BlockDriverState *bs, BdrvChild *child,
- Error **errp)
+static void GRAPH_WRLOCK
+quorum_del_child(BlockDriverState *bs, BdrvChild *child, Error **errp)
{
BDRVQuorumState *s = bs->opaque;
char indexstr[INDEXSTR_LEN];
@@ -1145,18 +1138,14 @@ static void quorum_del_child(BlockDriverState *bs,
BdrvChild *child,
s->next_child_index--;
}
- bdrv_drained_begin(bs);
-
/* We can safely remove this child now */
memmove(&s->children[i], &s->children[i + 1],
(s->num_children - i - 1) * sizeof(BdrvChild *));
s->children = g_renew(BdrvChild *, s->children, --s->num_children);
- bdrv_graph_wrlock(NULL);
+
bdrv_unref_child(bs, child);
- bdrv_graph_wrunlock();
quorum_refresh_flags(bs);
- bdrv_drained_end(bs);
}
static void quorum_gather_child_options(BlockDriverState *bs, QDict *target,
diff --git a/blockdev.c b/blockdev.c
index 372eaf198c..325b7a3bef 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3545,8 +3545,8 @@ out:
aio_context_release(aio_context);
}
-static BdrvChild *bdrv_find_child(BlockDriverState *parent_bs,
- const char *child_name)
+static BdrvChild * GRAPH_RDLOCK
+bdrv_find_child(BlockDriverState *parent_bs, const char *child_name)
{
BdrvChild *child;
@@ -3565,9 +3565,11 @@ void qmp_x_blockdev_change(const char *parent, const
char *child,
BlockDriverState *parent_bs, *new_bs = NULL;
BdrvChild *p_child;
+ bdrv_graph_wrlock(NULL);
+
parent_bs = bdrv_lookup_bs(parent, parent, errp);
if (!parent_bs) {
- return;
+ goto out;
}
if (!child == !node) {
@@ -3576,7 +3578,7 @@ void qmp_x_blockdev_change(const char *parent, const char
*child,
} else {
error_setg(errp, "Either child or node must be specified");
}
- return;
+ goto out;
}
if (child) {
@@ -3584,7 +3586,7 @@ void qmp_x_blockdev_change(const char *parent, const char
*child,
if (!p_child) {
error_setg(errp, "Node '%s' does not have child '%s'",
parent, child);
- return;
+ goto out;
}
bdrv_del_child(parent_bs, p_child, errp);
}
@@ -3593,10 +3595,13 @@ void qmp_x_blockdev_change(const char *parent, const
char *child,
new_bs = bdrv_find_node(node);
if (!new_bs) {
error_setg(errp, "Node '%s' not found", node);
- return;
+ goto out;
}
bdrv_add_child(parent_bs, new_bs, errp);
}
+
+out:
+ bdrv_graph_wrunlock();
}
BlockJobInfoList *qmp_query_block_jobs(Error **errp)
--
2.41.0
- [PULL 27/28] block-backend: process zoned requests in the current AioContext, (continued)
- [PULL 27/28] block-backend: process zoned requests in the current AioContext, Kevin Wolf, 2023/09/15
- [PULL 26/28] block-backend: process I/O in the current AioContext, Kevin Wolf, 2023/09/15
- [PULL 12/28] block: Mark bdrv_attach_child() GRAPH_WRLOCK, Kevin Wolf, 2023/09/15
- [PULL 07/28] block-coroutine-wrapper: Allow arbitrary parameter names, Kevin Wolf, 2023/09/15
- [PULL 11/28] block: Call transaction callbacks with lock held, Kevin Wolf, 2023/09/15
- [PULL 13/28] block: Mark bdrv_parent_perms_conflict() and callers GRAPH_RDLOCK, Kevin Wolf, 2023/09/15
- [PULL 18/28] block: Take graph rdlock in bdrv_change_aio_context(), Kevin Wolf, 2023/09/15
- [PULL 19/28] block: Mark bdrv_root_unref_child() GRAPH_WRLOCK, Kevin Wolf, 2023/09/15
- [PULL 25/28] test-bdrv-drain: avoid race with BH in IOThread drain test, Kevin Wolf, 2023/09/15
- [PULL 24/28] block: remove AIOCBInfo->get_aio_context(), Kevin Wolf, 2023/09/15
- [PULL 21/28] block: Mark bdrv_add/del_child() and caller GRAPH_WRLOCK,
Kevin Wolf <=
- [PULL 20/28] block: Mark bdrv_unref_child() GRAPH_WRLOCK, Kevin Wolf, 2023/09/15
- [PULL 28/28] block-coroutine-wrapper: use qemu_get_current_aio_context(), Kevin Wolf, 2023/09/15
- Re: [PULL 00/28] Block layer patches, Stefan Hajnoczi, 2023/09/18