From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53103) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VCZAy-0001vX-F2 for qemu-devel@nongnu.org; Thu, 22 Aug 2013 14:00:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VCZAt-0004Rv-Hy for qemu-devel@nongnu.org; Thu, 22 Aug 2013 14:00:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59400) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VCZAt-0004Rm-9s for qemu-devel@nongnu.org; Thu, 22 Aug 2013 13:59:59 -0400 Message-ID: <52165124.30101@redhat.com> Date: Thu, 22 Aug 2013 11:57:56 -0600 From: Eric Blake MIME-Version: 1.0 References: <1377163743-25029-1-git-send-email-stefanha@redhat.com> <1377163743-25029-3-git-send-email-stefanha@redhat.com> In-Reply-To: <1377163743-25029-3-git-send-email-stefanha@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="c5jBfpVLiOvQ9hXvRMl6VHDCW7KjeRm1g" Subject: Re: [Qemu-devel] [PATCH 2/2] osdep: warn if open(O_DIRECT) on fails with EINVAL List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , Deepak C Shetty , qemu-devel@nongnu.org, Markus Armbruster This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --c5jBfpVLiOvQ9hXvRMl6VHDCW7KjeRm1g Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 08/22/2013 03:29 AM, Stefan Hajnoczi wrote: > Print a warning when opening a file O_DIRECT fails with EINVAL. This > saves users a lot of time trying to figure out the EINVAL error, which > is typical when attempting to open a file O_DIRECT on Linux tmpfs. >=20 > Reported-by: Deepak C Shetty > Signed-off-by: Stefan Hajnoczi > --- > util/osdep.c | 7 +++++++ > 1 file changed, 7 insertions(+) >=20 > diff --git a/util/osdep.c b/util/osdep.c > index 685c8ae..62072b4 100644 > --- a/util/osdep.c > +++ b/util/osdep.c > @@ -207,6 +207,13 @@ int qemu_open(const char *name, int flags, ...) > } > #endif > =20 > +#ifdef O_DIRECT On some other projects I've worked with (hello gnulib!), the compatibility headers do: #define O_DIRECT 0 so that the rest of the code can just blindly use open(...,|O_DIRECT) (provided, of course, that not having O_DIRECT semantics is not fatal...). If that is done, then this #ifdef will always be true... > + if (ret =3D=3D -1 && errno =3D=3D EINVAL && (flags & O_DIRECT)) { =2E..but the '(flags & O_DIRECT)' subclause would be false, so that's saf= e. If O_DIRECT were always defined, then '#if O_DIRECT', or better yet, 'if (O_DIRECT)', is a better way to conditionalize code that depends on it being a useful value (no false positives when defined to 0, and by moving the check from preprocessor to C, you get benefits of compiler verification of type safety to avoid bitrot in what is otherwise dead cod= e). Grepping through qemu, I see that block/raw-posix.c has some weird #ifdef'ery: /* OS X does not have O_DSYNC */ #ifndef O_DSYNC #ifdef O_SYNC #define O_DSYNC O_SYNC #elif defined(O_FSYNC) #define O_DSYNC O_FSYNC #endif #endif /* Approximate O_DIRECT with O_DSYNC if O_DIRECT isn't available */ #ifndef O_DIRECT #define O_DIRECT O_DSYNC #endif which strikes me as odd (O_DSYNC and O_DIRECT have orthogonal, and somewhat opposing goals - the former says don't return from write() until data is on disk, which slows things down for safety; the latter says "I'm a special user space, get out of my way, by not caching anything, and leaving the flushing to me", to speed things up if the user knows what they are doing - then again, the Linux man page hints that they are often used together when worrying about guarantees for synchronous I/O). But enough of that side diversion - that one #define of O_DIRECT is not related to the file you are touching. I like the elegance of your patch (nicer than the race in my double-open attempt). Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --c5jBfpVLiOvQ9hXvRMl6VHDCW7KjeRm1g Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.14 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJSFlEkAAoJEKeha0olJ0NqC2MH/2XRorjTkgj44bFkzeKGfLjN E+uoJWhWTQSRlxt4inC40bCeEjVVz4FHbyIGqD39Cr3NUyXgWaN0WjodRCKcknUm Se4+tRv/8a1JDEXxtDW3M4ZumFjCNJ8PeG0bBbrHDhNPLYTW74vHIuYapdbUBiPM avOFjSRV0VzwShnr4Hc0os3+myF/+aaFv3IsHwiy9HMfvfQI0WddePtLerLPbGTU VsITpgjBCjeWP5YGcglY051XhDka6ykMEfkJ1bcelHsN1kzO5a2I2mnJGep3gmI3 SS8HdoKDt4YekYigSe6pLSpFVIhHsapOlKVNckK9eTHjP/qX0KUvjAk3aGRbkEg= =6Jhy -----END PGP SIGNATURE----- --c5jBfpVLiOvQ9hXvRMl6VHDCW7KjeRm1g--