|
From: | Eric Blake |
Subject: | Re: backing chain & block status & filters |
Date: | Tue, 28 Apr 2020 11:18:48 -0500 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 |
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 onlycare 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 thislayer rather than any backing, set by block. Attention: it may not be setfor drivers without backing support, still data is of course read from this layer. Note, that for such drivers BDRV_BLOCK_ALLOCATED may meanallocation on fs level, which occupies real space on disk.. So, for such driversZERO | 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 isunknown (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
[Prev in Thread] | Current Thread | [Next in Thread] |