Re: [Qemu-block] [PATCH v12 02/10] block: Update comments on BDRV_BLOCK_* meanings

[Eric Blake]

On 05/05/2017 03:06 PM, Max Reitz wrote:
> On 04.05.2017 05:07, Eric Blake wrote:
>> We had some conflicting documentation: a nice 8-way table that
>> described all possible combinations of DATA, ZERO, and
>> OFFSET_VALID, contrasted with text that implied that OFFSET_VALID
>> always meant raw data could be read directly.  Furthermore, the
>> text refers a lot to bs->file, even though the interface was
>> updated back in 67a0fd2a to let the driver pass back which BDS (not
>> necessarily bs->file).
> 
> Not sure about my English skills here, but is this missing a verb? ("to
> pass back which BDS...")

maybe s/which/a specific/

> 
>>                         As the 8-way table is the intended
>> semantics, simplify the rest of the text to get rid of the
>> confusion.
>>
>> ALLOCATED is always set by the block layer for convenience (drivers
>> do not have to worry about it). RAW is used only internally, but
> 
> Just one space after the period? How inconsistent! :-)

But do commit messages really count?  :)


>> + * Internal flag:
>> + * BDRV_BLOCK_RAW: used internally to indicate that the request was
>> + *                 answered by a passthrough driver such as raw and that the
> 
> s/passthrough/filter/? But I'm not even sure myself. Well, "passthrough"
> is a safe bet, so let's just go with it.

Calling 'raw' a filter driver is a bit weird - but in one sense, it is a
no-op filter (filter the protocol layer into the format layer by doing
nothing).  Meanwhile 'commit' certainly sounds like more of a filter
than a passthrough.  I could go either way, and filter is slightly
shorter.  If there's a real reason to respin the series, 'filter' seems
reasonable if we're worried about line length, otherwise I'm just as
inclined to leave it alone.

> 
> With the commit message fixed or a “no it's fine”:
> 
> Reviewed-by: Max Reitz <address@hidden>
> 

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

signature.asc
Description: OpenPGP digital signature