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

[Vladimir Sementsov-Ogievskiy]

28.05.2020 20:09, Stefan Hajnoczi wrote:
> 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.

It's on current master. Yes it conflicts with coroutine wrappers.

Still, actually, "[PATCH v3] block: Factor out bdrv_run_co()" was substituted
by "[PATCH v5 0/7] coroutines: generate wrapper code", which is partly reviewed.
Sorry, I should have answered on "Factor out bdrv_run_co()" about that. 
Actually,
it was mentioned in "[PATCH v2] block: Factor out bdrv_run_co()", I just posted
v3 too early.

So, I'd prefer to merge these series now about block-status, instead of
"[PATCH v3] block: Factor out bdrv_run_co()", and then I'll rebase
"[PATCH v5 0/7] coroutines: generate wrapper code"


--
Best regards,
Vladimir