Skip to content

Commit

Permalink
block: Take graph lock for most of .bdrv_open
Browse files Browse the repository at this point in the history 
Most implementations of .bdrv_open first open their file child (which is
an operation that internally takes the write lock and therefore we
shouldn't hold the graph lock while calling it), and afterwards many
operations that require holding the graph lock, e.g. for accessing
bs->file.

This changes block drivers that follow this pattern to take the graph
lock after opening the child node.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20231027155333.420094-24-kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
  • Loading branch information
@kevmw
kevmw committed Nov 8, 2023
1 parent 65ff757 commit a4b740d
Show file tree
Hide file tree
Showing 16 changed files with 60 additions and 20 deletions.
16 changes: 10 additions & 6 deletions block/blkdebug.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,8 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
goto out;
}

bdrv_graph_rdlock_main_loop();

bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED |
(BDRV_REQ_FUA & bs->file->bs->supported_write_flags);
bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED |
Expand All @@ -520,7 +522,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
if (s->align && (s->align >= INT_MAX || !is_power_of_2(s->align))) {
error_setg(errp, "Cannot meet constraints with align %" PRIu64,
s->align);
goto out;
goto out_rdlock;
}
align = MAX(s->align, bs->file->bs->bl.request_alignment);

Expand All @@ -530,7 +532,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
!QEMU_IS_ALIGNED(s->max_transfer, align))) {
error_setg(errp, "Cannot meet constraints with max-transfer %" PRIu64,
s->max_transfer);
goto out;
goto out_rdlock;
}

s->opt_write_zero = qemu_opt_get_size(opts, "opt-write-zero", 0);
Expand All @@ -539,7 +541,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
!QEMU_IS_ALIGNED(s->opt_write_zero, align))) {
error_setg(errp, "Cannot meet constraints with opt-write-zero %" PRIu64,
s->opt_write_zero);
goto out;
goto out_rdlock;
}

s->max_write_zero = qemu_opt_get_size(opts, "max-write-zero", 0);
Expand All @@ -549,7 +551,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
MAX(s->opt_write_zero, align)))) {
error_setg(errp, "Cannot meet constraints with max-write-zero %" PRIu64,
s->max_write_zero);
goto out;
goto out_rdlock;
}

s->opt_discard = qemu_opt_get_size(opts, "opt-discard", 0);
Expand All @@ -558,7 +560,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
!QEMU_IS_ALIGNED(s->opt_discard, align))) {
error_setg(errp, "Cannot meet constraints with opt-discard %" PRIu64,
s->opt_discard);
goto out;
goto out_rdlock;
}

s->max_discard = qemu_opt_get_size(opts, "max-discard", 0);
Expand All @@ -568,12 +570,14 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
MAX(s->opt_discard, align)))) {
error_setg(errp, "Cannot meet constraints with max-discard %" PRIu64,
s->max_discard);
goto out;
goto out_rdlock;
}

bdrv_debug_event(bs, BLKDBG_NONE);

ret = 0;
out_rdlock:
bdrv_graph_rdunlock_main_loop();
out:
if (ret < 0) {
qemu_mutex_destroy(&s->lock);
Expand Down
4 changes: 4 additions & 0 deletions block/bochs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
struct bochs_header bochs;
int ret;

GLOBAL_STATE_CODE();

/* No write support yet */
bdrv_graph_rdlock_main_loop();
ret = bdrv_apply_auto_read_only(bs, NULL, errp);
Expand All @@ -118,6 +120,8 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
return ret;
}

GRAPH_RDLOCK_GUARD_MAINLOOP();

ret = bdrv_pread(bs->file, 0, sizeof(bochs), &bochs, 0);
if (ret < 0) {
return ret;
Expand Down
4 changes: 4 additions & 0 deletions block/cloop.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags,
uint32_t offsets_size, max_compressed_block_size = 1, i;
int ret;

GLOBAL_STATE_CODE();

bdrv_graph_rdlock_main_loop();
ret = bdrv_apply_auto_read_only(bs, NULL, errp);
bdrv_graph_rdunlock_main_loop();
Expand All @@ -79,6 +81,8 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags,
return ret;
}

GRAPH_RDLOCK_GUARD_MAINLOOP();

/* read header */
ret = bdrv_pread(bs->file, 128, 4, &s->block_size, 0);
if (ret < 0) {
Expand Down
2 changes: 2 additions & 0 deletions block/copy-before-write.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,8 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
return -EINVAL;
}

