Re: [Qemu-devel] [Fwd: Re: [PATCH 1/2] qemu-img convert: Rewrite copying logic]

[Peter Lieven]

Am 20.02.2015 um 14:27 schrieb Peter Lieven:
>
> On 2015-02-19 at 10:47, Kevin Wolf wrote:
>> Am 18.02.2015 um 18:18 hat Max Reitz geschrieben:
> [snip]
>
>>> Actually, now that I'm looking at is_allocated_sectors_min(), if the
>>> first sectors is not allocated, it will return false and thus both
>>> the current qemu-img convert and the new one after this patch won't
>>> write data. That's a bug, I think (because it is guaranteed by the
>>> man page).
>> Or the description in the man page is wrong.
>>
>> The intention with -S was that we avoid splitting up writes into too
>> many small chunks because that costs a lot of time. If you look at it
>> from that angle, it's doing exactly the right thing because skipping
>> zeros at the start doesn't increase the number of write requests.
> Feel free to fix it. But I remember someone recently asking about
> preallocation for qemu-img convert in the #qemu IRC channel, and "-S 0"
> was one of the valid answers.
>
> The nice thing about -S 0 is that it works with all image formats; I
> know that qcow2 is always our main concern (especially when it comes to
> the target of qemu-img convert), but I think using -S 0 for
> preallocation is justified.
>
> Considering that in this case the man page was lying (because the code
> did not allocate everything), I'd be fine with fixing up the man page
> and leaving the behavior as-is, though, too.

For the uncompressed case the code *did* allocate everyting.
has_zero_init was always set to 0 for -S 0 regardless what the backend
reported and in this case is_allocated_sectors_min is never called.

For the new version I would use the same behaviour altough you
might bdrv_write_zeroes to optimize the write, but in the -S 0 case
without BDRV_REQ_MAY_UNMAP.

Peter