Re: [Qemu-devel] [PATCH v5 12/21] block: define get_block_status return value

[Paolo Bonzini]

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.

<pedantic>
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".
</pedantic>

Paolo