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
