qemu-devel.nongnu.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).