Re: [Qemu-devel] [PATCH 17/26] qemu-io: fix the alloc command

[Kevin Wolf]

Am 12.04.2012 14:01, schrieb Paolo Bonzini:
> Because sector_num is not updated, the loop would either go on
> forever or return garbage.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  qemu-io.c |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/qemu-io.c b/qemu-io.c
> index 43643c8..27a0c3c 100644
> --- a/qemu-io.c
> +++ b/qemu-io.c
> @@ -1560,7 +1560,7 @@ out:
>  
>  static int alloc_f(int argc, char **argv)
>  {
> -    int64_t offset;
> +    int64_t offset, sector_num;
>      int nb_sectors, remaining;
>      char s1[64];
>      int num, sum_alloc;
> @@ -1581,12 +1581,18 @@ static int alloc_f(int argc, char **argv)
>  
>      remaining = nb_sectors;
>      sum_alloc = 0;
> +    sector_num = offset >> 9;
>      while (remaining) {
> -        ret = bdrv_is_allocated(bs, offset >> 9, nb_sectors, &num);
> +        ret = bdrv_is_allocated(bs, sector_num, remaining, &num);
> +        sector_num += num;
>          remaining -= num;
>          if (ret) {
>              sum_alloc += num;
>          }
> +        if (num == 0) {
> +            nb_sectors -= remaining;
> +            remaining = 0;
> +        }
>      }
>  
>      cvtstr(offset, s1, sizeof(s1));

This doesn't provide the semantics I expected, i.e. the semantics of
bdrv_is_allocated, which is the number of contiguous clusters that are
allocated or unallocated. Instead you provide the number of all
allocated in the whole area even if there are some unallocated clusters
in the middle.

Do you think there's a use case for the sum of the whole area?

Kevin