Re: [PATCH v3 22/36] block: add bdrv_remove_filter_or_cow transaction action

[Kevin Wolf]

Am 17.03.2021 um 15:35 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 76 insertions(+), 2 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 11f7ad0818..2fca1f2ad5 100644
> --- a/block.c
> +++ b/block.c
> @@ -2929,12 +2929,19 @@ static void bdrv_replace_child(BdrvChild *child, 
> BlockDriverState *new_bs)
>      }
>  }
>  
> +static void bdrv_child_free(void *opaque)
> +{
> +    BdrvChild *c = opaque;
> +
> +    g_free(c->name);
> +    g_free(c);
> +}
> +
>  static void bdrv_remove_empty_child(BdrvChild *child)
>  {
>      assert(!child->bs);
>      QLIST_SAFE_REMOVE(child, next);
> -    g_free(child->name);
> -    g_free(child);
> +    bdrv_child_free(child);
>  }
>  
>  typedef struct BdrvAttachChildCommonState {
> @@ -4956,6 +4963,73 @@ static bool should_update_child(BdrvChild *c, 
> BlockDriverState *to)
>      return ret;
>  }
>  
> +typedef struct BdrvRemoveFilterOrCowChild {
> +    BdrvChild *child;
> +    bool is_backing;
> +} BdrvRemoveFilterOrCowChild;
> +
> +/* this doesn't restore original child bs, only the child itself */

Hm, this comment tells me that it's intentional, but why is it correct?

> +static void bdrv_remove_filter_or_cow_child_abort(void *opaque)
> +{
> +    BdrvRemoveFilterOrCowChild *s = opaque;
> +    BlockDriverState *parent_bs = s->child->opaque;
> +
> +    QLIST_INSERT_HEAD(&parent_bs->children, s->child, next);
> +    if (s->is_backing) {
> +        parent_bs->backing = s->child;
> +    } else {
> +        parent_bs->file = s->child;
> +    }
> +}

Kevin