Re: [Qemu-devel] [PATCH 1.8 5/6] qemu-img: add option to align writes to cluster_sectors during convert

[Paolo Bonzini]

Il 25/11/2013 17:11, Peter Lieven ha scritto:
> On 25.11.2013 16:11, Paolo Bonzini wrote:
>> Il 25/11/2013 14:57, Peter Lieven ha scritto:
>>> Signed-off-by: Peter Lieven <address@hidden>
>> Ok, given this patch I think the cluster_size is the right one to use
>> here---and also the way you used the optimal unmap granularity makes
>> sense; you could also use MAX(optimal unmap granularity, optimal
>> transfer length granularity).
>>
>> However, there is no need to write one cluster at a time.  What matters,
>> I think, is to align the *end* of the transfer, so that the next
>> transfer can start aligned.
>>
>>> +            if (align && cluster_sectors > 0) {
>>> +                int64_t next_aligned_sector = (sector_num +
>>> cluster_sectors);
>> So this should be "+ n", not "+ cluster_sectors".
>>
>> Perhaps it could be conditional on "n > cluster_sectors" (small requests
>> happen when you have sparse region, and breaking them doesn't help).
> 
> would you also agree to n >= cluster_sectors. In my case
> and if especially if n is bound by iobuf_size the case n > cluster_sectors
> will be hard to meet.

Of course.  In fact > alone is wrong ("n > cluster_sectors || n ==
iobuf_size" could be right, but perhaps it's a useless complication).

Paolo