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

[Paolo Bonzini]

Il 08/05/2012 14:57, Kevin Wolf ha scritto:
>> > 
>> > 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.

commit a7824a886ed50eb4fe3c6fcd6afd8814a6973583
Author: Kevin Wolf <address@hidden>
Date:   Mon Jul 20 16:48:43 2009 +0200

    qemu-io: Rework alloc command
    
    The alloc command in qemu-io is mostly useless currently. Instead of doing a
    single call to bdrv_is_allocated, we must call bdrv_is_allocated in a loop
    until we have found out for each requested sector if it is allocated or not
    (bdrv_is_allocated returns a number of sectors that are known to be in the 
same
    state as the first one, but it is not required to include all of them)
    
    This changes the output format of the alloc command so that a change to the
    expected output of qemu-iotests 019 is necessary once this is included.
    
    Signed-off-by: Kevin Wolf <address@hidden>
    Signed-off-by: Anthony Liguori <address@hidden>

I guess this guy knows. :)

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

I think for tests it's more useful, you don't want to depend on the 
implementation
of is_allocated.  That's what I can guess from the above commit message and from
the way 019 uses alloc.

Paolo