Re: [PATCH 2/2] qemu-img: align next status sector on destination alignment.

[Eric Blake]

On 11/12/20 6:40 AM, Peter Lieven wrote:

>>          /*
>> -         * Avoid that s->sector_next_status becomes unaligned to the source
>> -         * request alignment and/or cluster size to avoid unnecessary read
>> -         * cycles.
>> +         * Avoid that s->sector_next_status becomes unaligned to the
>> +         * source/destination request alignment and/or cluster size to avoid
>> +         * unnecessary read/write cycles.
>>           */
>> -        tail = (sector_num - src_cur_offset + n) % 
>> s->src_alignment[src_cur];
>> +        alignment = MAX(s->src_alignment[src_cur], s->alignment);
>> +        assert(is_power_of_2(alignment));
>> +
>> +        tail = (sector_num - src_cur_offset + n) % alignment;
>>          if (n > tail) {
>>              n -= tail;
>>          }
> 
> 
> I was also considering including the s->alignment when adding this chance. 
> However, you need the least common multiple of both alignments, not the 
> maximum, otherwise
> 
> you might get misaligned to either source or destination.
> 
> 
> Why exactly do you need the power of two requirement?

The power of two requirement ensures that you h ave the least common
multiple of both alignments ;)

However, there ARE devices in practice that have advertised a
non-power-of-2 discard granularity, such as 15MiB (see commits 430b26a8,
63188c24).  Which means you probably don't want to assert power-of-2,
and in turn need to worry about least common multiple.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org