Re: [Qemu-devel] [PATCH] block: char devices on FreeBSD are not behind a pager

[Kevin Wolf <kwolf@redhat.com>]

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.pau@citrix.com>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@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.

Kevin