22.10.2020 21:13, Andrey Shinkevich wrote:
> Limit COR operations to the bottom node (inclusively) in the backing
> chain when the bottom node name is given. It will be useful for a block
> stream job when the COR-filter is applied. The bottom 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@virtuozzo.com>
> ---
> block/copy-on-read.c | 42 ++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/block/copy-on-read.c b/block/copy-on-read.c
> index 3d8e4db..8178a91 100644
> --- a/block/copy-on-read.c
> +++ b/block/copy-on-read.c
> @@ -123,8 +123,46 @@ static int coroutine_fn cor_co_preadv_part(BlockDriverState *bs,
> size_t qiov_offset,
> int flags)
> {
> - return bdrv_co_preadv_part(bs->file, offset, bytes, qiov, qiov_offset,
> - flags | BDRV_REQ_COPY_ON_READ);
> + int64_t n = 0;
no need to initialize n actually.
> + int local_flags;
> + int ret;
> + BDRVStateCOR *state = bs->opaque;
> +
> + if (!state->bottom_bs) {
> + return bdrv_co_preadv_part(bs->file, offset, bytes, qiov, qiov_offset,
> + flags | BDRV_REQ_COPY_ON_READ);
> + }
> +
> + while (bytes) {
> + local_flags = flags;
> +
> + /* In case of failure, try to copy-on-read anyway */
> + ret = bdrv_is_allocated(bs->file->bs, offset, bytes, &n);
> + if (!ret || ret < 0) {
simper: if (ret <= 0) {
> + ret = bdrv_is_allocated_above(bdrv_backing_chain_next(bs->file->bs),
> + state->bottom_bs, true, offset,
> + n, &n);
> + if (ret == 1 || ret < 0) {
> + local_flags |= BDRV_REQ_COPY_ON_READ;
> + }
> + /* Finish earlier if the end of a backing file has been reached */
> + if (ret == 0 && n == 0) {
"n == 0" should be enough
> + break;
> + }
> + }
> +
> + ret = bdrv_co_preadv_part(bs->file, offset, n, qiov, qiov_offset,
> + local_flags);
> + if (ret < 0) {
> + return ret;
> + }
> +
> + offset += n;
> + qiov_offset += n;
> + bytes -= n;
> + }
> +
> + return 0;
> }
>
My remarks are minor, so:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
And yes, I still think that this is good to be merged with two previous patches.
--
Best regards,
Vladimir