Re: [Qemu-devel] [PATCH 1/5] block: Add BDS.never_freeze

[Andrey Shinkevich]

On 28/06/2019 01:32, Max Reitz wrote:
> The commit and the mirror block job must be able to drop their filter
> node at any point.  However, this will not be possible if any of the
> BdrvChild links to them is frozen.  Therefore, we need to prevent them
> from ever becoming frozen.
> 
> Signed-off-by: Max Reitz <address@hidden>
> ---
>   include/block/block_int.h | 3 +++
>   block.c                   | 8 ++++++++
>   block/commit.c            | 4 ++++
>   block/mirror.c            | 4 ++++
>   4 files changed, 19 insertions(+)
> 
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index d6415b53c1..50902531b7 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -885,6 +885,9 @@ struct BlockDriverState {
>   
>       /* Only read/written by whoever has set active_flush_req to true.  */
>       unsigned int flushed_gen;             /* Flushed write generation */
> +
> +    /* BdrvChild links to this node may never be frozen */
> +    bool never_freeze;
>   };
>   
>   struct BlockBackendRootState {
> diff --git a/block.c b/block.c
> index c139540f2b..6565192b91 100644
> --- a/block.c
> +++ b/block.c
> @@ -4416,6 +4416,14 @@ int bdrv_freeze_backing_chain(BlockDriverState *bs, 
> BlockDriverState *base,
>           return -EPERM;
>       }
>   
> +    for (i = bs; i != base; i = backing_bs(i)) {
> +        if (i->backing && backing_bs(i)->never_freeze) {
> +            error_setg(errp, "Cannot freeze '%s' link to '%s'",
> +                       i->backing->name, backing_bs(i)->node_name);
> +            return -EPERM;
> +        }
> +    }
> +
>       for (i = bs; i != base; i = backing_bs(i)) {
>           if (i->backing) {
>               i->backing->frozen = true;
> diff --git a/block/commit.c b/block/commit.c
> index ca7e408b26..2c5a6d4ebc 100644
> --- a/block/commit.c
> +++ b/block/commit.c
> @@ -298,6 +298,10 @@ void commit_start(const char *job_id, BlockDriverState 
> *bs,
>       if (!filter_node_name) {
>           commit_top_bs->implicit = true;
>       }
> +
> +    /* So that we can always drop this node */
> +    commit_top_bs->never_freeze = true;
> +
>       commit_top_bs->total_sectors = top->total_sectors;
>   
>       bdrv_append(commit_top_bs, top, &local_err);
> diff --git a/block/mirror.c b/block/mirror.c
> index 2fcec70e35..8cb75fb409 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -1551,6 +1551,10 @@ static BlockJob *mirror_start_job(
>       if (!filter_node_name) {
>           mirror_top_bs->implicit = true;
>       }
> +
> +    /* So that we can always drop this node */
> +    mirror_top_bs->never_freeze = true;
> +
>       mirror_top_bs->total_sectors = bs->total_sectors;
>       mirror_top_bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED;
>       mirror_top_bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED |
> 

Reviewed-by: Andrey Shinkevich <address@hidden>
-- 
With the best regards,
Andrey Shinkevich