Re: [PATCH v11 05/13] copy-on-read: limit COR operations to base in COR driver

On 14.10.2020 15:01, Max Reitz wrote: > On 12.10.20 19:43, Andrey Shinkevich wrote: >> Limit COR operations by the base node in the backing chain when the >> overlay base node name is given. It will be useful for a block stream >> job when the COR-filter is applied. The overlay base node is passed as >> the base itself may change due to concurrent commit jobs on the same >> backing chain. >> >> Signed-off-by: Andrey Shinkevich <andrey.shinkevich(a)virtuozzo.com&gt; >> --- >>   block/copy-on-read.c | 39 +++++++++++++++++++++++++++++++++++++-- >>   1 file changed, 37 insertions(+), 2 deletions(-) >> >> diff --git a/block/copy-on-read.c b/block/copy-on-read.c >> index c578b1b..dfbd6ad 100644 >> --- a/block/copy-on-read.c >> +++ b/block/copy-on-read.c >> @@ -122,8 +122,43 @@ static int coroutine_fn >> cor_co_preadv_part(BlockDriverState *bs, >>                                              size_t qiov_offset, >>                                              int flags) >>   { [...] >> +            ret = bdrv_is_allocated_above(bdrv_cow_bs(bs->file->bs), >> +                                          state->base_overlay, true, >> offset, >> +                                          n, &n); >> +            if (ret) { >> +                local_flags |= BDRV_REQ_COPY_ON_READ; >> +            } >> +        } > > Furthermore, I just noticed – can the is_allocated functions not return > 0 in @n, when @offset is a the EOF?  Is that something to look out for? >   (I’m not sure.) > > Max > The check for EOF is managed earlier in the stream_run() for a block-stream job. For other cases of using the COR-filter, the check for EOF can be added to the cor_co_preadv_part(). I would be more than happy if we can escape the duplicated checking for is_allocated in the block-stream. But how the stream_run() can stop calling the blk_co_preadv() when EOF is reached if is_allocated removed from it?

May the cor_co_preadv_part() return EOF (or other error code) to be handled by a caller if (ret == 0 && n == 0 && (flags & BDRV_REQ_PREFETCH)?