Re: [Qemu-devel] [PATCH v5 04/23] qcow2: Switch is_zero_sectors() to byte-based

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1660 bytes --]

On 10/10/2017 09:15 AM, Kevin Wolf wrote:
> Am 04.10.2017 um 04:00 hat Eric Blake geschrieben:
>> We are gradually converting to byte-based interfaces, as they are
>> easier to reason about than sector-based.  Convert another internal
>> function (no semantic change), and rename it to is_zero() in the
>> process.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> Reviewed-by: Fam Zheng <famz@redhat.com>
>> Reviewed-by: John Snow <jsnow@redhat.com>
>>

>> +static bool is_zero(BlockDriverState *bs, int64_t offset, int64_t bytes)
>>  {
>>      int nr;
>>      int64_t res;
>> +    int64_t start;
>>
>> -    if (start + count > bs->total_sectors) {
>> -        count = bs->total_sectors - start;
>> +    /* Widen to sector boundaries, then clamp to image length, before
>> +     * checking status of underlying sectors */
>> +    start = QEMU_ALIGN_DOWN(offset, BDRV_SECTOR_SIZE);
>> +    bytes = QEMU_ALIGN_UP(offset + bytes, BDRV_SECTOR_SIZE) - start;
> 
> Why do we still widen to sector boundaries after this series is fully
> applied? Isn't the whole point that we don't have to do this any more?

The widening is necessary as long as...

>> +    res = bdrv_get_block_status_above(bs, NULL, start >> BDRV_SECTOR_BITS,
>> +                                      bytes >> BDRV_SECTOR_BITS, &nr, NULL);

we still shift. But you are right; patch 12/23 should be able to drop
the rounding.  I'll double check that it still works, but assuming that
it does, v6 will include that cleanup.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]