From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, stefanha@redhat.com, eesposit@redhat.com,
eblake@redhat.com, pbonzini@redhat.com,
vsementsov@yandex-team.ru, qemu-devel@nongnu.org
Subject: [PATCH 21/24] qcow2: Take locks for accessing bs->file
Date: Fri, 27 Oct 2023 17:53:30 +0200 [thread overview]
Message-ID: <20231027155333.420094-22-kwolf@redhat.com> (raw)
In-Reply-To: <20231027155333.420094-1-kwolf@redhat.com>
This updates the qcow2 code to add GRAPH_RDLOCK annotations for all
places that read bs->file.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/qcow2.h | 48 ++++++++++++++++++++++++++-----------------
block/qcow2-bitmap.c | 14 +++++++------
block/qcow2-cluster.c | 25 +++++++++++-----------
block/qcow2.c | 13 ++++++++----
4 files changed, 59 insertions(+), 41 deletions(-)
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,
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 904f00d1b3..11cf1d24e6 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 7f022d5d78..dc42bbca3b 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
--
2.41.0
next prev parent reply other threads:[~2023-10-27 15:54 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-27 15:53 [PATCH 00/24] block: Graph locking part 6 (bs->file/backing) Kevin Wolf
2023-10-27 15:53 ` [PATCH 01/24] block: Mark bdrv_probe_blocksizes() and callers GRAPH_RDLOCK Kevin Wolf
2023-10-27 19:40 ` Eric Blake
2023-10-27 15:53 ` [PATCH 02/24] block: Mark bdrv_has_zero_init() " Kevin Wolf
2023-10-27 19:59 ` Eric Blake
2023-10-27 15:53 ` [PATCH 03/24] block: Mark bdrv_filter_bs() " Kevin Wolf
2023-10-27 20:02 ` Eric Blake
2023-10-27 20:45 ` Eric Blake
2023-10-27 15:53 ` [PATCH 04/24] block: Mark bdrv_root_attach_child() GRAPH_WRLOCK Kevin Wolf
2023-10-27 20:22 ` Eric Blake
2023-11-03 9:45 ` Kevin Wolf
2023-11-03 12:33 ` Eric Blake
2023-10-27 15:53 ` [PATCH 05/24] block: Mark block_job_add_bdrv() GRAPH_WRLOCK Kevin Wolf
2023-10-27 20:27 ` Eric Blake
2023-10-27 15:53 ` [PATCH 06/24] block: Mark bdrv_filter_or_cow_bs() and callers GRAPH_RDLOCK Kevin Wolf
2023-10-27 20:33 ` Eric Blake
2023-10-27 15:53 ` [PATCH 07/24] block: Mark bdrv_skip_implicit_filters() " Kevin Wolf
2023-10-27 20:37 ` Eric Blake
2023-10-27 15:53 ` [PATCH 08/24] block: Mark bdrv_skip_filters() " Kevin Wolf
2023-10-27 20:52 ` Eric Blake
2023-10-27 15:53 ` [PATCH 09/24] block: Mark bdrv_(un)freeze_backing_chain() " Kevin Wolf
2023-10-27 21:00 ` Eric Blake
2023-11-03 9:54 ` Kevin Wolf
2023-10-27 15:53 ` [PATCH 10/24] block: Mark bdrv_chain_contains() " Kevin Wolf
2023-10-27 21:17 ` Eric Blake
2023-10-27 15:53 ` [PATCH 11/24] block: Mark bdrv_filter_child() " Kevin Wolf
2023-10-27 21:19 ` Eric Blake
2023-10-27 15:53 ` [PATCH 12/24] block: Mark bdrv_cow_child() " Kevin Wolf
2023-10-27 21:20 ` Eric Blake
2023-10-27 15:53 ` [PATCH 13/24] block: Mark bdrv_set_backing_hd_drained() GRAPH_WRLOCK Kevin Wolf
2023-10-27 21:22 ` Eric Blake
2023-10-27 15:53 ` [PATCH 14/24] block: Inline bdrv_set_backing_noperm() Kevin Wolf
2023-10-27 21:23 ` Eric Blake
2023-10-27 15:53 ` [PATCH 15/24] block: Mark bdrv_replace_node_common() GRAPH_WRLOCK Kevin Wolf
2023-10-27 21:27 ` Eric Blake
2023-10-27 15:53 ` [PATCH 16/24] block: Mark bdrv_replace_node() GRAPH_WRLOCK Kevin Wolf
2023-10-27 21:33 ` Eric Blake
2023-11-03 10:32 ` Kevin Wolf
2023-11-03 12:37 ` Eric Blake
2023-10-27 15:53 ` [PATCH 17/24] block: Protect bs->backing with graph_lock Kevin Wolf
2023-10-27 21:46 ` Eric Blake
2023-10-27 15:53 ` [PATCH 18/24] blkverify: Add locking for request_fn Kevin Wolf
2023-10-30 13:51 ` Eric Blake
2023-10-27 15:53 ` [PATCH 19/24] block: Introduce bdrv_co_change_backing_file() Kevin Wolf
2023-10-30 13:57 ` Eric Blake
2023-11-03 10:33 ` Kevin Wolf
2023-11-03 12:38 ` Eric Blake
2023-10-27 15:53 ` [PATCH 20/24] block: Add missing GRAPH_RDLOCK annotations Kevin Wolf
2023-10-30 21:19 ` Eric Blake
2023-10-27 15:53 ` Kevin Wolf [this message]
2023-10-30 21:25 ` [PATCH 21/24] qcow2: Take locks for accessing bs->file Eric Blake
2023-10-27 15:53 ` [PATCH 22/24] vhdx: " Kevin Wolf
2023-10-30 21:26 ` Eric Blake
2023-10-27 15:53 ` [PATCH 23/24] block: Take graph lock for most of .bdrv_open Kevin Wolf
2023-10-30 21:34 ` Eric Blake
2023-11-03 10:05 ` Kevin Wolf
2023-10-27 15:53 ` [PATCH 24/24] block: Protect bs->file with graph_lock Kevin Wolf
2023-10-30 21:37 ` Eric Blake
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231027155333.420094-22-kwolf@redhat.com \
--to=kwolf@redhat.com \
--cc=eblake@redhat.com \
--cc=eesposit@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
--cc=vsementsov@yandex-team.ru \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).