Re: [Qemu-devel] [PATCH/RFC] block: Ensure that block size constraints are considered

[Kevin Wolf]

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