On 09/27/2017 02:05 PM, John Snow wrote: > > > On 09/13/2017 12:03 PM, Eric Blake wrote: >> As long as we are querying the status for a chunk smaller than >> the known image size, we are guaranteed that a successful return >> will have set pnum to a non-zero size (pnum is zero only for >> queries beyond the end of the file). Use that to slightly >> simplify the calculation of the current chunk size being compared. >> Likewise, we don't have to shrink the amount of data operated on >> until we know we have to read the file, and therefore have to fit >> in the bounds of our buffer. Also, note that 'total_sectors_over' >> is equivalent to 'progress_base'. >> >> With these changes in place, sectors_to_process() is now dead code, >> and can be removed. >> >> Signed-off-by: Eric Blake <[email protected]> >>
>> @@ -1402,14 +1393,9 @@ static int img_compare(int argc, char **argv)
>> goto out;
>> }
>> allocated2 = status2 & BDRV_BLOCK_ALLOCATED;
>> - if (pnum1) {
>> - nb_sectors = MIN(nb_sectors,
>> - DIV_ROUND_UP(pnum1, BDRV_SECTOR_SIZE));
>> - }
>> - if (pnum2) {
>> - nb_sectors = MIN(nb_sectors,
>> - DIV_ROUND_UP(pnum2, BDRV_SECTOR_SIZE));
>> - }
>> +
>> + assert(pnum1 && pnum2);
>> + nb_sectors = DIV_ROUND_UP(MIN(pnum1, pnum2), BDRV_SECTOR_SIZE);
>
> In the apocalyptic future where non-sector sized returns are possible,
> does this math make sense?
>
> e.g. say the return is zeroes, but it's not aligned anymore, so we
> assume we have an extra half a sector's worth of zeroes here.
Not introduced in this patch, but a good question for 12/23. We want to
round up rather than down to ensure that we don't inf-loop on a partial
sector response; but at the same time, you're right that if we got a
report of a half-sector zero and we widen it, we can't guarantee that
the second half is zero.
On the bright side, this rounding goes away when later patches switch
img_compare to be byte-based, later in this series. But you're right
that it is probably smarter to have 12/23 assert that things are already
aligned (and thus we don't need to round in the first place).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
