El 20/10/14 a les 19.22, Kevin Wolf ha escrit:
> Am 20.10.2014 um 18:39 hat Roger Pau Monne geschrieben:
>> Acknowledge this and forcefully set BDRV_O_NOCACHE and O_DIRECT in order to
>> force QEMU to use aligned buffers.
>>
>> Signed-off-by: Roger Pau Monné <roger....@citrix.com>
>> Cc: Kevin Wolf <kw...@redhat.com>
>> Cc: Stefan Hajnoczi <stefa...@redhat.com>
>> ---
>> block/raw-posix.c | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/block/raw-posix.c b/block/raw-posix.c
>> index 86ce4f2..63841dd 100644
>> --- a/block/raw-posix.c
>> +++ b/block/raw-posix.c
>> @@ -472,6 +472,18 @@ static int raw_open_common(BlockDriverState *bs, QDict
>> *options,
>> }
>> #endif
>> }
>> +#ifdef __FreeBSD__
>> + if (S_ISCHR(st.st_mode)) {
>> + /*
>> + * The file is a char device (disk), which on FreeBSD isn't behind
>> + * a pager, so set BDRV_O_NOCACHE unconditionally. This is needed
>> + * so Qemu makes sure all IO operations on the device are aligned
>> + * to sector size, or else FreeBSD will reject them with EINVAL.
>> + */
>> + bs->open_flags |= BDRV_O_NOCACHE;
>> + s->open_flags |= O_DIRECT;
>> + }
>> +#endif
>
> No, this doesn't look right. Block drivers must not modify the options
> that they get. (Yes, the Linux AIO case is broken in this respect.
> Hopefully we'll be able to fix it soon.)
>
> Depending on what the real requirements are, I can see two options:
>
> 1. Require cache.direct=on (i.e. O_DIRECT) for char devices on FreeBSD.
> If the user didn't set the option, print a nice error message telling
> them what option to set.
>
> 2. If O_DIRECT isn't actually required to open the file, but you only
> need to make sure to use the right alignment, modify
> raw_probe_alignment() so that it returns an alignment > 1 even for
> non-O_DIRECT files on FreeBSD if they are character devices.
>
> I don't know FreeBSD good enough, but if it fulfills the requirements,
> option 2 is certainly the more elegant one.
Thanks for the review. O_DIRECT is not required to open the file, so
option 2 seems sensible.
I've added a new flag to BDRVRawState that's used to check if underlying
device needs requests to be aligned. This flag is set by default if
BDRV_O_NOCACHE is used, or if the OS is FreeBSD and the underlying
device is a char dev. This new flag is used as a replacement of the
O_DIRECT and BDRV_O_NOCACHE checks that were used in raw_probe_alignment
and raw_aio_submit. Does this sound OK?
Roger.
