Re: [Qemu-devel] [PATCH v5 03/10] block: simplify bdrv_find_base() and bdrv_find_overlay()

[Benoît Canet]

The Friday 13 Jun 2014 à 14:52:31 (-0400), Jeff Cody wrote :
> This simplifies the function bdrv_find_overlay().  With this change,
> bdrv_find_base() is just a subset of usage of bdrv_find_overlay(),
> so this also takes advantage of that.
> 
> Reviewed-by: Eric Blake <address@hidden>
> Signed-off-by: Jeff Cody <address@hidden>
> ---
>  block.c | 45 ++++++++++-----------------------------------
>  1 file changed, 10 insertions(+), 35 deletions(-)
> 
> diff --git a/block.c b/block.c
> index f9ea1b4..83996e3 100644
> --- a/block.c
> +++ b/block.c
> @@ -2569,32 +2569,23 @@ int bdrv_change_backing_file(BlockDriverState *bs,
>   *
>   * Returns NULL if bs is not found in active's image chain,
>   * or if active == bs.
> + *
> + * Returns the bottommost base image if bs == NULL.
>   */
>  BlockDriverState *bdrv_find_overlay(BlockDriverState *active,
>                                      BlockDriverState *bs)
>  {
> -    BlockDriverState *overlay = NULL;
> -    BlockDriverState *intermediate;
> -
> -    assert(active != NULL);
> -    assert(bs != NULL);
> -
> -    /* if bs is the same as active, then by definition it has no overlay
> -     */
> -    if (active == bs) {
> -        return NULL;
> +    while (active && bs != active->backing_hd) {
> +        active = active->backing_hd;
>      }
>  
> -    intermediate = active;
> -    while (intermediate->backing_hd) {
> -        if (intermediate->backing_hd == bs) {
> -            overlay = intermediate;
> -            break;
> -        }
> -        intermediate = intermediate->backing_hd;
> -    }
> +    return active;
> +}
>  
> -    return overlay;
> +/* Given a BDS, searches for the base layer. */
> +BlockDriverState *bdrv_find_base(BlockDriverState *bs)
> +{
> +    return bdrv_find_overlay(bs, NULL);
>  }
>  
>  typedef struct BlkIntermediateStates {
> @@ -4376,22 +4367,6 @@ int bdrv_get_backing_file_depth(BlockDriverState *bs)
>      return 1 + bdrv_get_backing_file_depth(bs->backing_hd);
>  }
>  
> -BlockDriverState *bdrv_find_base(BlockDriverState *bs)
> -{
> -    BlockDriverState *curr_bs = NULL;
> -
> -    if (!bs) {
> -        return NULL;
> -    }
> -
> -    curr_bs = bs;
> -
> -    while (curr_bs->backing_hd) {
> -        curr_bs = curr_bs->backing_hd;
> -    }
> -    return curr_bs;
> -}
> -
>  /**************************************************************/
>  /* async I/Os */
>  
> -- 
> 1.8.3.1
> 
> 

That is a neat simplification.

Reviewed-by: Benoit Canet <address@hidden>