[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 12/21] block: define get_block_status return
Re: [Qemu-devel] [PATCH v5 12/21] block: define get_block_status return value
Tue, 06 May 2014 14:31:37 +0200
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0
Il 06/05/2014 13:34, Kevin Wolf ha scritto:
But the result still wouldn't be right. We have an implication that you
can't simply reverse:
If this block is unallocated and unallocated blocks read as zero,
then this block reads as zero.
You're trying to use something like "If the block reads as zero and
unallocated blocks read as zero, it must be unallocated", which quite
obviously doesn't work.
This is not what the condition says in is_allocated. The right one is
(excluding the backing_hd case, which is handled with a function like
the one I sketched in the previous message):
If the block reads as zero and not all unallocated blocks read as
zero, the sector will be read from BS rather than the backing file,
even if it is not allocated.
This is true, because bdrv_get_block_status does not ask the backing
file whether it is zero in the backing file. Thus, reading from the
backing file does not guarantee to read zeroes.
Since you wrote "something like", I guess it's not the difference
between what you wrote and what I wrote is not too relevant. But here
is the pedantic explanation of it:
- is_allocated does not answer the question "is the block allocated",
but rather "will the sector be read from BS or from a backing file?" (or
from an imaginary all-zeroes backing file if there is none). This is
why it is better to rename is_allocated before it brings more confusion
(and the brain damage I got from this confusion is also the reason why
BDRV_BLOCK_DATA is not called BDRV_BLOCK_ALLOCATED).
- this is the right hand side of a "||", so the outcome is not "it must
be unallocated", but "it must be allocated". Or better, given the
previous point, "the sector will be read from BS"
- the bdrv_has_zero_init (which is wrong, but can be fixed) is negated,
so it is not "unallocated blocks are zero" but rather "if the block
reads as zero and not all unallocated blocks read as zero".