[for-2.4,5/5] block: Fix backing file child when modifying graph

This patch moves bdrv_attach_child() from the individual places that add
a backing file to a BDS to bdrv_set_backing_hd(), which is called by all
of them. It also adds bdrv_detach_child() there.

For normal operation (starting with one backing file chain and not
changing it until the topmost image is closed) and live snapshots, this
constitutes no change in behaviour.

For all other cases, this is a fix for the bug that the old backing file
was still referenced as a child, and the new one wasn't referenced.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c                   | 5 +++--
 include/block/block_int.h | 1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

On 08.07.2015 21:36, Kevin Wolf wrote:
> This patch moves bdrv_attach_child() from the individual places that add
> a backing file to a BDS to bdrv_set_backing_hd(), which is called by all
> of them. It also adds bdrv_detach_child() there.
>
> For normal operation (starting with one backing file chain and not
> changing it until the topmost image is closed) and live snapshots, this
> constitutes no change in behaviour.
>
> For all other cases, this is a fix for the bug that the old backing file
> was still referenced as a child, and the new one wasn't referenced.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   block.c                   | 5 +++--
>   include/block/block_int.h | 1 +
>   2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/block.c b/block.c
> index d5c9f03..d088ee0 100644
> --- a/block.c
> +++ b/block.c
> @@ -1141,6 +1141,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd)
>       if (bs->backing_hd) {
>           assert(bs->backing_blocker);
>           bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker);
> +        bdrv_detach_child(bs->backing_child);
>       } else if (backing_hd) {
>           error_setg(&bs->backing_blocker,
>                      "node is used as backing hd of '%s'",
> @@ -1151,8 +1152,10 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd)
>       if (!backing_hd) {
>           error_free(bs->backing_blocker);
>           bs->backing_blocker = NULL;
> +        bs->backing_child = NULL;

I'd prefer this to be directly below the bdrv_detach_child() call, but 
that's just a question of personal preference.

Reviewed-by: Max Reitz <mreitz@redhat.com>

>           goto out;
>       }
> +    bs->backing_child = bdrv_attach_child(bs, backing_hd, &child_backing);
>       bs->open_flags &= ~BDRV_O_NO_BACKING;
>       pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_hd->filename);
>       pstrcpy(bs->backing_format, sizeof(bs->backing_format),
> @@ -1236,7 +1239,6 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
>           goto free_exit;
>       }
>
> -    bdrv_attach_child(bs, backing_hd, &child_backing);
>       bdrv_set_backing_hd(bs, backing_hd);
>
>   free_exit:
> @@ -2171,7 +2173,6 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top)
>       /* The contents of 'tmp' will become bs_top, as we are
>        * swapping bs_new and bs_top contents. */
>       bdrv_set_backing_hd(bs_top, bs_new);
> -    bdrv_attach_child(bs_top, bs_new, &child_backing);
>   }
>
>   static void bdrv_delete(BlockDriverState *bs)
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 8996baf..2cc973c 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -379,6 +379,7 @@ struct BlockDriverState {
>       char exact_filename[PATH_MAX];
>
>       BlockDriverState *backing_hd;
> +    BdrvChild *backing_child;
>       BlockDriverState *file;
>
>       NotifierList close_notifiers;
>