Re: backing chain & block status & filters

[Eric Blake <eblake@redhat.com>]

On 4/28/20 10:13 AM, Vladimir Sementsov-Ogievskiy wrote:

>>> Hm.  I could imagine that there are formats that have non-zero holes
>>> (e.g. 0xff or just garbage).  It would be a bit wrong for them to return
>>> ZERO or DATA then.
>>>
>>> But OTOH we don’t care about such cases, do we?  We need to know whether
>>> ranges are zero, data, or unallocated.  If they aren’t zero, we only
>>> care about whether reading from it will return data from this layer 
>>> or not.
>>>
>>> So I suppose that anything that doesn’t support backing files (or
>>> filtered children) should always return ZERO and/or DATA.
>>
>> I'm not sure I agree with the notion that everything should be
>> BDRV_BLOCK_ALLOCATED at the lowest layer. It's not what it means today
>> at least. If we want to change this, we will have to check all callers
>> of bdrv_is_allocated() and friends who might use this to find holes in
>> the file.
> 
> Yes. Because they are doing incorrect (or at least undocumented and 
> unreliable) thing.

Here's some previous mails discussing the same question about what 
block_status should actually mean.  At the time, I was so scared of the 
prospect of something breaking if I changed things that I ended up 
keeping status quo, so here we are revisiting the topic several years 
later, still asking the same questions.

https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg00069.html
https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg03757.html

> 
>>
>> Basically, the way bdrv_is_allocated() works today is that we assume an
>> implicit zeroed backing layer even for block drivers that don't support
>> backing files.
> 
> But read doesn't work so: it will read data from the bottom layer, not from
> this implicit zeroed backing layer. And it is inconsistent. On read data
> comes exactly from this layer, not from its implicit backing. So it should
> return BDRV_BLOCK_ALLOCATED, accordingly to its definition..
> 
> Or, we should at least document current behavior:
> 
>    BDRV_BLOCK_ALLOCATED: the content of the block is determined by this
>    layer rather than any backing, set by block. Attention: it may not be 
> set
>    for drivers without backing support, still data is of course read from
>    this layer. Note, that for such drivers BDRV_BLOCK_ALLOCATED may mean
>    allocation on fs level, which occupies real space on disk.. So, for 
> such drivers
> 
>    ZERO | ALLOCATED means that, read as zero, data may be allocated on 
> fs, or
>    (most probably) not,
>    don't look at ALLOCATED flag, as it is added by generic layer for 
> another logic,
>    not related to fs-allocation.
> 
>    0 means that, most probably, data doesn't occupy space on fs, 
> zero-status is
>    unknown (most probably non-zero)
> 

That may be right in describing the current situation, but again, needs 
a GOOD audit of what we are actually using it for, and whether it is 
what we really WANT to be using it for.  If we're going to 
audit/refactor the code, we might as well get semantics that are 
actually useful, rather than painfully contorted to documentation that 
happens to match our current contorted code.

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