From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:57258) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tb6QT-0003Le-Sr for qemu-devel@nongnu.org; Wed, 21 Nov 2012 04:17:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tb6QL-0005la-UF for qemu-devel@nongnu.org; Wed, 21 Nov 2012 04:16:57 -0500 Received: from mx1.redhat.com ([209.132.183.28]:25076) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tb6QL-0005lW-Mf for qemu-devel@nongnu.org; Wed, 21 Nov 2012 04:16:49 -0500 Message-ID: <50AC9B96.9070908@redhat.com> Date: Wed, 21 Nov 2012 10:15:02 +0100 From: Kevin Wolf MIME-Version: 1.0 References: <1353488287-47077-1-git-send-email-borntraeger@de.ibm.com> In-Reply-To: <1353488287-47077-1-git-send-email-borntraeger@de.ibm.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit 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: Christian Borntraeger Cc: jfrei@linux.vnet.ibm.com, Heinz Graalfs , qemu-devel@nongnu.org, Stefan Hajnoczi , agraf@suse.de Am 21.11.2012 09:58, schrieb Christian Borntraeger: > From: Heinz Graalfs > > While testing IPL code (booting) for s390x we faced some problems > with cache=none on dasds (4k block size) on bdrv_preads with length > values != block size. > > This patch makes sure that bdrv_pread and friends work fine with > unaligned access even with cache=none > - propagate alignment value also into bs->file struct > - modify the size in case of no cache to avoid EINVAL on > pread() etc. (file was opened with O_DIRECT). > > This patch seems to cure the problems. > > CC: Kevin Wolf > CC: Stefan Hajnoczi > Signed-off-by: Heinz Graalfs > Signed-off-by: Christian Borntraeger > --- > block.c | 3 +++ > block/raw-posix.c | 6 ++++++ > 2 files changed, 9 insertions(+) > > diff --git a/block.c b/block.c > index 854ebd6..f23c562 100644 > --- a/block.c > +++ b/block.c > @@ -4242,6 +4242,9 @@ BlockDriverAIOCB *bdrv_aio_ioctl(BlockDriverState *bs, > void bdrv_set_buffer_alignment(BlockDriverState *bs, int align) > { > bs->buffer_alignment = align; > + if ((bs->open_flags & BDRV_O_NOCACHE)) { > + bs->file->buffer_alignment = align; > + } Any reason to restrict this to BDRV_O_NOCACHE? There have been patches to change the BDRV_O_NOCACHE flag from the monitor, in which case bdrv_set_buffer_alignment() wouldn't be called anew and O_DIRECT requests start to fail again. > } > > 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. Kevin