From 8f8973416e231bc1e45b2c4a3d419fbb8ef91c94 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 27 Oct 2023 17:53:30 +0200 Subject: [PATCH] qcow2: Take locks for accessing bs->file This updates the qcow2 code to add GRAPH_RDLOCK annotations for all places that read bs->file. Signed-off-by: Kevin Wolf Message-ID: <20231027155333.420094-22-kwolf@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block/qcow2-bitmap.c | 14 ++++++++------ block/qcow2-cluster.c | 25 +++++++++++++------------ block/qcow2.c | 13 +++++++++---- block/qcow2.h | 48 +++++++++++++++++++++++++++++------------------- 4 files changed, 59 insertions(+), 41 deletions(-) diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index 3058309c47..0e567ed588 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -105,7 +105,7 @@ static inline bool can_write(BlockDriverState *bs) return !bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE); } -static int update_header_sync(BlockDriverState *bs) +static int GRAPH_RDLOCK update_header_sync(BlockDriverState *bs) { int ret; @@ -221,8 +221,9 @@ clear_bitmap_table(BlockDriverState *bs, uint64_t *bitmap_table, } } -static int bitmap_table_load(BlockDriverState *bs, Qcow2BitmapTable *tb, - uint64_t **bitmap_table) +static int GRAPH_RDLOCK +bitmap_table_load(BlockDriverState *bs, Qcow2BitmapTable *tb, + uint64_t **bitmap_table) { int ret; BDRVQcow2State *s = bs->opaque; @@ -551,8 +552,9 @@ static uint32_t bitmap_list_count(Qcow2BitmapList *bm_list) * Get bitmap list from qcow2 image. Actually reads bitmap directory, * checks it and convert to bitmap list. */ -static Qcow2BitmapList *bitmap_list_load(BlockDriverState *bs, uint64_t offset, - uint64_t size, Error **errp) +static Qcow2BitmapList * GRAPH_RDLOCK +bitmap_list_load(BlockDriverState *bs, uint64_t offset, uint64_t size, + Error **errp) { int ret; BDRVQcow2State *s = bs->opaque; @@ -961,7 +963,7 @@ static void set_readonly_helper(gpointer bitmap, gpointer value) * If header_updated is not NULL then it is set appropriately regardless of * the return value. */ -bool coroutine_fn GRAPH_RDLOCK +bool coroutine_fn qcow2_load_dirty_bitmaps(BlockDriverState *bs, bool *header_updated, Error **errp) { diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 5af439bd11..ce8c0076b3 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -391,11 +391,10 @@ fail: * If the L2 entry is invalid return -errno and set @type to * QCOW2_SUBCLUSTER_INVALID. */ -static int qcow2_get_subcluster_range_type(BlockDriverState *bs, - uint64_t l2_entry, - uint64_t l2_bitmap, - unsigned sc_from, - QCow2SubclusterType *type) +static int GRAPH_RDLOCK +qcow2_get_subcluster_range_type(BlockDriverState *bs, uint64_t l2_entry, + uint64_t l2_bitmap, unsigned sc_from, + QCow2SubclusterType *type) { BDRVQcow2State *s = bs->opaque; uint32_t val; @@ -442,9 +441,10 @@ static int qcow2_get_subcluster_range_type(BlockDriverState *bs, * On failure return -errno and update @l2_index to point to the * invalid entry. */ -static int count_contiguous_subclusters(BlockDriverState *bs, int nb_clusters, - unsigned sc_index, uint64_t *l2_slice, - unsigned *l2_index) +static int GRAPH_RDLOCK +count_contiguous_subclusters(BlockDriverState *bs, int nb_clusters, + unsigned sc_index, uint64_t *l2_slice, + unsigned *l2_index) { BDRVQcow2State *s = bs->opaque; int i, count = 0; @@ -1329,7 +1329,8 @@ calculate_l2_meta(BlockDriverState *bs, uint64_t host_cluster_offset, * requires a new allocation (that is, if the cluster is unallocated * or has refcount > 1 and therefore cannot be written in-place). */ -static bool cluster_needs_new_alloc(BlockDriverState *bs, uint64_t l2_entry) +static bool GRAPH_RDLOCK +cluster_needs_new_alloc(BlockDriverState *bs, uint64_t l2_entry) { switch (qcow2_get_cluster_type(bs, l2_entry)) { case QCOW2_CLUSTER_NORMAL: @@ -1360,9 +1361,9 @@ static bool cluster_needs_new_alloc(BlockDriverState *bs, uint64_t l2_entry) * allocated and can be overwritten in-place (this includes clusters * of type QCOW2_CLUSTER_ZERO_ALLOC). */ -static int count_single_write_clusters(BlockDriverState *bs, int nb_clusters, - uint64_t *l2_slice, int l2_index, - bool new_alloc) +static int GRAPH_RDLOCK +count_single_write_clusters(BlockDriverState *bs, int nb_clusters, + uint64_t *l2_slice, int l2_index, bool new_alloc) { BDRVQcow2State *s = bs->opaque; uint64_t l2_entry = get_l2_entry(s, l2_slice, l2_index); diff --git a/block/qcow2.c b/block/qcow2.c index 7ab399ffc5..cf2468858f 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -95,9 +95,10 @@ static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename) } -static int qcow2_crypto_hdr_read_func(QCryptoBlock *block, size_t offset, - uint8_t *buf, size_t buflen, - void *opaque, Error **errp) +static int GRAPH_RDLOCK +qcow2_crypto_hdr_read_func(QCryptoBlock *block, size_t offset, + uint8_t *buf, size_t buflen, + void *opaque, Error **errp) { BlockDriverState *bs = opaque; BDRVQcow2State *s = bs->opaque; @@ -156,7 +157,7 @@ qcow2_crypto_hdr_init_func(QCryptoBlock *block, size_t headerlen, void *opaque, /* The graph lock must be held when called in coroutine context */ -static int coroutine_mixed_fn +static int coroutine_mixed_fn GRAPH_RDLOCK qcow2_crypto_hdr_write_func(QCryptoBlock *block, size_t offset, const uint8_t *buf, size_t buflen, void *opaque, Error **errp) @@ -2029,6 +2030,8 @@ static void qcow2_reopen_commit(BDRVReopenState *state) { BDRVQcow2State *s = state->bs->opaque; + GRAPH_RDLOCK_GUARD_MAINLOOP(); + qcow2_update_options_commit(state->bs, state->opaque); if (!s->data_file) { /* @@ -2064,6 +2067,8 @@ static void qcow2_reopen_abort(BDRVReopenState *state) { BDRVQcow2State *s = state->bs->opaque; + GRAPH_RDLOCK_GUARD_MAINLOOP(); + if (!s->data_file) { /* * If we don't have an external data file, s->data_file was cleared by diff --git a/block/qcow2.h b/block/qcow2.h index 948335979f..a9e3481c6e 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -641,7 +641,7 @@ static inline void set_l2_bitmap(BDRVQcow2State *s, uint64_t *l2_slice, l2_slice[idx + 1] = cpu_to_be64(bitmap); } -static inline bool has_data_file(BlockDriverState *bs) +static inline bool GRAPH_RDLOCK has_data_file(BlockDriverState *bs) { BDRVQcow2State *s = bs->opaque; return (s->data_file != bs->file); @@ -709,8 +709,8 @@ static inline int64_t qcow2_vm_state_offset(BDRVQcow2State *s) return (int64_t)s->l1_vm_state_index << (s->cluster_bits + s->l2_bits); } -static inline QCow2ClusterType qcow2_get_cluster_type(BlockDriverState *bs, - uint64_t l2_entry) +static inline QCow2ClusterType GRAPH_RDLOCK +qcow2_get_cluster_type(BlockDriverState *bs, uint64_t l2_entry) { BDRVQcow2State *s = bs->opaque; @@ -743,7 +743,7 @@ static inline QCow2ClusterType qcow2_get_cluster_type(BlockDriverState *bs, * (this checks the whole entry and bitmap, not only the bits related * to subcluster @sc_index). */ -static inline +static inline GRAPH_RDLOCK QCow2SubclusterType qcow2_get_subcluster_type(BlockDriverState *bs, uint64_t l2_entry, uint64_t l2_bitmap, @@ -834,9 +834,9 @@ int64_t qcow2_refcount_metadata_size(int64_t clusters, size_t cluster_size, int refcount_order, bool generous_increase, uint64_t *refblock_count); -int qcow2_mark_dirty(BlockDriverState *bs); -int qcow2_mark_corrupt(BlockDriverState *bs); -int qcow2_update_header(BlockDriverState *bs); +int GRAPH_RDLOCK qcow2_mark_dirty(BlockDriverState *bs); +int GRAPH_RDLOCK qcow2_mark_corrupt(BlockDriverState *bs); +int GRAPH_RDLOCK qcow2_update_header(BlockDriverState *bs); void GRAPH_RDLOCK qcow2_signal_corruption(BlockDriverState *bs, bool fatal, int64_t offset, @@ -890,10 +890,11 @@ int GRAPH_RDLOCK qcow2_write_caches(BlockDriverState *bs); int coroutine_fn qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix); -void qcow2_process_discards(BlockDriverState *bs, int ret); +void GRAPH_RDLOCK qcow2_process_discards(BlockDriverState *bs, int ret); -int qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset, - int64_t size); +int GRAPH_RDLOCK +qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset, + int64_t size); int GRAPH_RDLOCK qcow2_pre_write_overlap_check(BlockDriverState *bs, int ign, int64_t offset, int64_t size, bool data_file); @@ -939,8 +940,9 @@ qcow2_alloc_host_offset(BlockDriverState *bs, uint64_t offset, int coroutine_fn GRAPH_RDLOCK qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs, uint64_t offset, int compressed_size, uint64_t *host_offset); -void qcow2_parse_compressed_l2_entry(BlockDriverState *bs, uint64_t l2_entry, - uint64_t *coffset, int *csize); +void GRAPH_RDLOCK +qcow2_parse_compressed_l2_entry(BlockDriverState *bs, uint64_t l2_entry, + uint64_t *coffset, int *csize); int coroutine_fn GRAPH_RDLOCK qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m); @@ -993,8 +995,9 @@ qcow2_check_fix_snapshot_table(BlockDriverState *bs, BdrvCheckResult *result, BdrvCheckMode fix); /* qcow2-cache.c functions */ -Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables, - unsigned table_size); +Qcow2Cache * GRAPH_RDLOCK +qcow2_cache_create(BlockDriverState *bs, int num_tables, unsigned table_size); + int qcow2_cache_destroy(Qcow2Cache *c); void qcow2_cache_entry_mark_dirty(Qcow2Cache *c, void *table); @@ -1020,17 +1023,24 @@ void *qcow2_cache_is_table_offset(Qcow2Cache *c, uint64_t offset); void qcow2_cache_discard(Qcow2Cache *c, void *table); /* qcow2-bitmap.c functions */ -int coroutine_fn +int coroutine_fn GRAPH_RDLOCK qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res, void **refcount_table, int64_t *refcount_table_size); + bool coroutine_fn GRAPH_RDLOCK -qcow2_load_dirty_bitmaps(BlockDriverState *bs, bool *header_updated, Error **errp); -bool qcow2_get_bitmap_info_list(BlockDriverState *bs, - Qcow2BitmapInfoList **info_list, Error **errp); +qcow2_load_dirty_bitmaps(BlockDriverState *bs, bool *header_updated, + Error **errp); + +bool GRAPH_RDLOCK +qcow2_get_bitmap_info_list(BlockDriverState *bs, + Qcow2BitmapInfoList **info_list, Error **errp); + int GRAPH_RDLOCK qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp); int GRAPH_RDLOCK qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Error **errp); -int coroutine_fn qcow2_truncate_bitmaps_check(BlockDriverState *bs, Error **errp); + +int coroutine_fn GRAPH_RDLOCK +qcow2_truncate_bitmaps_check(BlockDriverState *bs, Error **errp); bool GRAPH_RDLOCK qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, bool release_stored, -- 2.11.0