[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v4 13/23] qemu-img: Simplify logic
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v4 13/23] qemu-img: Simplify logic in img_compare() |
Date: |
Wed, 27 Sep 2017 14:15:50 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
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 <address@hidden>
>>
>> @@ -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
- [Qemu-block] [PATCH v4 10/23] block: Switch bdrv_common_block_status_above() to byte-based, (continued)
- [Qemu-block] [PATCH v4 10/23] block: Switch bdrv_common_block_status_above() to byte-based, Eric Blake, 2017/09/13
- [Qemu-block] [PATCH v4 09/23] block: Switch BdrvCoGetBlockStatusData to byte-based, Eric Blake, 2017/09/13
- [Qemu-block] [PATCH v4 08/23] block: Switch bdrv_co_get_block_status() to byte-based, Eric Blake, 2017/09/13
- [Qemu-block] [PATCH v4 11/23] block: Switch bdrv_co_get_block_status_above() to byte-based, Eric Blake, 2017/09/13
- [Qemu-block] [PATCH v4 13/23] qemu-img: Simplify logic in img_compare(), Eric Blake, 2017/09/13
- [Qemu-block] [PATCH v4 14/23] qemu-img: Speed up compare on pre-allocated larger file, Eric Blake, 2017/09/13
- [Qemu-block] [PATCH v4 12/23] block: Convert bdrv_get_block_status_above() to bytes, Eric Blake, 2017/09/13
- [Qemu-block] [PATCH v4 15/23] qemu-img: Add find_nonzero(), Eric Blake, 2017/09/13
- [Qemu-block] [PATCH v4 16/23] qemu-img: Drop redundant error message in compare, Eric Blake, 2017/09/13