Re: [Qemu-devel] [PATCH v4 1/3] block: include base when checking image chain for block allocation

[Alberto Garcia]

On Mon 08 Apr 2019 08:22:19 PM CEST, Andrey Shinkevich wrote:
>   * Return true if (a prefix of) the given range is allocated in any image
> - * between BASE and TOP (inclusive).  BASE can be NULL to check if the given
> + * between BASE and TOP (TOP included). To check the BASE image, set the
> + * 'include_base' to 'true'. The BASE can be NULL to check if the given
>   * offset is allocated in any image of the chain.  Return false otherwise,
>   * or negative errno on failure.

I'm not a native speaker but that sounds a bit odd to me.

Alternative: 

  * Return true if (a prefix of) the given range is allocated in any image
  * between BASE and TOP (BASE is only included if include_base is set).
  * BASE can be NULL to check if the given offset is allocated in any
  * image of the chain.  Return false otherwise, or negative errno on
  * failure.

> -    while (intermediate && intermediate != base) {
> +    while (include_base || intermediate != base) {
>          int64_t pnum_inter;
>          int64_t size_inter;
>  
> @@ -2360,6 +2364,10 @@ int bdrv_is_allocated_above(BlockDriverState *top,
>              n = pnum_inter;
>          }
>  
> +        if (intermediate == base) {
> +            break;
> +        }
> +
>          intermediate = backing_bs(intermediate);

I find that the new condition + the break make things a bit less
readable. I think it would be simpler with something like this:

   BlockDriverState *stop_at = include_base ? backing_bs(base) : base;

   while (intermediate != stop_at) {
      ...
   }

(stop_at is a terrible name, but I can't think of anything better at the
moment)

Berto