From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:45787) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SRlAF-0007Ez-SN for qemu-devel@nongnu.org; Tue, 08 May 2012 10:13:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SRlA9-0006u3-K9 for qemu-devel@nongnu.org; Tue, 08 May 2012 10:13:19 -0400 Received: from mail-wg0-f53.google.com ([74.125.82.53]:53066) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SRlA9-0006td-Ab for qemu-devel@nongnu.org; Tue, 08 May 2012 10:13:13 -0400 Received: by wgbfm10 with SMTP id fm10so5657103wgb.10 for ; Tue, 08 May 2012 07:13:11 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <4FA929F4.5050905@redhat.com> Date: Tue, 08 May 2012 16:13:08 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1334232076-19018-1-git-send-email-pbonzini@redhat.com> <1334232076-19018-18-git-send-email-pbonzini@redhat.com> <4FA91842.6040300@redhat.com> <4FA91A48.3050803@redhat.com> <4FA91CA9.8080603@redhat.com> In-Reply-To: <4FA91CA9.8080603@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 17/26] qemu-io: fix the alloc command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com 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