diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 8b1e80bd0b..e02fae6a0c 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -1474,9 +1474,9 @@ static int coroutine_fn handle_dependencies(BlockDriverState *bs, return 0; } -static void coroutine_mixed_fn wait_for_dependencies(BlockDriverState *bs, - uint64_t guest_offset, - uint64_t bytes) +void coroutine_mixed_fn qcow2_wait_for_dependencies(BlockDriverState *bs, + uint64_t guest_offset, + uint64_t bytes) { BDRVQcow2State *s = bs->opaque; QCowL2Meta *m = NULL; @@ -2035,7 +2035,7 @@ int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset, * We don't need to allocate a QCowL2Meta for the discard operation because * s->lock is held for the duration of the whole operation. */ - wait_for_dependencies(bs, offset, bytes); + qcow2_wait_for_dependencies(bs, offset, bytes); /* Caller must pass aligned values, except at image end */ assert(QEMU_IS_ALIGNED(offset, s->cluster_size)); @@ -2204,7 +2204,7 @@ int coroutine_fn qcow2_subcluster_zeroize(BlockDriverState *bs, uint64_t offset, * We don't need to allocate a QCowL2Meta for the zeroize operation because * s->lock is held for the duration of the whole operation. */ - wait_for_dependencies(bs, offset, bytes); + qcow2_wait_for_dependencies(bs, offset, bytes); /* If we have to stay in sync with an external data file, zero out * s->data_file first. */ diff --git a/block/qcow2.c b/block/qcow2.c index 81fd299b4c..19271b10a4 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -4234,10 +4234,16 @@ qcow2_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int64_t bytes, } qemu_co_mutex_lock(&s->lock); - /* We can have new write after previous check */ offset -= head; bytes = s->subcluster_size; nr = s->subcluster_size; + /* + * Wait for in-flight allocating writes first: otherwise the type + * check below could pass on UNALLOCATED while a yet-to-link_l2 write + * completes during qcow2_subcluster_zeroize()'s own wait, letting the + * resumed MAY_UNMAP discard the just-written data. + */ + qcow2_wait_for_dependencies(bs, offset, bytes); ret = qcow2_get_host_offset(bs, offset, &nr, &off, &type); if (ret < 0 || (type != QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN && diff --git a/block/qcow2.h b/block/qcow2.h index 192a45d596..ce517040c4 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -966,6 +966,10 @@ int coroutine_fn GRAPH_RDLOCK qcow2_subcluster_zeroize(BlockDriverState *bs, uint64_t offset, uint64_t bytes, int flags); +void coroutine_mixed_fn +qcow2_wait_for_dependencies(BlockDriverState *bs, uint64_t guest_offset, + uint64_t bytes); + int GRAPH_RDLOCK qcow2_expand_zero_clusters(BlockDriverState *bs, BlockDriverAmendStatusCB *status_cb, diff --git a/tests/qemu-iotests/046 b/tests/qemu-iotests/046 index e03dd40147..0d84b5c1c7 100755 --- a/tests/qemu-iotests/046 +++ b/tests/qemu-iotests/046 @@ -226,6 +226,26 @@ aio_write -z 0x140000 0x10000 resume A aio_flush EOF + +# Start an allocating write to a previously unallocated cluster and, before +# its L2 update is linked, issue a concurrent sub-cluster zero write with +# MAY_UNMAP that targets a disjoint range within the same cluster. The zero +# write's head/tail are zero (cluster is unallocated), so qcow2_co_pwrite_zeroes +# would expand it to the full subcluster. Without waiting for dependencies +# before the zero write's "unallocated" type check, that check passes, +# qcow2_subcluster_zeroize then yields in wait_for_dependencies, the allocating +# write links its L2 entry, and the resumed zeroize unmaps the cluster - +# silently discarding the just-written data. Waiting first makes the zero write +# fall back to a bounce-buffered real write, which only touches its own +# subrange. +cat <