1
0
mirror of https://gitlab.com/qemu-project/qemu synced 2024-07-08 20:17:27 +00:00

block: Protect bs->file with graph_lock

Almost all functions that access bs->file already take the graph
lock now. Add locking to the remaining users and finally annotate the
struct field itself as protected by the graph lock.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20231027155333.420094-25-kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This commit is contained in:
Kevin Wolf 2023-10-27 17:53:33 +02:00
parent a4b740db5e
commit 1f051dcbdf
15 changed files with 97 additions and 41 deletions

11
block.c
View File

@ -1707,12 +1707,14 @@ bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv, const char *node_name,
return 0; return 0;
open_failed: open_failed:
bs->drv = NULL; bs->drv = NULL;
bdrv_graph_wrlock(NULL);
if (bs->file != NULL) { if (bs->file != NULL) {
bdrv_graph_wrlock(NULL);
bdrv_unref_child(bs, bs->file); bdrv_unref_child(bs, bs->file);
bdrv_graph_wrunlock();
assert(!bs->file); assert(!bs->file);
} }
bdrv_graph_wrunlock();
g_free(bs->opaque); g_free(bs->opaque);
bs->opaque = NULL; bs->opaque = NULL;
return ret; return ret;
@ -1854,9 +1856,12 @@ static int bdrv_open_common(BlockDriverState *bs, BlockBackend *file,
Error *local_err = NULL; Error *local_err = NULL;
bool ro; bool ro;
GLOBAL_STATE_CODE();
bdrv_graph_rdlock_main_loop();
assert(bs->file == NULL); assert(bs->file == NULL);
assert(options != NULL && bs->options != options); assert(options != NULL && bs->options != options);
GLOBAL_STATE_CODE(); bdrv_graph_rdunlock_main_loop();
opts = qemu_opts_create(&bdrv_runtime_opts, NULL, 0, &error_abort); opts = qemu_opts_create(&bdrv_runtime_opts, NULL, 0, &error_abort);
if (!qemu_opts_absorb_qdict(opts, options, errp)) { if (!qemu_opts_absorb_qdict(opts, options, errp)) {

View File

@ -130,7 +130,13 @@ static int coroutine_fn GRAPH_RDLOCK blkreplay_co_flush(BlockDriverState *bs)
static int blkreplay_snapshot_goto(BlockDriverState *bs, static int blkreplay_snapshot_goto(BlockDriverState *bs,
const char *snapshot_id) const char *snapshot_id)
{ {
return bdrv_snapshot_goto(bs->file->bs, snapshot_id, NULL); BlockDriverState *file_bs;
bdrv_graph_rdlock_main_loop();
file_bs = bs->file->bs;
bdrv_graph_rdunlock_main_loop();
return bdrv_snapshot_goto(file_bs, snapshot_id, NULL);
} }
static BlockDriver bdrv_blkreplay = { static BlockDriver bdrv_blkreplay = {

View File

@ -203,7 +203,7 @@ static int coroutine_fn GRAPH_RDLOCK cbw_co_flush(BlockDriverState *bs)
* It's guaranteed that guest writes will not interact in the region until * It's guaranteed that guest writes will not interact in the region until
* cbw_snapshot_read_unlock() called. * cbw_snapshot_read_unlock() called.
*/ */
static coroutine_fn BlockReq * static BlockReq * coroutine_fn GRAPH_RDLOCK
cbw_snapshot_read_lock(BlockDriverState *bs, int64_t offset, int64_t bytes, cbw_snapshot_read_lock(BlockDriverState *bs, int64_t offset, int64_t bytes,
int64_t *pnum, BdrvChild **file) int64_t *pnum, BdrvChild **file)
{ {

View File

@ -65,6 +65,9 @@ static int block_crypto_read_func(QCryptoBlock *block,
BlockDriverState *bs = opaque; BlockDriverState *bs = opaque;
ssize_t ret; ssize_t ret;
GLOBAL_STATE_CODE();
GRAPH_RDLOCK_GUARD_MAINLOOP();
ret = bdrv_pread(bs->file, offset, buflen, buf, 0); ret = bdrv_pread(bs->file, offset, buflen, buf, 0);
if (ret < 0) { if (ret < 0) {
error_setg_errno(errp, -ret, "Could not read encryption header"); error_setg_errno(errp, -ret, "Could not read encryption header");
@ -83,6 +86,9 @@ static int block_crypto_write_func(QCryptoBlock *block,
BlockDriverState *bs = opaque; BlockDriverState *bs = opaque;
ssize_t ret; ssize_t ret;
GLOBAL_STATE_CODE();
GRAPH_RDLOCK_GUARD_MAINLOOP();
ret = bdrv_pwrite(bs->file, offset, buflen, buf, 0); ret = bdrv_pwrite(bs->file, offset, buflen, buf, 0);
if (ret < 0) { if (ret < 0) {
error_setg_errno(errp, -ret, "Could not write encryption header"); error_setg_errno(errp, -ret, "Could not write encryption header");

View File

@ -70,7 +70,8 @@ static int dmg_probe(const uint8_t *buf, int buf_size, const char *filename)
return 0; return 0;
} }
static int read_uint64(BlockDriverState *bs, int64_t offset, uint64_t *result) static int GRAPH_RDLOCK
read_uint64(BlockDriverState *bs, int64_t offset, uint64_t *result)
{ {
uint64_t buffer; uint64_t buffer;
int ret; int ret;
@ -84,7 +85,8 @@ static int read_uint64(BlockDriverState *bs, int64_t offset, uint64_t *result)
return 0; return 0;
} }
static int read_uint32(BlockDriverState *bs, int64_t offset, uint32_t *result) static int GRAPH_RDLOCK
read_uint32(BlockDriverState *bs, int64_t offset, uint32_t *result)
{ {
uint32_t buffer; uint32_t buffer;
int ret; int ret;
@ -321,8 +323,9 @@ fail:
return ret; return ret;
} }
static int dmg_read_resource_fork(BlockDriverState *bs, DmgHeaderState *ds, static int GRAPH_RDLOCK
uint64_t info_begin, uint64_t info_length) dmg_read_resource_fork(BlockDriverState *bs, DmgHeaderState *ds,
uint64_t info_begin, uint64_t info_length)
{ {
BDRVDMGState *s = bs->opaque; BDRVDMGState *s = bs->opaque;
int ret; int ret;
@ -388,8 +391,9 @@ fail:
return ret; return ret;
} }
static int dmg_read_plist_xml(BlockDriverState *bs, DmgHeaderState *ds, static int GRAPH_RDLOCK
uint64_t info_begin, uint64_t info_length) dmg_read_plist_xml(BlockDriverState *bs, DmgHeaderState *ds,
uint64_t info_begin, uint64_t info_length)
{ {
BDRVDMGState *s = bs->opaque; BDRVDMGState *s = bs->opaque;
int ret; int ret;

View File

@ -59,11 +59,10 @@ typedef struct ParallelsDirtyBitmapFeature {
} QEMU_PACKED ParallelsDirtyBitmapFeature; } QEMU_PACKED ParallelsDirtyBitmapFeature;
/* Given L1 table read bitmap data from the image and populate @bitmap */ /* Given L1 table read bitmap data from the image and populate @bitmap */
static int parallels_load_bitmap_data(BlockDriverState *bs, static int GRAPH_RDLOCK
const uint64_t *l1_table, parallels_load_bitmap_data(BlockDriverState *bs, const uint64_t *l1_table,
uint32_t l1_size, uint32_t l1_size, BdrvDirtyBitmap *bitmap,
BdrvDirtyBitmap *bitmap, Error **errp)
Error **errp)
{ {
BDRVParallelsState *s = bs->opaque; BDRVParallelsState *s = bs->opaque;
int ret = 0; int ret = 0;
@ -120,10 +119,9 @@ finish:
* @data buffer (of @data_size size) is the Dirty bitmaps feature which * @data buffer (of @data_size size) is the Dirty bitmaps feature which
* consists of ParallelsDirtyBitmapFeature followed by L1 table. * consists of ParallelsDirtyBitmapFeature followed by L1 table.
*/ */
static BdrvDirtyBitmap *parallels_load_bitmap(BlockDriverState *bs, static BdrvDirtyBitmap * GRAPH_RDLOCK
uint8_t *data, parallels_load_bitmap(BlockDriverState *bs, uint8_t *data, size_t data_size,
size_t data_size, Error **errp)
Error **errp)
{ {
int ret; int ret;
ParallelsDirtyBitmapFeature bf; ParallelsDirtyBitmapFeature bf;
@ -183,8 +181,9 @@ static BdrvDirtyBitmap *parallels_load_bitmap(BlockDriverState *bs,
return bitmap; return bitmap;
} }
static int parallels_parse_format_extension(BlockDriverState *bs, static int GRAPH_RDLOCK
uint8_t *ext_cluster, Error **errp) parallels_parse_format_extension(BlockDriverState *bs, uint8_t *ext_cluster,
Error **errp)
{ {
BDRVParallelsState *s = bs->opaque; BDRVParallelsState *s = bs->opaque;
int ret; int ret;

View File

@ -200,7 +200,7 @@ static int mark_used(BlockDriverState *bs, unsigned long *bitmap,
* bitmap anyway, as much as we can. This information will be used for * bitmap anyway, as much as we can. This information will be used for
* error resolution. * error resolution.
*/ */
static int parallels_fill_used_bitmap(BlockDriverState *bs) static int GRAPH_RDLOCK parallels_fill_used_bitmap(BlockDriverState *bs)
{ {
BDRVParallelsState *s = bs->opaque; BDRVParallelsState *s = bs->opaque;
int64_t payload_bytes; int64_t payload_bytes;
@ -1185,7 +1185,7 @@ static int parallels_probe(const uint8_t *buf, int buf_size,
return 0; return 0;
} }
static int parallels_update_header(BlockDriverState *bs) static int GRAPH_RDLOCK parallels_update_header(BlockDriverState *bs)
{ {
BDRVParallelsState *s = bs->opaque; BDRVParallelsState *s = bs->opaque;
unsigned size = MAX(bdrv_opt_mem_align(bs->file->bs), unsigned size = MAX(bdrv_opt_mem_align(bs->file->bs),
@ -1428,6 +1428,8 @@ static void parallels_close(BlockDriverState *bs)
{ {
BDRVParallelsState *s = bs->opaque; BDRVParallelsState *s = bs->opaque;
GRAPH_RDLOCK_GUARD_MAINLOOP();
if ((bs->open_flags & BDRV_O_RDWR) && !(bs->open_flags & BDRV_O_INACTIVE)) { if ((bs->open_flags & BDRV_O_RDWR) && !(bs->open_flags & BDRV_O_INACTIVE)) {
s->header->inuse = 0; s->header->inuse = 0;
parallels_update_header(bs); parallels_update_header(bs);

View File

@ -90,7 +90,8 @@ typedef struct BDRVParallelsState {
Error *migration_blocker; Error *migration_blocker;
} BDRVParallelsState; } BDRVParallelsState;
int parallels_read_format_extension(BlockDriverState *bs, int GRAPH_RDLOCK
int64_t ext_off, Error **errp); parallels_read_format_extension(BlockDriverState *bs, int64_t ext_off,
Error **errp);
#endif #endif

View File

@ -173,7 +173,8 @@ static int preallocate_open(BlockDriverState *bs, QDict *options, int flags,
return 0; return 0;
} }
static int preallocate_truncate_to_real_size(BlockDriverState *bs, Error **errp) static int GRAPH_RDLOCK
preallocate_truncate_to_real_size(BlockDriverState *bs, Error **errp)
{ {
BDRVPreallocateState *s = bs->opaque; BDRVPreallocateState *s = bs->opaque;
int ret; int ret;
@ -204,6 +205,9 @@ static void preallocate_close(BlockDriverState *bs)
{ {
BDRVPreallocateState *s = bs->opaque; BDRVPreallocateState *s = bs->opaque;
GLOBAL_STATE_CODE();
GRAPH_RDLOCK_GUARD_MAINLOOP();
qemu_bh_cancel(s->drop_resize_bh); qemu_bh_cancel(s->drop_resize_bh);
qemu_bh_delete(s->drop_resize_bh); qemu_bh_delete(s->drop_resize_bh);
@ -227,6 +231,9 @@ static int preallocate_reopen_prepare(BDRVReopenState *reopen_state,
PreallocateOpts *opts = g_new0(PreallocateOpts, 1); PreallocateOpts *opts = g_new0(PreallocateOpts, 1);
int ret; int ret;
GLOBAL_STATE_CODE();
GRAPH_RDLOCK_GUARD_MAINLOOP();
if (!preallocate_absorb_opts(opts, reopen_state->options, if (!preallocate_absorb_opts(opts, reopen_state->options,
reopen_state->bs->file->bs, errp)) { reopen_state->bs->file->bs, errp)) {
g_free(opts); g_free(opts);
@ -287,7 +294,7 @@ static bool can_write_resize(uint64_t perm)
return (perm & BLK_PERM_WRITE) && (perm & BLK_PERM_RESIZE); return (perm & BLK_PERM_WRITE) && (perm & BLK_PERM_RESIZE);
} }
static bool has_prealloc_perms(BlockDriverState *bs) static bool GRAPH_RDLOCK has_prealloc_perms(BlockDriverState *bs)
{ {
BDRVPreallocateState *s = bs->opaque; BDRVPreallocateState *s = bs->opaque;
@ -503,7 +510,8 @@ preallocate_co_getlength(BlockDriverState *bs)
return ret; return ret;
} }
static int preallocate_drop_resize(BlockDriverState *bs, Error **errp) static int GRAPH_RDLOCK
preallocate_drop_resize(BlockDriverState *bs, Error **errp)
{ {
BDRVPreallocateState *s = bs->opaque; BDRVPreallocateState *s = bs->opaque;
int ret; int ret;
@ -529,15 +537,16 @@ static int preallocate_drop_resize(BlockDriverState *bs, Error **errp)
*/ */
s->data_end = s->file_end = s->zero_start = -EINVAL; s->data_end = s->file_end = s->zero_start = -EINVAL;
bdrv_graph_rdlock_main_loop();
bdrv_child_refresh_perms(bs, bs->file, NULL); bdrv_child_refresh_perms(bs, bs->file, NULL);
bdrv_graph_rdunlock_main_loop();
return 0; return 0;
} }
static void preallocate_drop_resize_bh(void *opaque) static void preallocate_drop_resize_bh(void *opaque)
{ {
GLOBAL_STATE_CODE();
GRAPH_RDLOCK_GUARD_MAINLOOP();
/* /*
* In case of errors, we'll simply keep the exclusive lock on the image * In case of errors, we'll simply keep the exclusive lock on the image
* indefinitely. * indefinitely.

View File

@ -612,7 +612,7 @@ static int bdrv_qed_reopen_prepare(BDRVReopenState *state,
return 0; return 0;
} }
static void bdrv_qed_close(BlockDriverState *bs) static void GRAPH_RDLOCK bdrv_qed_do_close(BlockDriverState *bs)
{ {
BDRVQEDState *s = bs->opaque; BDRVQEDState *s = bs->opaque;
@ -631,6 +631,14 @@ static void bdrv_qed_close(BlockDriverState *bs)
qemu_vfree(s->l1_table); qemu_vfree(s->l1_table);
} }
static void GRAPH_UNLOCKED bdrv_qed_close(BlockDriverState *bs)
{
GLOBAL_STATE_CODE();
GRAPH_RDLOCK_GUARD_MAINLOOP();
bdrv_qed_do_close(bs);
}
static int coroutine_fn GRAPH_UNLOCKED static int coroutine_fn GRAPH_UNLOCKED
bdrv_qed_co_create(BlockdevCreateOptions *opts, Error **errp) bdrv_qed_co_create(BlockdevCreateOptions *opts, Error **errp)
{ {
@ -1574,7 +1582,7 @@ bdrv_qed_co_invalidate_cache(BlockDriverState *bs, Error **errp)
BDRVQEDState *s = bs->opaque; BDRVQEDState *s = bs->opaque;
int ret; int ret;
bdrv_qed_close(bs); bdrv_qed_do_close(bs);
bdrv_qed_init_state(bs); bdrv_qed_init_state(bs);
qemu_co_mutex_lock(&s->table_lock); qemu_co_mutex_lock(&s->table_lock);

View File

@ -185,7 +185,7 @@ enum {
/** /**
* Header functions * Header functions
*/ */
int qed_write_header_sync(BDRVQEDState *s); int GRAPH_RDLOCK qed_write_header_sync(BDRVQEDState *s);
/** /**
* L2 cache functions * L2 cache functions

View File

@ -95,9 +95,9 @@ end:
return ret; return ret;
} }
static int raw_apply_options(BlockDriverState *bs, BDRVRawState *s, static int GRAPH_RDLOCK
uint64_t offset, bool has_size, uint64_t size, raw_apply_options(BlockDriverState *bs, BDRVRawState *s, uint64_t offset,
Error **errp) bool has_size, uint64_t size, Error **errp)
{ {
int64_t real_size = 0; int64_t real_size = 0;
@ -145,6 +145,9 @@ static int raw_reopen_prepare(BDRVReopenState *reopen_state,
uint64_t offset, size; uint64_t offset, size;
int ret; int ret;
GLOBAL_STATE_CODE();
GRAPH_RDLOCK_GUARD_MAINLOOP();
assert(reopen_state != NULL); assert(reopen_state != NULL);
assert(reopen_state->bs != NULL); assert(reopen_state->bs != NULL);

View File

@ -311,7 +311,7 @@ static void GRAPH_UNLOCKED
secondary_do_checkpoint(BlockDriverState *bs, Error **errp) secondary_do_checkpoint(BlockDriverState *bs, Error **errp)
{ {
BDRVReplicationState *s = bs->opaque; BDRVReplicationState *s = bs->opaque;
BdrvChild *active_disk = bs->file; BdrvChild *active_disk;
Error *local_err = NULL; Error *local_err = NULL;
int ret; int ret;
@ -328,6 +328,7 @@ secondary_do_checkpoint(BlockDriverState *bs, Error **errp)
return; return;
} }
active_disk = bs->file;
if (!active_disk->bs->drv) { if (!active_disk->bs->drv) {
error_setg(errp, "Active disk %s is ejected", error_setg(errp, "Active disk %s is ejected",
active_disk->bs->node_name); active_disk->bs->node_name);
@ -755,11 +756,13 @@ static void replication_stop(ReplicationState *rs, bool failover, Error **errp)
return; return;
} }
bdrv_graph_rdlock_main_loop();
s->stage = BLOCK_REPLICATION_FAILOVER; s->stage = BLOCK_REPLICATION_FAILOVER;
s->commit_job = commit_active_start( s->commit_job = commit_active_start(
NULL, bs->file->bs, s->secondary_disk->bs, NULL, bs->file->bs, s->secondary_disk->bs,
JOB_INTERNAL, 0, BLOCKDEV_ON_ERROR_REPORT, JOB_INTERNAL, 0, BLOCKDEV_ON_ERROR_REPORT,
NULL, replication_done, bs, true, errp); NULL, replication_done, bs, true, errp);
bdrv_graph_rdunlock_main_loop();
break; break;
default: default:
aio_context_release(aio_context); aio_context_release(aio_context);

View File

@ -300,7 +300,8 @@ static void vmdk_free_last_extent(BlockDriverState *bs)
} }
/* Return -ve errno, or 0 on success and write CID into *pcid. */ /* Return -ve errno, or 0 on success and write CID into *pcid. */
static int vmdk_read_cid(BlockDriverState *bs, int parent, uint32_t *pcid) static int GRAPH_RDLOCK
vmdk_read_cid(BlockDriverState *bs, int parent, uint32_t *pcid)
{ {
char *desc; char *desc;
uint32_t cid; uint32_t cid;
@ -415,6 +416,9 @@ static int vmdk_reopen_prepare(BDRVReopenState *state,
BDRVVmdkReopenState *rs; BDRVVmdkReopenState *rs;
int i; int i;
GLOBAL_STATE_CODE();
GRAPH_RDLOCK_GUARD_MAINLOOP();
assert(state != NULL); assert(state != NULL);
assert(state->bs != NULL); assert(state->bs != NULL);
assert(state->opaque == NULL); assert(state->opaque == NULL);
@ -451,6 +455,9 @@ static void vmdk_reopen_commit(BDRVReopenState *state)
BDRVVmdkReopenState *rs = state->opaque; BDRVVmdkReopenState *rs = state->opaque;
int i; int i;
GLOBAL_STATE_CODE();
GRAPH_RDLOCK_GUARD_MAINLOOP();
for (i = 0; i < s->num_extents; i++) { for (i = 0; i < s->num_extents; i++) {
if (rs->extents_using_bs_file[i]) { if (rs->extents_using_bs_file[i]) {
s->extents[i].file = state->bs->file; s->extents[i].file = state->bs->file;
@ -465,7 +472,7 @@ static void vmdk_reopen_abort(BDRVReopenState *state)
vmdk_reopen_clean(state); vmdk_reopen_clean(state);
} }
static int vmdk_parent_open(BlockDriverState *bs) static int GRAPH_RDLOCK vmdk_parent_open(BlockDriverState *bs)
{ {
char *p_name; char *p_name;
char *desc; char *desc;
@ -2547,7 +2554,10 @@ vmdk_co_do_create(int64_t size,
ret = -EINVAL; ret = -EINVAL;
goto exit; goto exit;
} }
bdrv_graph_co_rdlock();
ret = vmdk_read_cid(blk_bs(backing), 0, &parent_cid); ret = vmdk_read_cid(blk_bs(backing), 0, &parent_cid);
bdrv_graph_co_rdunlock();
blk_co_unref(backing); blk_co_unref(backing);
if (ret) { if (ret) {
error_setg(errp, "Failed to read parent CID"); error_setg(errp, "Failed to read parent CID");

View File

@ -1181,7 +1181,7 @@ struct BlockDriverState {
*/ */
QLIST_HEAD(, BdrvChild GRAPH_RDLOCK_PTR) children; QLIST_HEAD(, BdrvChild GRAPH_RDLOCK_PTR) children;
BdrvChild * GRAPH_RDLOCK_PTR backing; BdrvChild * GRAPH_RDLOCK_PTR backing;
BdrvChild *file; BdrvChild * GRAPH_RDLOCK_PTR file;
QLIST_HEAD(, BdrvChild GRAPH_RDLOCK_PTR) parents; QLIST_HEAD(, BdrvChild GRAPH_RDLOCK_PTR) parents;