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 <ebl...@redhat.com> >>
>> @@ -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