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
