qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] block: Avoid unecessary drv->bdrv_getlength() c


From: Fam Zheng
Subject: Re: [Qemu-devel] [PATCH] block: Avoid unecessary drv->bdrv_getlength() calls
Date: Mon, 04 Nov 2013 19:25:16 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0


On 11/04/2013 07:18 PM, Kevin Wolf wrote:
Am 04.11.2013 um 08:24 hat Fam Zheng geschrieben:

On Tue 29 Oct 2013 08:12:38 PM CST, Kevin Wolf wrote:
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 <address@hidden>
raw-win32.c probably needs to have a .has_variable_length=true in
bdrv_host_device.  Apart from that,

Reviewed-by: Paolo Bonzini <address@hidden>
Thanks, good catch. I've added this now.
This breaks VMDK because it can't read description file: buffer is
empty in bdrv_probe, when the file size is <512 bytes. (for
raw-posix).
Perhaps we should replace bs->total_sectors with bs->total_byts one
day... For now, how about introducing a bdrv_getlength_bytes() that
always calls the driver and never converts bytes to sectors?
I think it will work. And is it correct to round up total_bytes to BDRV_SECTOR_SIZE before storing to total_sectors? From my view it's better than losing some bytes by rounding down.

Fam




reply via email to

[Prev in Thread] Current Thread [Next in Thread]