Re: [PATCH v4 0/5] fix & merge block_status_above and is_allocated_above

[Stefan Hajnoczi]

On Thu, May 28, 2020 at 01:15:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Thanks to Eric, the whole series is reviewed now!
> 
> v4:
> 01: fix grammar in comment, add Eric's r-b
> 02-05: add Eric's r-b
> 
> Hi all!
> 
> These series are here to address the following problem:
> block-status-above functions may consider space after EOF of
> intermediate backing files as unallocated, which is wrong, as these
> backing files are the reason of producing zeroes, we never go further by
> backing chain after a short backing file. So, if such short-backing file
> is _inside_ requested sub-chain of the backing chain, we should never
> report space after its EOF as unallocated.
> 
> See patches 01,04,05 for details.
> 
> Note, that this series leaves for another day the general problem
> around block-status: misuse of BDRV_BLOCK_ALLOCATED as is-fs-allocated
> vs go-to-backing.
> Audit for this problem is done here:
> "backing chain & block status & filters"
> https://lists.gnu.org/archive/html/qemu-devel/2020-04/msg04706.html
> And I'm going to prepare series to address this problem.
> 
> Also, get_block_status func have same disease, but remains unfixed here:
> I want to make separate series for it, as it need some more refactoring,
> which should be based on series
> "[PATCH v5 0/7] coroutines: generate wrapper code"
> 
> Vladimir Sementsov-Ogievskiy (5):
>   block/io: fix bdrv_co_block_status_above
>   block/io: bdrv_common_block_status_above: support include_base
>   block/io: bdrv_common_block_status_above: support bs == base
>   block/io: fix bdrv_is_allocated_above
>   iotests: add commit top->base cases to 274
> 
>  block/io.c                 | 105 +++++++++++++++++++------------------
>  block/qcow2.c              |  16 +++++-
>  tests/qemu-iotests/274     |  20 +++++++
>  tests/qemu-iotests/274.out |  65 +++++++++++++++++++++++
>  4 files changed, 152 insertions(+), 54 deletions(-)

Hi Vladimir,
Which series is this based on? It does not apply. Is there a dependency
on the coroutine wrappers series?

Aside from the issue applying the patches this series looks good to me.

Thanks,
Stefan

signature.asc
Description: PGP signature