Re: [Qemu-devel] [PATCH 3/8] block/io: handle alignment and max_transfer for copy_range

[Vladimir Sementsov-Ogievskiy]

07.08.2019 20:28, Max Reitz wrote:
> On 07.08.19 10:07, Vladimir Sementsov-Ogievskiy wrote:
>> copy_range ignores these limitations, let's improve it. block/backup
>> code handles max_transfer for copy_range by itself, now it's not needed
>> more, drop it.
> 
> Shouldn’t this be two separate patches?

Not a problem, will do.

> 
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>> ---
>>   block/backup.c | 11 ++---------
>>   block/io.c     | 41 +++++++++++++++++++++++++++++++++--------
>>   2 files changed, 35 insertions(+), 17 deletions(-)
> 
> [...]
> 
>> diff --git a/block/io.c b/block/io.c
>> index 06305c6ea6..5abbd0fff2 100644
>> --- a/block/io.c
>> +++ b/block/io.c
>> @@ -3005,6 +3005,12 @@ static int coroutine_fn bdrv_co_copy_range_internal(
>>   {
>>       BdrvTrackedRequest req;
>>       int ret;
>> +    uint32_t align = MAX(src->bs->bl.request_alignment,
>> +                         dst->bs->bl.request_alignment);
>> +    uint32_t max_transfer =
>> +            
>> QEMU_ALIGN_DOWN(MIN_NON_ZERO(MIN_NON_ZERO(src->bs->bl.max_transfer,
>> +                                                      
>> dst->bs->bl.max_transfer),
>> +                                         INT_MAX), align);
> 
> I suppose the outer QEMU_ALIGN_DOWN() may result in @max_transfer of 0
> (in theory, if one’s max_transfer is smaller than the other’s alignment).
> 
> Not likely, but should still be handled.
> 
> The rest looks good to me.
> 
> Max
> 
>>       /* TODO We can support BDRV_REQ_NO_FALLBACK here */
>>       assert(!(read_flags & BDRV_REQ_NO_FALLBACK));
> 


-- 
Best regards,
Vladimir