Re: [Qemu-block] [Qemu-devel] [PATCH 17/17] block: Make bdrv_is_allocated_above() byte-based

[Eric Blake]

On 04/24/2017 08:48 PM, Eric Blake wrote:

>>
>>
>>> -            n = psectors_inter;
>>> +             offset + pnum_inter < (intermediate->total_sectors *
>>> +                                    BDRV_SECTOR_SIZE))) {
>>
>> Naive question: not worth using either bdrv_getlength for bytes, or the
>> bdrv_nb_sectors helpers?
> 
> bdrv_getlength(intermediate) should indeed be the same as
> intermediate->total_sectors * BDRV_SECTOR_SIZE (for now - ultimately it
> would be nice to track a byte length rather than a sector length for
> images). I can make that cleanup for v2.

This one's tricky.  Calling bdrv_nb_sectors()/bdrv_getlength() (the two
are identical other than scale) guarantees that you have a sane answer
for a variably-sized BDS, but can fail with -ENOMEDIUM.  Which, given
the position in the 'if' clause, makes it a difficult rewrite to
properly catch.  On the other hand, since we just barely called
bdrv_is_allocated(intermediate), which in turn called
bdrv_co_get_block_status(), and that calls bdrv_nb_sectors(), we are
assured that intermediate->total_sectors has not changed in the meantime.

So my options are to either add a big comment stating why we are safe,
or to use bdrv_getlength() anyways but with proper error checking.  Best
done as a separate patch from the conversion in scale; so I'll do that
for v2.

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

signature.asc
Description: OpenPGP digital signature