On Wed, 07/05 06:56, Eric Blake wrote: > On 07/04/2017 02:06 AM, Fam Zheng wrote: > > On Mon, 07/03 17:14, Eric Blake wrote: > >> @@ -1717,6 +1718,10 @@ int64_t coroutine_fn > >> bdrv_co_get_block_status_from_backing(BlockDriverState *bs, > >> * Drivers not implementing the functionality are assumed to not support > >> * backing files, hence all their sectors are reported as allocated. > >> * > >> + * If 'allocation' is true, the caller only cares about allocation > >> + * status; this is a hint that a larger 'pnum' result is more > >> + * important than including BDRV_BLOCK_OFFSET_VALID in the return. > >> + * > > > > This is slightly unintuitive. I would guess "allocation == false" means "I > > don't > > care about the allocation status" but it actually is "I don't care about the > > consecutiveness". The "only" semantics is missing in the parameter name. > > > > Maybe rename it to "consecutive" and invert the logic? I.e. "consecutive == > > true" means BDRV_BLOCK_OFFSET_VALID is wanted. > > Reasonable idea; other [shorter] names I've been toying with: > strict > mapping > precise > > any of which, if true (set true by bdrv_get_block_status), means that I > care more about BDRV_BLOCK_OFFSET_VALID and validity for learning host > offsets, if false it means I'm okay getting a larger *pnum even if it > extends over disjoint host offsets; or: > > fast > > which if true (set true by bdrv_is_allocated) means I want a larger > *pnum even if I have to sacrifice accuracy by omitting > BDRV_BLOCK_OFFSET_VALID, because I'm trying to reduce effort spent. > > > > > Sorry for bikeshedding. > > Not a problem, I also had some double-takes in writing my own code > trying to remember which way I wanted the 'allocation' boolean to be > set, so coming up with a more intuitive name/default state in order to > help legibility is worth it. Do any of my above suggestions sound better? >
I'd vote for "mapping" because it has a close connection with offset (as in BDRV_BLOCK_OFFSET_VALID). Or simply call it "offset" and if false, never return BDRV_BLOCK_OFFSET_VALID. Fam
