All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com
Subject: Re: [Qemu-devel] [PATCH 17/26] qemu-io: fix the alloc command
Date: Tue, 08 May 2012 16:13:08 +0200	[thread overview]
Message-ID: <4FA929F4.5050905@redhat.com> (raw)
In-Reply-To: <4FA91CA9.8080603@redhat.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

      reply	other threads:[~2012-05-08 14:13 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1334232076-19018-1-git-send-email-pbonzini@redhat.com>
2012-05-04 16:20 ` [Qemu-devel] [PATCH 00/26] My block patches queue for 1.1 Paolo Bonzini
2012-05-07 12:44   ` Kevin Wolf
     [not found] ` <1334232076-19018-18-git-send-email-pbonzini@redhat.com>
2012-05-08 12:57   ` [Qemu-devel] [PATCH 17/26] qemu-io: fix the alloc command Kevin Wolf
2012-05-08 13:06     ` Paolo Bonzini
2012-05-08 13:16       ` Kevin Wolf
2012-05-08 14:13         ` Paolo Bonzini [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FA929F4.5050905@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.