Re: [PATCH 07/22] blkverify: Implement .bdrv_recurse_can_replace()

[Vladimir Sementsov-Ogievskiy]

20.09.2019 18:27, Max Reitz wrote:
> Signed-off-by: Max Reitz <address@hidden>
> ---
>   block/blkverify.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/block/blkverify.c b/block/blkverify.c
> index 304b0a1368..0add3ab483 100644
> --- a/block/blkverify.c
> +++ b/block/blkverify.c
> @@ -282,6 +282,20 @@ static bool 
> blkverify_recurse_is_first_non_filter(BlockDriverState *bs,
>       return bdrv_recurse_is_first_non_filter(s->test_file->bs, candidate);
>   }
>   
> +static bool blkverify_recurse_can_replace(BlockDriverState *bs,
> +                                          BlockDriverState *to_replace)
> +{
> +    BDRVBlkverifyState *s = bs->opaque;
> +
> +    /*
> +     * blkverify quits the whole qemu process if there is a mismatch
> +     * between bs->file->bs and s->test_file->bs.  Therefore, we know
> +     * know that both must match bs and we can recurse down to either.
> +     */
> +    return bdrv_recurse_can_replace(bs->file->bs, to_replace) ||
> +           bdrv_recurse_can_replace(s->test_file->bs, to_replace);

Ok, now I understand, what bdrv_recurse_can_replace actually does:

It searches for to_replace in bs-rooted subtree, going only through equal
children..

So, we can replace @to_replace, by something equal to @bs, if @to_replace is
in equal-subtree of @bs.

I'll try to explain my misleading:

bdrv_recurse_can_replace declaration looks like bs and to_replace may be 
absolutely
unrelated nodes. So, why bs should decide, can we replace the unrelated 
to_replace
node by something..

So, it may be simpler to follow, if it was called 
bdrv_recurse_filtered_subtree, or
bdrv_recurse_transparent_subtree..

Still, now I understand, and don't care. It's better anyway than 
bdrv_recurse_is_first_non_filter

> +}
> +
>   static void blkverify_refresh_filename(BlockDriverState *bs)
>   {
>       BDRVBlkverifyState *s = bs->opaque;
> @@ -328,6 +342,7 @@ static BlockDriver bdrv_blkverify = {
>   
>       .is_filter                        = true,
>       .bdrv_recurse_is_first_non_filter = 
> blkverify_recurse_is_first_non_filter,
> +    .bdrv_recurse_can_replace         = blkverify_recurse_can_replace,

it will be never called, as bdrv_recurse_can_replace handles filters by itself.

>   };
>   
>   static void bdrv_blkverify_init(void)
> 


-- 
Best regards,
Vladimir