When opening an image with discard=off, we punch hole in the image when writing zeroes, making the image sparse. This breaks users that want to ensure that writes cannot fail with ENOSPACE by using fully allocated images.
bdrv_co_pwrite_zeroes() correctly disable BDRV_REQ_MAY_UNMAP if we opened the child without discard=unmap or discard=on. But we don't go through this function when accessing the top node. Move the check down to bdrv_co_do_pwrite_zeroes() which seems to be used in all code paths. Issues: - We don't punch hole by default, so images are kept allocated. Before this change we punched holes by default. I'm not sure this is a good change in behavior. - Need to run all block tests - Not sure that we have tests covering unmapping, we may need new tests - We may need new tests to cover this change Signed-off-by: Nir Soffer <nsof...@redhat.com> --- Changes since v1: - Replace the incorrect has_discard change with the right fix v1 was here: https://lists.nongnu.org/archive/html/qemu-block/2024-06/msg00198.html block/io.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/block/io.c b/block/io.c index 7217cf811b..301514c880 100644 --- a/block/io.c +++ b/block/io.c @@ -1860,10 +1860,15 @@ bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int64_t bytes, /* By definition there is no user buffer so this flag doesn't make sense */ if (flags & BDRV_REQ_REGISTERED_BUF) { return -EINVAL; } + /* If opened with discard=off we should never unmap. */ + if (!(bs->open_flags & BDRV_O_UNMAP)) { + flags &= ~BDRV_REQ_MAY_UNMAP; + } + /* Invalidate the cached block-status data range if this write overlaps */ bdrv_bsc_invalidate_range(bs, offset, bytes); assert(alignment % bs->bl.request_alignment == 0); head = offset % alignment; @@ -2313,14 +2318,10 @@ int coroutine_fn bdrv_co_pwrite_zeroes(BdrvChild *child, int64_t offset, { IO_CODE(); trace_bdrv_co_pwrite_zeroes(child->bs, offset, bytes, flags); assert_bdrv_graph_readable(); - if (!(child->bs->open_flags & BDRV_O_UNMAP)) { - flags &= ~BDRV_REQ_MAY_UNMAP; - } - return bdrv_co_pwritev(child, offset, bytes, NULL, BDRV_REQ_ZERO_WRITE | flags); } /* -- 2.45.1