On 04/24/2017 06:06 PM, John Snow wrote: > > > On 04/11/2017 06:29 PM, Eric Blake wrote: >> We are gradually moving away from sector-based interfaces, towards >> byte-based. In the common case, allocation is unlikely to ever use >> values that are not naturally sector-aligned, but it is possible >> that byte-based values will let us be more precise about allocation >> at the end of an unaligned file that can do byte-based access. >> >> Changing the signature of the function to use int64_t *pnum ensures >> that the compiler enforces that all callers are updated. For now, >> the io.c layer still assert()s that all callers are sector-aligned, >> but that can be relaxed when a later patch implements byte-based >> block status. Therefore, for the most part this patch is just the >> addition of scaling at the callers followed by inverse scaling at >> bdrv_is_allocated(). But some code, particularly stream_run(), >> gets a lot simpler because it no longer has to mess with sectors. >>
>> +++ b/block/io.c
>> @@ -1930,52 +1930,46 @@ int coroutine_fn bdrv_is_allocated(BlockDriverState
>> *bs, int64_t offset,
>> /*
>> * Given an image chain: ... -> [BASE] -> [INTER1] -> [INTER2] -> [TOP]
>> *
>> - * Return true if the given sector is allocated in any image between
>> + * Return true if the given offset is allocated in any image between
>
> perhaps "range" instead of "offset"?
>
>> * BASE and TOP (inclusive). BASE can be NULL to check if the given
>> - * sector is allocated in any image of the chain. Return false otherwise,
>> + * offset is allocated in any image of the chain. Return false otherwise,
>> * or negative errno on failure.
Seems reasonable.
>> /*
>> - * [sector_num, nb_sectors] is unallocated on top but intermediate
>> - * might have
>> - *
>> - * [sector_num+x, nr_sectors] allocated.
>> + * [offset, bytes] is unallocated on top but intermediate
>> + * might have [offset+x, bytes-x] allocated.
>> */
>> - if (n > psectors_inter &&
>> + if (n > pnum_inter &&
>> (intermediate == top ||
>> - sector_num + psectors_inter < intermediate->total_sectors)) {
>
>
>
>> - 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.
>
> Reviewed-by: John Snow <js...@redhat.com>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
