block: Mark bdrv_add/del_child() and caller GRAPH_WRLOCK
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>
This commit is contained in:
parent
32a8aba37e
commit
9def6082cf
@ -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,
|
||||
|
17
blockdev.c
17
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)
|
||||
|
@ -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);
|
||||
|
||||
/**
|
||||
*
|
||||
|
@ -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
|
||||
|
Loading…
Reference in New Issue
Block a user