Re: [Qemu-devel] [PATCH v10 8/9] qcow2: skip writing zero buffers to empty COW areas

[Anton Nefedov]

On 14/12/2018 7:20 PM, Vladimir Sementsov-Ogievskiy wrote:
> 03.12.2018 13:14, Anton Nefedov wrote:
>> If COW areas of the newly allocated clusters are zeroes on the backing image,
>> efficient bdrv_write_zeroes(flags=BDRV_REQ_ALLOCATE) can be used on the whole
>> cluster instead of writing explicit zero buffers later in perform_cow().
>>
>> iotest 060:
>> write to the discarded cluster does not trigger COW anymore.
>> Use a backing image instead.
>>
> 
> [..]
> 
>> --- a/tests/qemu-iotests/060
>> +++ b/tests/qemu-iotests/060
>> @@ -150,27 +150,33 @@ $QEMU_IO -c "$OPEN_RO" -c "read -P 1 0 512" | 
>> _filter_qemu_io
>>    echo
>>    echo "=== Testing overlap while COW is in flight ==="
>>    echo
>> +BACKING_IMG=$TEST_IMG.base
>> +TEST_IMG=$BACKING_IMG _make_test_img 1G
>> +
>> +$QEMU_IO -c 'write 64k 64k' "$BACKING_IMG" | _filter_qemu_io
>> +
>>    # compat=0.10 is required in order to make the following discard actually
>> -# unallocate the sector rather than make it a zero sector - we want COW, 
>> after
>> -# all.
>> -IMGOPTS='compat=0.10' _make_test_img 1G
>> +# unallocate the sector rather than make it a zero sector as we would like
>> +# to reuse it for another guest offset
>> +IMGOPTS='compat=0.10' _make_test_img -b "$BACKING_IMG" 1G
>>    # Write two clusters, the second one enforces creation of an L2 table 
>> after
>>    # the first data cluster.
>>    $QEMU_IO -c 'write 0k 64k' -c 'write 512M 64k' "$TEST_IMG" | 
>> _filter_qemu_io
>> -# Discard the first cluster. This cluster will soon enough be reallocated 
>> and
>> -# used for COW.
>> +# Discard the first cluster. This cluster will soon enough be reallocated
>>    $QEMU_IO -c 'discard 0k 64k' "$TEST_IMG" | _filter_qemu_io
>>    # Now, corrupt the image by marking the second L2 table cluster as free.
>>    poke_file "$TEST_IMG" '131084' "\x00\x00" # 0x2000c
>> -# Start a write operation requiring COW on the image stopping it right 
>> before
>> -# doing the read; then, trigger the corruption prevention by writing 
>> anything to
>> -# any unallocated cluster, leading to an attempt to overwrite the second L2
>> +# Start a write operation requiring COW on the image;
>> +# this write will reuse the host offset released by a previous discard.
>> +# Stop it right before doing the read.
>> +# Then, trigger the corruption prevention by writing anything to
>> +# another unallocated cluster, leading to an attempt to overwrite the 
>> second L2
>>    # table. Finally, resume the COW write and see it fail (but not crash).
>>    echo "open -o file.driver=blkdebug $TEST_IMG
>>    break cow_read 0
>> -aio_write 0k 1k
>> +aio_write 64k 1k
>>    wait_break 0
>> -write 64k 64k
>> +write 128k 64k
> 
> don't understand why you need these changes.
> 
> works for me, without them, if write to backing at 0 offset, of course.
> 
> As I understand, discard create unallocated holes in top qcow2 for old qcow2 
> version.
> 

Ok, so COW happens regardless if this guest offset has been discarded
before. These offset changes are indeed not needed. Just the backing
file.