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

[Paolo Bonzini]

Il 08/05/2012 15:16, Kevin Wolf ha scritto:
> The commit message is talking about something different. Consider the
> following image, x is allocated, . is unallocated:
> 
> xxx...xxx
> 
> bdrv_is_allocated(offset = 0, length = 9 * cluster_size) can tell you "2
> clusters allocated", for example because after two clusters the L2 table
> ended. I found this pretty confusing and instead expected that qemu-io
> loops until it finds the first different cluster, i.e. the result would
> always be "3 clusters allocated". This is what I think the quoted commit
> (tried to) implement. (Yes, makes the existing code even more embarrassing)

No embarrassment, though I indeed didn't expect it to be broken by you. :)

Ok, I see now.  You would terminate the loop if is_allocated returns
something different from the result of the first call.

BTW I'm going to send the updated patches in a few minutes.  Last minute
testing found another bug (in HMP block_job_set_speed, which aborts).

>> 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.
>
> I think 019 would work with both.

Yes, because what you suggested also will not depend on the
implementation of is_allocated.

I'm inclined to keep code that is closest to what we have (even though
we have it by an odd series of events).  We can add the other one as a
switch later.

Paolo