Am 18.02.2020 um 13:42 hat Max Reitz geschrieben:
> Make all parents of backing files pass the appropriate BdrvChildRole.
> By doing so, we can switch their BdrvChildClass over to the generic
> child_of_bds, which will do the right thing when given a correct
> BdrvChildRole.
>
> Signed-off-by: Max Reitz <mre...@redhat.com>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
> ---
> block.c | 26 ++++++++++++++++++++------
> block/backup-top.c | 2 +-
> block/vvfat.c | 3 ++-
> tests/test-bdrv-drain.c | 13 +++++++------
> 4 files changed, 30 insertions(+), 14 deletions(-)
>
> diff --git a/block.c b/block.c
> index 43df38ca30..31affcb4ee 100644
> --- a/block.c
> +++ b/block.c
> @@ -2770,6 +2770,20 @@ static bool
> bdrv_inherits_from_recursive(BlockDriverState *child,
> return child != NULL;
> }
>
> +/*
> + * Return the BdrvChildRole for @bs's backing child. bs->backing is
> + * mostly used for COW backing children (role = COW), but also for
> + * filtered children (role = FILTERED | PRIMARY).
> + */
> +static BdrvChildRole bdrv_backing_role(BlockDriverState *bs)
> +{
> + if (bs->drv && bs->drv->is_filter) {
> + return BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY;
> + } else {
> + return BDRV_CHILD_COW;
> + }
> +}
bdrv_mirror_top and bdrv_commit_top don't set .is_filter, so it will
return the wrong role for them. (They also don't set .supports_backing,
so grepping for that wouldn't help...)
> /*
> * Sets the backing file link of a BDS. A new reference is created; callers
> * which don't need their own reference any more must call bdrv_unref().
> @@ -2797,8 +2811,8 @@ void bdrv_set_backing_hd(BlockDriverState *bs,
> BlockDriverState *backing_hd,
> goto out;
> }
>
> - bs->backing = bdrv_attach_child(bs, backing_hd, "backing",
> &child_backing,
> - 0, errp);
> + bs->backing = bdrv_attach_child(bs, backing_hd, "backing", &child_of_bds,
> + bdrv_backing_role(bs), errp);
> /* If backing_hd was already part of bs's backing chain, and
> * inherits_from pointed recursively to bs then let's update it to
> * point directly to bs (else it will become NULL). */
> @@ -2895,7 +2909,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict
> *parent_options,
> }
>
> backing_hd = bdrv_open_inherit(backing_filename, reference, options, 0,
> bs,
> - &child_backing, 0, errp);
> + &child_of_bds, BDRV_CHILD_COW, errp);
Wouldn't it be more consistent to use bdrv_backing_role() here, too?
Kevin
