From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56609) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XgGfd-00044o-HA for qemu-devel@nongnu.org; Mon, 20 Oct 2014 13:23:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XgGfY-0000Uf-My for qemu-devel@nongnu.org; Mon, 20 Oct 2014 13:23:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:2173) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XgGfY-0000Ub-Dd for qemu-devel@nongnu.org; Mon, 20 Oct 2014 13:22:56 -0400 Date: Mon, 20 Oct 2014 19:22:48 +0200 From: Kevin Wolf Message-ID: <20141020172248.GU3585@noname.redhat.com> References: <1413823167-26619-1-git-send-email-roger.pau@citrix.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1413823167-26619-1-git-send-email-roger.pau@citrix.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] block: char devices on FreeBSD are not behind a pager List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Roger Pau Monne Cc: qemu-devel@nongnu.org, Stefan Hajnoczi Am 20.10.2014 um 18:39 hat Roger Pau Monne geschrieben: > Acknowledge this and forcefully set BDRV_O_NOCACHE and O_DIRECT in orde= r to > force QEMU to use aligned buffers. >=20 > Signed-off-by: Roger Pau Monn=E9 > Cc: Kevin Wolf > Cc: Stefan Hajnoczi > --- > block/raw-posix.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) >=20 > 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, Q= Dict *options, > } > #endif > } > +#ifdef __FreeBSD__ > + if (S_ISCHR(st.st_mode)) { > + /* > + * The file is a char device (disk), which on FreeBSD isn't be= hind > + * a pager, so set BDRV_O_NOCACHE unconditionally. This is nee= ded > + * so Qemu makes sure all IO operations on the device are alig= ned > + * to sector size, or else FreeBSD will reject them with EINVA= L. > + */ > + bs->open_flags |=3D BDRV_O_NOCACHE; > + s->open_flags |=3D 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=3Don (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