Re: [Qemu-block] [PATCH v3 3/3] block/stream: introduce a bottom node

[Vladimir Sementsov-Ogievskiy]

05.04.2019 19:56, Andrey Shinkevich wrote:
> The bottom node is the intermediate block device that has the base as its
> backing image. It is used instead of the base node while a block stream
> job is running to avoid dependency on the base that may change due to the
> parallel jobs. The change may take place due to a filter node as well that
> is inserted between the base and the intermediate bottom node. It occurs
> when the base node is the top one for another commit or stream job.
> After the introduction of the bottom node, don't freeze its backing child,
> that's the base, anymore.
> 
> Suggested-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> Signed-off-by: Andrey Shinkevich <address@hidden>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>   block/stream.c         | 56 
> +++++++++++++++++++++++++++-----------------------
>   block/trace-events     |  2 +-
>   tests/qemu-iotests/245 |  4 ++--
>   3 files changed, 33 insertions(+), 29 deletions(-)
> 

[..]

> @@ -232,8 +232,13 @@ void stream_start(const char *job_id, BlockDriverState 
> *bs,
>       StreamBlockJob *s;
>       BlockDriverState *iter;
>       bool bs_read_only;
> +    BlockDriverState *bottom = NULL;

Why to set NULL? you can set to bdrv_find_overlay() here.

> +    int basic_flags = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED;
> +
> +    /* Find the bottom node that has the base as its backing image */

I don't think we need comment, as it's exactly what bdrv_find_overly does.

> +    bottom = bdrv_find_overlay(bs, base);
>   
> -    if (bdrv_freeze_backing_chain(bs, base, errp) < 0) {
> +    if (bdrv_freeze_backing_chain(bs, bottom, errp) < 0) {
>           return;
>       }
>   


-- 
Best regards,
Vladimir