[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [PATCH v2 03/15] block: Add flag to avoid

From: Eric Blake
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH v2 03/15] block: Add flag to avoid wasted work in bdrv_is_allocated()
Date: Wed, 5 Jul 2017 06:56:29 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0

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:

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:


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?

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

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]