block: Mark bdrv_co_block_status() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of
bdrv_co_block_status() need to hold a reader lock for the graph.
For some places, we know that they will hold the lock, but we don't have
the GRAPH_RDLOCK annotations yet. In this case, add assume_graph_lock()
with a FIXME comment. These places will be removed once everything is
properly annotated.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230203152202.49054-5-kwolf@redhat.com>
Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
diff --git a/block/backup.c b/block/backup.c
index 824d39a..46fca24 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -270,7 +270,10 @@
return -ECANCELED;
}
+ /* rdlock protects the subsequent call to bdrv_is_allocated() */
+ bdrv_graph_co_rdlock();
ret = block_copy_reset_unallocated(s->bcs, offset, &count);
+ bdrv_graph_co_rdunlock();
if (ret < 0) {
return ret;
}
diff --git a/block/block-backend.c b/block/block-backend.c
index f25f3ba..f5d9e3e 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1431,6 +1431,7 @@
BlockDriverState **file)
{
IO_CODE();
+ GRAPH_RDLOCK_GUARD();
return bdrv_co_block_status_above(blk_bs(blk), base, offset, bytes, pnum,
map, file);
}
@@ -1441,6 +1442,7 @@
int64_t bytes, int64_t *pnum)
{
IO_CODE();
+ GRAPH_RDLOCK_GUARD();
return bdrv_co_is_allocated_above(blk_bs(blk), base, include_base, offset,
bytes, pnum);
}
diff --git a/block/block-copy.c b/block/block-copy.c
index 30a4da0..d299fac 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -581,9 +581,9 @@
return ret;
}
-static coroutine_fn int block_copy_block_status(BlockCopyState *s,
- int64_t offset,
- int64_t bytes, int64_t *pnum)
+static coroutine_fn GRAPH_RDLOCK
+int block_copy_block_status(BlockCopyState *s, int64_t offset, int64_t bytes,
+ int64_t *pnum)
{
int64_t num;
BlockDriverState *base;
@@ -618,9 +618,9 @@
* Check if the cluster starting at offset is allocated or not.
* return via pnum the number of contiguous clusters sharing this allocation.
*/
-static int coroutine_fn block_copy_is_cluster_allocated(BlockCopyState *s,
- int64_t offset,
- int64_t *pnum)
+static int coroutine_fn GRAPH_RDLOCK
+block_copy_is_cluster_allocated(BlockCopyState *s, int64_t offset,
+ int64_t *pnum)
{
BlockDriverState *bs = s->source->bs;
int64_t count, total_count = 0;
@@ -630,6 +630,7 @@
assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
while (true) {
+ /* protected in backup_run() */
ret = bdrv_co_is_allocated(bs, offset, bytes, &count);
if (ret < 0) {
return ret;
@@ -704,7 +705,7 @@
* Returns 1 if dirty clusters found and successfully copied, 0 if no dirty
* clusters found and -errno on failure.
*/
-static int coroutine_fn
+static int coroutine_fn GRAPH_RDLOCK
block_copy_dirty_clusters(BlockCopyCallState *call_state)
{
BlockCopyState *s = call_state->s;
@@ -827,7 +828,8 @@
* it means that some I/O operation failed in context of _this_ block_copy call,
* not some parallel operation.
*/
-static int coroutine_fn block_copy_common(BlockCopyCallState *call_state)
+static int coroutine_fn GRAPH_RDLOCK
+block_copy_common(BlockCopyCallState *call_state)
{
int ret;
BlockCopyState *s = call_state->s;
@@ -892,6 +894,7 @@
static void coroutine_fn block_copy_async_co_entry(void *opaque)
{
+ GRAPH_RDLOCK_GUARD();
block_copy_common(opaque);
}
diff --git a/block/coroutines.h b/block/coroutines.h
index 2a1e0b3..dd9f3d4 100644
--- a/block/coroutines.h
+++ b/block/coroutines.h
@@ -43,7 +43,7 @@
int coroutine_fn GRAPH_RDLOCK
bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp);
-int coroutine_fn
+int coroutine_fn GRAPH_RDLOCK
bdrv_co_common_block_status_above(BlockDriverState *bs,
BlockDriverState *base,
bool include_base,
diff --git a/block/io.c b/block/io.c
index 35025fc..a0f8efc 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2224,11 +2224,10 @@
* BDRV_BLOCK_OFFSET_VALID bit is set, 'map' and 'file' (if non-NULL) are
* set to the host mapping and BDS corresponding to the guest offset.
*/
-static int coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
- bool want_zero,
- int64_t offset, int64_t bytes,
- int64_t *pnum, int64_t *map,
- BlockDriverState **file)
+static int coroutine_fn GRAPH_RDLOCK
+bdrv_co_block_status(BlockDriverState *bs, bool want_zero,
+ int64_t offset, int64_t bytes,
+ int64_t *pnum, int64_t *map, BlockDriverState **file)
{
int64_t total_size;
int64_t n; /* bytes */
@@ -2240,6 +2239,7 @@
bool has_filtered_child;
assert(pnum);
+ assert_bdrv_graph_readable();
*pnum = 0;
total_size = bdrv_getlength(bs);
if (total_size < 0) {
@@ -2470,6 +2470,7 @@
IO_CODE();
assert(!include_base || base); /* Can't include NULL base */
+ assert_bdrv_graph_readable();
if (!depth) {
depth = &dummy;
@@ -2595,6 +2596,8 @@
int64_t pnum = bytes;
IO_CODE();
+ assume_graph_lock(); /* FIXME */
+
if (!bytes) {
return 1;
}
diff --git a/block/mirror.c b/block/mirror.c
index fbbb4f6..94c523a 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -558,9 +558,11 @@
MirrorMethod mirror_method = MIRROR_METHOD_COPY;
assert(!(offset % s->granularity));
- ret = bdrv_block_status_above(source, NULL, offset,
- nb_chunks * s->granularity,
- &io_bytes, NULL, NULL);
+ WITH_GRAPH_RDLOCK_GUARD() {
+ ret = bdrv_block_status_above(source, NULL, offset,
+ nb_chunks * s->granularity,
+ &io_bytes, NULL, NULL);
+ }
if (ret < 0) {
io_bytes = MIN(nb_chunks * s->granularity, max_io_bytes);
} else if (ret & BDRV_BLOCK_DATA) {
@@ -863,8 +865,10 @@
return 0;
}
- ret = bdrv_is_allocated_above(bs, s->base_overlay, true, offset, bytes,
- &count);
+ WITH_GRAPH_RDLOCK_GUARD() {
+ ret = bdrv_is_allocated_above(bs, s->base_overlay, true, offset,
+ bytes, &count);
+ }
if (ret < 0) {
return ret;
}
diff --git a/block/qcow.c b/block/qcow.c
index 5eb1ab5..2d19a78 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -524,19 +524,16 @@
return 1;
}
-static int coroutine_fn qcow_co_block_status(BlockDriverState *bs,
- bool want_zero,
- int64_t offset, int64_t bytes,
- int64_t *pnum, int64_t *map,
- BlockDriverState **file)
+static int coroutine_fn GRAPH_RDLOCK
+qcow_co_block_status(BlockDriverState *bs, bool want_zero,
+ int64_t offset, int64_t bytes, int64_t *pnum,
+ int64_t *map, BlockDriverState **file)
{
BDRVQcowState *s = bs->opaque;
int index_in_cluster, ret;
int64_t n;
uint64_t cluster_offset;
- assume_graph_lock(); /* FIXME */
-
qemu_co_mutex_lock(&s->lock);
ret = get_cluster_offset(bs, offset, 0, 0, 0, 0, &cluster_offset);
qemu_co_mutex_unlock(&s->lock);
diff --git a/block/quorum.c b/block/quorum.c
index d1dcf2e..f3fe067 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -1217,11 +1217,10 @@
* return BDRV_BLOCK_ZERO if *all* children agree that a certain
* region contains zeroes, and BDRV_BLOCK_DATA otherwise.
*/
-static int coroutine_fn quorum_co_block_status(BlockDriverState *bs,
- bool want_zero,
- int64_t offset, int64_t count,
- int64_t *pnum, int64_t *map,
- BlockDriverState **file)
+static int coroutine_fn GRAPH_RDLOCK
+quorum_co_block_status(BlockDriverState *bs, bool want_zero,
+ int64_t offset, int64_t count,
+ int64_t *pnum, int64_t *map, BlockDriverState **file)
{
BDRVQuorumState *s = bs->opaque;
int i, ret;
diff --git a/block/stream.c b/block/stream.c
index 8744ad1..22368ce 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -161,21 +161,25 @@
copy = false;
- ret = bdrv_is_allocated(unfiltered_bs, offset, STREAM_CHUNK, &n);
- if (ret == 1) {
- /* Allocated in the top, no need to copy. */
- } else if (ret >= 0) {
- /* Copy if allocated in the intermediate images. Limit to the
- * known-unallocated area [offset, offset+n*BDRV_SECTOR_SIZE). */
- ret = bdrv_is_allocated_above(bdrv_cow_bs(unfiltered_bs),
- s->base_overlay, true,
- offset, n, &n);
- /* Finish early if end of backing file has been reached */
- if (ret == 0 && n == 0) {
- n = len - offset;
- }
+ WITH_GRAPH_RDLOCK_GUARD() {
+ ret = bdrv_is_allocated(unfiltered_bs, offset, STREAM_CHUNK, &n);
+ if (ret == 1) {
+ /* Allocated in the top, no need to copy. */
+ } else if (ret >= 0) {
+ /*
+ * Copy if allocated in the intermediate images. Limit to the
+ * known-unallocated area [offset, offset+n*BDRV_SECTOR_SIZE).
+ */
+ ret = bdrv_is_allocated_above(bdrv_cow_bs(unfiltered_bs),
+ s->base_overlay, true,
+ offset, n, &n);
+ /* Finish early if end of backing file has been reached */
+ if (ret == 0 && n == 0) {
+ n = len - offset;
+ }
- copy = (ret > 0);
+ copy = (ret > 0);
+ }
}
trace_stream_one_iteration(s, offset, n, ret);
if (copy) {
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index d0f8386..0700953 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -36,9 +36,9 @@
void block_copy_state_free(BlockCopyState *s);
void block_copy_reset(BlockCopyState *s, int64_t offset, int64_t bytes);
-int64_t coroutine_fn block_copy_reset_unallocated(BlockCopyState *s,
- int64_t offset,
- int64_t *count);
+
+int64_t coroutine_fn GRAPH_RDLOCK
+block_copy_reset_unallocated(BlockCopyState *s, int64_t offset, int64_t *count);
int coroutine_fn block_copy(BlockCopyState *s, int64_t offset, int64_t bytes,
bool ignore_ratelimit, uint64_t timeout_ns,
diff --git a/include/block/block-io.h b/include/block/block-io.h
index 9a09ec2..6303501 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -109,24 +109,24 @@
int64_t bytes, int64_t *pnum, int64_t *map,
BlockDriverState **file);
-int coroutine_fn bdrv_co_block_status_above(BlockDriverState *bs,
- BlockDriverState *base,
- int64_t offset, int64_t bytes,
- int64_t *pnum, int64_t *map,
- BlockDriverState **file);
+int coroutine_fn GRAPH_RDLOCK
+bdrv_co_block_status_above(BlockDriverState *bs, BlockDriverState *base,
+ int64_t offset, int64_t bytes, int64_t *pnum,
+ int64_t *map, BlockDriverState **file);
int bdrv_block_status_above(BlockDriverState *bs, BlockDriverState *base,
int64_t offset, int64_t bytes, int64_t *pnum,
int64_t *map, BlockDriverState **file);
-int coroutine_fn bdrv_co_is_allocated(BlockDriverState *bs, int64_t offset,
- int64_t bytes, int64_t *pnum);
+int coroutine_fn GRAPH_RDLOCK
+bdrv_co_is_allocated(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ int64_t *pnum);
int bdrv_is_allocated(BlockDriverState *bs, int64_t offset, int64_t bytes,
int64_t *pnum);
-int coroutine_fn bdrv_co_is_allocated_above(BlockDriverState *top,
- BlockDriverState *base,
- bool include_base, int64_t offset,
- int64_t bytes, int64_t *pnum);
+int coroutine_fn GRAPH_RDLOCK
+bdrv_co_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
+ bool include_base, int64_t offset, int64_t bytes,
+ int64_t *pnum);
int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
bool include_base, int64_t offset, int64_t bytes,
int64_t *pnum);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index eaf62be..5f0104a 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -606,7 +606,8 @@
* *pnum value for the block-status cache on protocol nodes, prior
* to clamping *pnum for return to its caller.
*/
- int coroutine_fn (*bdrv_co_block_status)(BlockDriverState *bs,
+ int coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_block_status)(
+ BlockDriverState *bs,
bool want_zero, int64_t offset, int64_t bytes, int64_t *pnum,
int64_t *map, BlockDriverState **file);
diff --git a/qemu-img.c b/qemu-img.c
index 7c05931..cd0178b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1991,7 +1991,9 @@
qemu_co_mutex_unlock(&s->lock);
break;
}
- n = convert_iteration_sectors(s, s->sector_num);
+ WITH_GRAPH_RDLOCK_GUARD() {
+ n = convert_iteration_sectors(s, s->sector_num);
+ }
if (n < 0) {
qemu_co_mutex_unlock(&s->lock);
s->ret = n;
diff --git a/tests/unit/test-block-iothread.c b/tests/unit/test-block-iothread.c
index 6dfac64..3a5e1eb 100644
--- a/tests/unit/test-block-iothread.c
+++ b/tests/unit/test-block-iothread.c
@@ -312,7 +312,8 @@
g_assert_cmpint(ret, ==, -EINVAL);
}
-static void test_sync_op_block_status(BdrvChild *c)
+/* Disable TSA to make bdrv_test.bdrv_co_block_status writable */
+static void TSA_NO_TSA test_sync_op_block_status(BdrvChild *c)
{
int ret;
int64_t n;
