From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36727) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VdIHh-0001g3-VI for qemu-devel@nongnu.org; Mon, 04 Nov 2013 06:25:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VdIHa-0002eM-93 for qemu-devel@nongnu.org; Mon, 04 Nov 2013 06:25:29 -0500 Received: from mx1.redhat.com ([209.132.183.28]:37463) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VdIHZ-0002eH-VT for qemu-devel@nongnu.org; Mon, 04 Nov 2013 06:25:22 -0500 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rA4BPL29005136 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 4 Nov 2013 06:25:21 -0500 Message-ID: <5277841C.80901@redhat.com> Date: Mon, 04 Nov 2013 19:25:16 +0800 From: Fam Zheng MIME-Version: 1.0 References: <1383046524-17801-1-git-send-email-kwolf@redhat.com> <526FA3D4.1060600@redhat.com> <20131029121238.GD4690@dhcp-200-207.str.redhat.com> <52774BA9.4080302@redhat.com> <20131104111822.GE4199@dhcp-200-207.str.redhat.com> In-Reply-To: <20131104111822.GE4199@dhcp-200-207.str.redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] block: Avoid unecessary drv->bdrv_getlength() calls List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Paolo Bonzini , asias@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, armbru@redhat.com 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 >>>> raw-win32.c probably needs to have a .has_variable_length=true in >>>> bdrv_host_device. Apart from that, >>>> >>>> Reviewed-by: Paolo Bonzini >>> 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