Re: [Qemu-block] [PATCH v3 03/20] file-posix: Switch to .bdrv_co_block_status()

[Eric Blake]

On 09/20/2017 04:57 AM, Fam Zheng wrote:
> On Thu, 09/14 09:40, Eric Blake wrote:
>> We are gradually moving away from sector-based interfaces, towards
>> byte-based.  Update the file protocol driver accordingly.  In mapping
>> mode, note that the entire file is reported as allocated, so we can
>> take a shortcut and skip lseek().
>>
>> Signed-off-by: Eric Blake <address@hidden>
>>

>>
>> -    ret = find_allocation(bs, start, &data, &hole);
>> +    if (!mapping) {
>> +        *pnum = bytes;
>> +        *file = bs;
>> +        return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID |
>> +            (offset & BDRV_BLOCK_OFFSET_MASK);
>> +    }
> 
> I may be missing something, because the last time I tried to understand the
> rationale behind "mapping" was already some time ago: shouldn't we still
> distinguish hole and data? What will omitting BDRV_BLOCK_ZERO help?

Hmm, the commit message is slightly off (in part, because I switched the
sense of the bool flag between series revisions, but did not properly
update the commit text to match).  In mapping mode, we want to return as
much information as possible (the client is something like 'qemu-img
map'), including where the holes lie.  But when we are NOT in mapping
mode, we care more about learning which portions of the file are
described in the current layer of the backing chain, rather than
delegating to another layer, regardless of whether the read will see
data or zeroes.  By the time we are at the POSIX file protocol layer, we
know that every byte in the file system/block device has a 1:1 mapping
to the bytes that the guest will read (we do not delegate to any backing
file), so we can simply report the entire remainder of the file as
allocated without worrying about holes.

Here's where the mapping flag was added and semantics documented (in
series 3; whereas the current email is series 4):
https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg03542.html

So what I really need to do is fix the commit message to read:

In mapping mode, we must report as much information as possible about
where holes can be found; but when we don't care about mapping, the user
is more interested in how much of the guest view will come from the
current layer rather than delegating to some other BDS, and we can take
the shortcut that all of the remainder of the file fits that
description, and therefore take a shortcut and skip lseek() for a larger
*pnum result.

(the same comment probably applies to several other patches in the series)

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

signature.asc
Description: OpenPGP digital signature