When creating/resizing an image with/to a non-cluster-aligned length, we currently implicitly COW the cluster tail after the guest disk end. That is unnecessary, but more importantly, when creating a qcow2 image for a pre-existing external raw data file, we should not write to it unless data preallocation was requested.
So far, writing to the data file past the guest disk end during qcow2 image creation may just be a peculiarity. But the next commit in this series will make qcow2 use the BDRV_O_NO_DATA_WRITE flag to suppress taking the WRITE flag on the data file during image creation, to allow users to create a qcow2 image with an existing raw image as its data file that is currently in use by the VM (i.e. the WRITE flag is unshared). To make this work, we really must not write to the data file at all during creation, unless data preallocation (falloc/full) has been requested. (The comment added in this commit therefore primarily reasons with the use of that flag, and how we must not break it. Admittedly, that only makes sense after the next patch in this series.) Note that this means that creating/resizing images with/to a guest disk size that is not aligned to clusters will create a partial data cluster at the image end when preallocating (except for falloc/full preallocation without a data file). I think that is fine, similarly to how creating a non-preallocated image will generally leave a partial cluster at the end, too (the L1 table). It does mean iotest 125 needs to be adjusted, though. Signed-off-by: Hanna Czenczek <[email protected]> --- block/qcow2.c | 18 ++++++++++++++++++ tests/qemu-iotests/125 | 17 +++++++++++------ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index 69d621e9bf..b601bd540d 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -3368,7 +3368,25 @@ preallocate_co(BlockDriverState *bs, uint64_t offset, uint64_t new_length, } for (m = meta; m != NULL; m = m->next) { + int64_t disk_end_ofs; + m->prealloc = true; + + /* + * Do not COW beyond the supposed image end: There is no point, and + * it could break BDRV_O_NO_DATA_WRITE from qcow2_co_create(): + * qcow2_alloc_host_offset() does not COW anything for the range we + * pass, but everything outside. If growing to a non-cluster + * aligned size, it will thus request COW beyond the image end, + * breaking the BDRV_O_NO_DATA_WRITE promise. + * (As long as the @offset passed to this function was aligned to + * full clusters, that is the only possible instance of COW. With + * qcow2_co_create(), it's always 0, so always aligned.) + */ + disk_end_ofs = new_length - (int64_t)m->offset; + if (m->cow_end.offset + m->cow_end.nb_bytes > disk_end_ofs) { + m->cow_end.nb_bytes = MAX(disk_end_ofs - m->cow_end.offset, 0); + } } ret = qcow2_handle_l2meta(bs, &meta, true); diff --git a/tests/qemu-iotests/125 b/tests/qemu-iotests/125 index 708e7c5ba2..8213de012e 100755 --- a/tests/qemu-iotests/125 +++ b/tests/qemu-iotests/125 @@ -172,10 +172,10 @@ done # Test image resizing using preallocation and unaligned offsets $QEMU_IMG create -f raw "$TEST_IMG.base" 128k | _filter_img_create $QEMU_IO -c 'write -q -P 1 0 128k' -f raw "$TEST_IMG.base" -for orig_size in 31k 33k; do - for dst_size in 96k 128k; do +for orig_size in $((31 * 1024)) $((33 * 1024)); do + for dst_size in $((96 * 1024)) $((128 * 1024)); do for prealloc in metadata full; do - echo "--- Resizing image from $orig_size to $dst_size (preallocation=$prealloc) ---" + echo "--- Resizing image from $((orig_size / 1024))k to $((dst_size / 1024))k (preallocation=$prealloc) ---" _make_test_img -F raw -b "$TEST_IMG.base" -o cluster_size=64k "$orig_size" $QEMU_IMG resize -f "$IMGFMT" --preallocation="$prealloc" "$TEST_IMG" "$dst_size" # The first part of the image should contain data from the backing file @@ -183,10 +183,15 @@ for orig_size in 31k 33k; do # The resized part of the image should contain zeroes $QEMU_IO -c "read -q -P 0 ${orig_size} 63k" "$TEST_IMG" # If the image does not have an external data file we can also verify its - # actual size. The resized image should have 7 clusters: - # header, L1 table, L2 table, refcount table, refcount block, 2 data clusters + # actual size. The resized image should have 5 metadata clusters + # (header, L1 table, L2 table, refcount table, refcount block) + # plus the data. We round up that data to full clusters for full + # preallocation, but not for metadata preallocation. if ! _get_data_file "$TEST_IMG" > /dev/null; then - expected_file_length=$((65536 * 7)) + expected_file_length=$((65536 * 5 + dst_size)) + if [ "$prealloc" = full ]; then + expected_file_length=$(((expected_file_length + 65535) / 65536 * 65536)) + fi file_length=$(stat -c '%s' "$TEST_IMG_FILE") if [ "$file_length" != "$expected_file_length" ]; then echo "ERROR: file length $file_length (expected $expected_file_length)" -- 2.52.0
