Re: [PATCH 2/7] stream: exclude a link to filter from freezing

[Vladimir Sementsov-Ogievskiy]

20.04.2020 21:36, Andrey Shinkevich wrote:
> A node above the base can be the filter of the concurrent job. In that
> case, the filter cannot be removed being a part of the frozen chain.
> Exclude the link to filter node from freezing and provide the safety
> check for a concurrent job.
>
> Signed-off-by: Andrey Shinkevich <address@hidden>
> ---
>   block/stream.c | 16 +++++++++++++---
>   1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/block/stream.c b/block/stream.c
> index bd4a351..d8b4bbe 100644
> --- a/block/stream.c
> +++ b/block/stream.c
> @@ -244,7 +244,7 @@ void stream_start(const char *job_id, BlockDriverState *bs,
>            above_base = bdrv_filtered_bs(above_base))
>       {}
>   
> -    if (bdrv_freeze_chain(bs, above_base, errp) < 0) {
> +    if (bdrv_freeze_chain(bs, bottom_cow_node, errp) < 0) {
>           return;
>       }
>   
> @@ -257,6 +257,15 @@ void stream_start(const char *job_id, BlockDriverState *bs,
>           }
>       }
>   
> +    /*
> +     * Check for an overlapping block-commit job that is not allowed.
> +     */
> +    if (bdrv_freeze_chain(bottom_cow_node, above_base, errp) < 0) {
> +        goto fail;
> +    } else {
> +        bdrv_unfreeze_chain(bottom_cow_node, above_base);
> +    }
> +
>       /* Prevent concurrent jobs trying to modify the graph structure here, we
>        * already have our own plans. Also don't allow resize as the image size 
> is
>        * queried only at the job start and then cached. */
> @@ -276,7 +285,8 @@ void stream_start(const char *job_id, BlockDriverState *bs,
>        * bdrv_reopen_set_read_only() due to parallel block jobs running.
>        */
>       base = bdrv_filtered_bs(above_base);
> -    for (iter = bdrv_filtered_bs(bs); iter && iter != base;
> +    for (iter = bdrv_filtered_bs(bs);
> +         iter && iter != base && iter->drv && !iter->drv->is_filter;
>            iter = bdrv_filtered_bs(iter))
>       {
>           block_job_add_bdrv(&s->common, "intermediate node", iter, 0,

So, we still "own" these nodes above base, but just exclude them from freezing?
It's wrong I think. We should exclude them at all (look my previous answer).

> @@ -298,5 +308,5 @@ fail:
>       if (bs_read_only) {
>           bdrv_reopen_set_read_only(bs, true, NULL);
>       }
> -    bdrv_unfreeze_chain(bs, above_base);
> +    bdrv_unfreeze_chain(bs, bottom_cow_node);
>   }


--
Best regards,
Vladimir