Re: [Qemu-devel] [PATCH v1 03/13] qcow2: do not COW the empty areas

[Denis V. Lunev]

On 05/22/2017 10:24 PM, Eric Blake wrote:
> On 05/19/2017 04:34 AM, Anton Nefedov wrote:
>> If COW area of the newly allocated cluster is zeroes, there is no reason
>> to write zero sectors in perform_cow() again now as whole clusters are
>> zeroed out in single chunks by handle_alloc_space().
> But that's only true if you can guarantee that handle_alloc_space()
> succeeded at ensuring the cluster reads as zeroes.  If you silently
> ignore errors (which is what patch 1/13 does), you risk assuming that
> the cluster reads as zeroes when in reality it does not, and then you
> have corrupted data.
>
> The idea of avoiding a COW of areas that read as zero at the source when
> the destination also already reads as zeroes makes sense, but I'm not
> convinced that this patch is safe as written.
we will recheck error path. OK.

>> Introduce QCowL2Meta field "reduced", since the existing fields
>> (offset and nb_bytes) still has to keep other write requests from
>> simultaneous writing in the area
>>
>> iotest 060:
>> write to the discarded cluster does not trigger COW anymore.
>> so, break on write_aio event instead, will work for the test
>> (but write won't fail anymore, so update reference output)
>>
>> iotest 066:
>> cluster-alignment areas that were not really COWed are now detected
>> as zeroes, hence the initial write has to be exactly the same size for
>> the maps to match
>>
>> performance tests: ===
>>
>> qemu-io,
>>   results in seconds to complete (less is better)
>>   random write 4k to empty image, no backing
>>     HDD
>>       64k cluster
>>         128M over 128M image:   160 -> 160 ( x1  )
>>         128M over   2G image:    86 ->  84 ( x1  )
>>         128M over   8G image:    40 ->  29 ( x1.4 )
>>       1M cluster
>>          32M over   8G image:    58 ->  23 ( x2.5 )
>>
>>     SSD
>>       64k cluster
>>           2G over   2G image:    71 ->  38 (  x1.9 )
>>         512M over   8G image:    85 ->   8 ( x10.6 )
>>       1M cluster
>>         128M over  32G image:   314 ->   2 ( x157  )
> At any rate, the benchmark numbers show that there is merit to pursuing
> the idea of reducing I/O when partial cluster writes can avoid writing
> COW'd zeroes on either side of the data.
>
yes! This is exactly the point and also with this approach we would
allow sequential non-aligned to cluster writes, which is also very good.

Den