From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47752) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TiMbU-00042F-U9 for qemu-devel@nongnu.org; Tue, 11 Dec 2012 04:58:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TiMbO-0007RK-St for qemu-devel@nongnu.org; Tue, 11 Dec 2012 04:58:20 -0500 Received: from e9.ny.us.ibm.com ([32.97.182.139]:44040) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TiMbO-0007RG-OS for qemu-devel@nongnu.org; Tue, 11 Dec 2012 04:58:14 -0500 Received: from /spool/local by e9.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 11 Dec 2012 04:58:13 -0500 Received: from d01relay04.pok.ibm.com (d01relay04.pok.ibm.com [9.56.227.236]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id 4979438C8041 for ; Tue, 11 Dec 2012 04:58:12 -0500 (EST) Received: from d01av03.pok.ibm.com (d01av03.pok.ibm.com [9.56.224.217]) by d01relay04.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id qBB9wBQq321404 for ; Tue, 11 Dec 2012 04:58:11 -0500 Received: from d01av03.pok.ibm.com (loopback [127.0.0.1]) by d01av03.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id qBB9wAFJ002588 for ; Tue, 11 Dec 2012 07:58:11 -0200 Message-ID: <1355219892.4245.14.camel@br8hfpp0.de.ibm.com> From: Heinz Graalfs Date: Tue, 11 Dec 2012 10:58:12 +0100 In-Reply-To: <50C5A37F.9020300@redhat.com> References: <1353488287-47077-1-git-send-email-borntraeger@de.ibm.com> <50AC9B96.9070908@redhat.com> <1354911963.3635.4.camel@br8hfpp0.de.ibm.com> <50C5A37F.9020300@redhat.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Mime-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH/RFC] block: Ensure that block size constraints are considered List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Christian Borntraeger , jfrei@linux.vnet.ibm.com, qemu-devel@nongnu.org, Stefan Hajnoczi , agraf@suse.de Hi Kevin, I'm using the bdrv_pread() function during boot partition detection ... In detail: bdrv_pread() is called to read 32 bytes from a 2048 bytes formatted disk. This results in setting up a read of 512 bytes (1 sector multiplied by 512 current code in paio_submit()), which is wrong for a O_DIRECT opened file, and produces the error. Heinz On Mon, 2012-12-10 at 09:55 +0100, Kevin Wolf wrote: > Am 07.12.2012 21:26, schrieb Heinz Graalfs: > > Hello Kevin, > > > > I'm resending my answer as of Nov 23rd. > > > > Is this still on your queue? > > No, it wasn't. I guess I was waiting for a new version of the patch. > > >>> } > >>> > >>> void *qemu_blockalign(BlockDriverState *bs, size_t size) > >>> diff --git a/block/raw-posix.c b/block/raw-posix.c > >>> index f2f0404..baebf1d 100644 > >>> --- a/block/raw-posix.c > >>> +++ b/block/raw-posix.c > >>> @@ -700,6 +700,12 @@ static BlockDriverAIOCB *paio_submit(BlockDriverState *bs, int fd, > >>> acb->aio_nbytes = nb_sectors * 512; > >>> acb->aio_offset = sector_num * 512; > >>> > >>> + /* O_DIRECT also requires an aligned length */ > >>> + if (bs->open_flags & BDRV_O_NOCACHE) { > >>> + acb->aio_nbytes += acb->bs->buffer_alignment - 1; > >>> + acb->aio_nbytes &= ~(acb->bs->buffer_alignment - 1); > >>> + } > >> > >> Modifying aio_nbytes, but not the iov looks wrong to me. This may work > >> in the handle_aiocb_rw_linear() code path, but not with actual vectored I/O. > > > > Current coding ensures that read IO buffers always seem to be aligned > > correctly. Whereas read length values are not always appropriate for an > > O_DIRECT scenario. > > > > For a 2048 formatted disk I verified that > > > > 1. non vectored IO - the length needs to be adapted several times, > > which is accomplished now by the patch. > > > > 2. vectored IO - the qiov's total length is always a multiple of the > > logical block size > > (which is also verified in virtio_blk_handle_read()) > > The particular iov length fields are already correctly setup as a > > multiple of the logical block size when processed in > > virtio_blk_handle_request(). > > I must admit that I don't quite understand this. As far as I know, > virtio-blk doesn't make any difference between requests with niov = 1 > and real vectored requests. So how can the length of the latter always > be right, whereas the length of the former may be wrong? > > The other point is that requests may not even be coming from virtio-blk. > They could be made by other device emulations or they could come from a > block job. (They also could be the result of a merge in the block layer, > though if the original requests were aligned, the result will stay aligned) > > Kevin >