From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LWFT8-00082i-5q for qemu-devel@nongnu.org; Sun, 08 Feb 2009 14:37:30 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LWFT5-00081E-NG for qemu-devel@nongnu.org; Sun, 08 Feb 2009 14:37:28 -0500 Received: from [199.232.76.173] (port=32979 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LWFT5-00081B-Ig for qemu-devel@nongnu.org; Sun, 08 Feb 2009 14:37:27 -0500 Received: from mx2.redhat.com ([66.187.237.31]:39365) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LWFT5-0007yY-1r for qemu-devel@nongnu.org; Sun, 08 Feb 2009 14:37:27 -0500 Message-ID: <498F348D.3040309@redhat.com> Date: Sun, 08 Feb 2009 21:37:49 +0200 From: Avi Kivity MIME-Version: 1.0 References: <1234115947-31622-1-git-send-email-avi@redhat.com> <1234115947-31622-4-git-send-email-avi@redhat.com> <498F2E1E.8030605@codemonkey.ws> In-Reply-To: <498F2E1E.8030605@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 3/3] Drop internal bdrv_pread()/bdrv_pwrite() APIs Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: qemu-devel@nongnu.org Anthony Liguori wrote: > Avi Kivity wrote: >> Now that scsi generic no longer uses bdrv_pread() and bdrv_pwrite(), >> we can >> drop the corresponding internal APIs, which overlap >> bdrv_read()/bdrv_write() >> and, being byte oriented, are unnatural for a block device. >> >> Signed-off-by: Avi Kivity >> >> int bdrv_truncate(BlockDriverState *bs, int64_t offset) >> diff --git a/block_int.h b/block_int.h >> index e4630f0..cc9966b 100644 >> --- a/block_int.h >> +++ b/block_int.h >> @@ -58,10 +58,6 @@ struct BlockDriver { >> int aiocb_size; >> >> const char *protocol_name; >> - int (*bdrv_pread)(BlockDriverState *bs, int64_t offset, >> - uint8_t *buf, int count); >> - int (*bdrv_pwrite)(BlockDriverState *bs, int64_t offset, >> - const uint8_t *buf, int count); >> > > $ grep -l bdrv_pwrite *.c hw/*.c > block.c > block-qcow2.c > block-qcow.c > block-raw-posix.c > block-raw-win32.c > block-vmdk.c > block-vpc.c > savevm.c > hw/scsi-generic.c > > So there's a lot of users other than scsi-generic. Usually, these > callers are in the block layer to read/write metadata that isn't > always block aligned. Some buffer adjustment could fix savevm.c to > ensure alignment. It's more accurate to say that there are now no users that depend on the request size. The other users will happily allow expansion of a small request to a sector. > These users are now relegated to using emulated pread/pwrite? Won't > that have a noticable impact on performance if updating small bits of > metadata. For instance, I think updating qcow2 refcounts would look > bad since you have to read/write the full block to update 4 bytes of > data. Granted it'll be cached, but... I haven't measured but I'll bet the impact is unnoticable. We're doubling the syscall count for writes, but the actual transfer (if cached) or the I/O (if uncached) will swamp that. For reads, we're copying a bit more data, but that won't even tickle performance. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.