GRAPH_RDLOCK_GUARD_MAINLOOP();

ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);

Expand Down
4 changes: 2 additions & 2 deletions block/copy-on-read.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ cor_open(BlockDriverState *bs, QDict *options, int flags, Error **errp)
return ret;
}

GRAPH_RDLOCK_GUARD_MAINLOOP();

bs->supported_read_flags = BDRV_REQ_PREFETCH;

bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED |
Expand All @@ -61,8 +63,6 @@ cor_open(BlockDriverState *bs, QDict *options, int flags, Error **errp)
bs->file->bs->supported_zero_flags);

if (bottom_node) {
GRAPH_RDLOCK_GUARD_MAINLOOP();

bottom_bs = bdrv_find_node(bottom_node);
if (!bottom_bs) {
error_setg(errp, "Bottom node '%s' not found", bottom_node);
Expand Down
4 changes: 4 additions & 0 deletions block/crypto.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,11 +263,15 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
unsigned int cflags = 0;
QDict *cryptoopts = NULL;

GLOBAL_STATE_CODE();

ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
if (ret < 0) {
return ret;
}

GRAPH_RDLOCK_GUARD_MAINLOOP();

bs->supported_write_flags = BDRV_REQ_FUA &
bs->file->bs->supported_write_flags;

Expand Down
5 changes: 5 additions & 0 deletions block/dmg.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,8 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
int64_t offset;
int ret;

GLOBAL_STATE_CODE();

bdrv_graph_rdlock_main_loop();
ret = bdrv_apply_auto_read_only(bs, NULL, errp);
bdrv_graph_rdunlock_main_loop();
Expand All @@ -463,6 +465,9 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
if (ret < 0) {
return ret;
}

GRAPH_RDLOCK_GUARD_MAINLOOP();

/*
* NB: if uncompress submodules are absent,
* ie block_module_load return value == 0, the function pointers
Expand Down
2 changes: 2 additions & 0 deletions block/filter-compress.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ static int compress_open(BlockDriverState *bs, QDict *options, int flags,
return ret;
}

GRAPH_RDLOCK_GUARD_MAINLOOP();

if (!bs->file->bs->drv || !block_driver_can_compress(bs->file->bs->drv)) {
error_setg(errp,
"Compression is not supported for underlying format: %s",
Expand Down
4 changes: 2 additions & 2 deletions block/parallels.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1255,6 +1255,8 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
return ret;
}

GRAPH_RDLOCK_GUARD_MAINLOOP();

file_nb_sectors = bdrv_nb_sectors(bs->file->bs);
if (file_nb_sectors < 0) {
return -EINVAL;
Expand Down Expand Up @@ -1359,11 +1361,9 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
bitmap_new(DIV_ROUND_UP(s->header_size, s->bat_dirty_block));

/* Disable migration until bdrv_activate method is added */
bdrv_graph_rdlock_main_loop();
error_setg(&s->migration_blocker, "The Parallels format used by node '%s' "
"does not support live migration",
bdrv_get_device_or_node_name(bs));
bdrv_graph_rdunlock_main_loop();

ret = migrate_add_blocker_normal(&s->migration_blocker, errp);
if (ret < 0) {
Expand Down
4 changes: 4 additions & 0 deletions block/preallocate.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ static int preallocate_open(BlockDriverState *bs, QDict *options, int flags,
BDRVPreallocateState *s = bs->opaque;
int ret;

GLOBAL_STATE_CODE();

/*
* s->data_end and friends should be initialized on permission update.
* For this to work, mark them invalid.
Expand All @@ -155,6 +157,8 @@ static int preallocate_open(BlockDriverState *bs, QDict *options, int flags,
return ret;
}

GRAPH_RDLOCK_GUARD_MAINLOOP();

if (!preallocate_absorb_opts(&s->opts, options, bs->file->bs, errp)) {
return -EINVAL;
}
Expand Down
11 changes: 7 additions & 4 deletions block/qcow.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,11 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,

ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
if (ret < 0) {
goto fail;
goto fail_unlocked;
}

bdrv_graph_rdlock_main_loop();

ret = bdrv_pread(bs->file, 0, sizeof(header), &header, 0);
if (ret < 0) {
goto fail;
Expand Down Expand Up @@ -301,11 +303,9 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
}

/* Disable migration when qcow images are used */
bdrv_graph_rdlock_main_loop();
error_setg(&s->migration_blocker, "The qcow format used by node '%s' "
"does not support live migration",
bdrv_get_device_or_node_name(bs));
bdrv_graph_rdunlock_main_loop();

ret = migrate_add_blocker_normal(&s->migration_blocker, errp);
if (ret < 0) {
Expand All @@ -315,9 +315,12 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
qobject_unref(encryptopts);
qapi_free_QCryptoBlockOpenOptions(crypto_opts);
qemu_co_mutex_init(&s->lock);
bdrv_graph_rdunlock_main_loop();
return 0;

fail:
fail:
bdrv_graph_rdunlock_main_loop();
fail_unlocked:
g_free(s->l1_table);
qemu_vfree(s->l2_cache);
g_free(s->cluster_cache);
Expand Down
6 changes: 4 additions & 2 deletions block/raw-format.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,8 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
BdrvChildRole file_role;
int ret;

GLOBAL_STATE_CODE();

ret = raw_read_options(options, &offset, &has_size, &size, errp);
if (ret < 0) {
return ret;
Expand All @@ -490,6 +492,8 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,

bdrv_open_child(NULL, options, "file", bs, &child_of_bds,
file_role, false, errp);

GRAPH_RDLOCK_GUARD_MAINLOOP();
if (!bs->file) {
return -EINVAL;
}
Expand All @@ -504,9 +508,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
BDRV_REQ_ZERO_WRITE;

if (bs->probed && !bdrv_is_read_only(bs)) {
bdrv_graph_rdlock_main_loop();
bdrv_refresh_filename(bs->file->bs);
bdrv_graph_rdunlock_main_loop();
fprintf(stderr,
"WARNING: Image format was not specified for '%s' and probing "
"guessed raw.\n"
Expand Down
3 changes: 3 additions & 0 deletions block/snapshot-access.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ static int snapshot_access_open(BlockDriverState *bs, QDict *options, int flags,
bdrv_open_child(NULL, options, "file", bs, &child_of_bds,
BDRV_CHILD_DATA | BDRV_CHILD_PRIMARY,
false, errp);

GRAPH_RDLOCK_GUARD_MAINLOOP();

if (!bs->file) {
return -EINVAL;
}
Expand Down
3 changes: 3 additions & 0 deletions block/throttle.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ static int throttle_open(BlockDriverState *bs, QDict *options,
if (ret < 0) {
return ret;
}

GRAPH_RDLOCK_GUARD_MAINLOOP();

bs->supported_write_flags = bs->file->bs->supported_write_flags |
BDRV_REQ_WRITE_UNCHANGED;
bs->supported_zero_flags = bs->file->bs->supported_zero_flags |
Expand Down
4 changes: 2 additions & 2 deletions block/vdi.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,8 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
return ret;
}

GRAPH_RDLOCK_GUARD_MAINLOOP();

logout("\n");

ret = bdrv_pread(bs->file, 0, sizeof(header), &header, 0);
Expand Down Expand Up @@ -492,11 +494,9 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
}

/* Disable migration when vdi images are used */
bdrv_graph_rdlock_main_loop();
error_setg(&s->migration_blocker, "The vdi format used by node '%s' "
"does not support live migration",
bdrv_get_device_or_node_name(bs));
bdrv_graph_rdunlock_main_loop();

ret = migrate_add_blocker_normal(&s->migration_blocker, errp);
if (ret < 0) {
Expand Down
4 changes: 2 additions & 2 deletions block/vpc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,8 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
return ret;
}

GRAPH_RDLOCK_GUARD_MAINLOOP();

opts = qemu_opts_create(&vpc_runtime_opts, NULL, 0, &error_abort);
if (!qemu_opts_absorb_qdict(opts, options, errp)) {
ret = -EINVAL;
Expand Down Expand Up @@ -446,11 +448,9 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
}

/* Disable migration when VHD images are used */
bdrv_graph_rdlock_main_loop();
error_setg(&s->migration_blocker, "The vpc format used by node '%s' "
"does not support live migration",
bdrv_get_device_or_node_name(bs));
bdrv_graph_rdunlock_main_loop();

ret = migrate_add_blocker_normal(&s->migration_blocker, errp);
if (ret < 0) {
Expand Down

0 comments on commit a4b740d

Please sign in to comment.