[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH/RFC] block: Ensure that block size constraints a
From: |
Heinz Graalfs |
Subject: |
Re: [Qemu-devel] [PATCH/RFC] block: Ensure that block size constraints are considered |
Date: |
Tue, 11 Dec 2012 10:58:12 +0100 |
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
>