qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: asias@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com,
	armbru@redhat.com
Subject: Re: [Qemu-devel] [PATCH] block: Avoid unecessary drv->bdrv_getlength() calls
Date: Tue, 29 Oct 2013 13:12:38 +0100	[thread overview]
Message-ID: <20131029121238.GD4690@dhcp-200-207.str.redhat.com> (raw)
In-Reply-To: <526FA3D4.1060600@redhat.com>

Am 29.10.2013 um 13:02 hat Paolo Bonzini geschrieben:
> Il 29/10/2013 12:35, Kevin Wolf ha scritto:
> > The block layer generally keeps the size of an image cached in
> > bs->total_sectors so that it doesn't have to perform expensive
> > operations to get the size whenever it needs it.
> > 
> > This doesn't work however when using a backend that can change its size
> > without qemu being aware of it, i.e. passthrough of removable media like
> > CD-ROMs or floppy disks. For this reason, the caching is disabled when a
> > removable device is used.
> > 
> > It is obvious that checking whether the _guest_ device has removable
> > media isn't the right thing to do when we want to know whether the size
> > of the host backend can change. To make things worse, non-top-level
> > BlockDriverStates never have any device attached, which makes qemu
> > assume they are removable, so drv->bdrv_getlength() is always called on
> > the protocol layer. In the case of raw-posix, this causes unnecessary
> > lseek() system calls, which turned out to be rather expensive.
> > 
> > This patch completely changes the logic and disables bs->total_sectors
> > caching only for certain block driver types, for which a size change is
> > expected: host_cdrom and host_floppy; also the raw format in case it
> > sits on top of one of these protocols, but in the common case the nested
> > bdrv_getlength() call on the protocol driver will use the cache again
> > and avoid an expensive drv->bdrv_getlength() call.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>

> raw-win32.c probably needs to have a .has_variable_length=true in
> bdrv_host_device.  Apart from that,
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Thanks, good catch. I've added this now.

Kevin

  reply	other threads:[~2013-10-29 12:12 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-29 11:35 [Qemu-devel] [PATCH] block: Avoid unecessary drv->bdrv_getlength() calls Kevin Wolf
2013-10-29 12:02 ` Paolo Bonzini
2013-10-29 12:12   ` Kevin Wolf [this message]
2013-11-04  7:24     ` Fam Zheng
2013-11-04 11:18       ` Kevin Wolf
2013-11-04 11:25         ` Fam Zheng

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=20131029121238.GD4690@dhcp-200-207.str.redhat.com \
    --to=kwolf@redhat.com \
    --cc=armbru@redhat.com \
    --cc=asias@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.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).