qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	Deepak C Shetty <deepakcs@linux.vnet.ibm.com>,
	qemu-devel@nongnu.org, Markus Armbruster <armbru@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2 2/2] osdep: warn if opening a file O_DIRECT on tmpfs fails
Date: Wed, 21 Aug 2013 13:04:01 -0600	[thread overview]
Message-ID: <52150F21.7030305@redhat.com> (raw)
In-Reply-To: <1377090999-18217-3-git-send-email-stefanha@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1726 bytes --]

On 08/21/2013 07:16 AM, Stefan Hajnoczi wrote:
> Print a warning when opening a file O_DIRECT on tmpfs fails.  This saves
> users a lot of time trying to figure out the EINVAL error.
> 
> Daniel P. Berrange <berrange@redhat.com> suggested opening the file
> without O_DIRECT as a portable way to check whether the file system
> supports O_DIRECT.  That gets messy when flags contains O_CREAT since
> we'd create a file but return an error - or a race condition if we try
> to unlink the file.  It's simpler to check the file system type.

An alternative to this is as follows:

if the user passes in O_CREAT|O_DIRECT (but not O_EXCL), we _first_ try
O_CREAT|O_DIRECT|O_EXCL.  If that succeeds, O_DIRECT works and we
created the file, nothing further to do.  If it fails with EINVAL,
O_DIRECT is unsupported.  If it fails with EEXIST, the file already
exists, and we retry open(O_DIRECT) (no O_CREAT, because the file
already exists), and get our answer that way.  (It would be possible to
audit whether the kernel favors EINVAL vs. EEXIST in the case where both
errors are possible [ie. the file exists and O_DIRECT is unsupported],
to slightly optimize this code; but I'm not sure it's worth it).  If the
user passes in O_DIRECT but not O_CREAT, then you jump straight to the
middle case (since they didn't want creation in the first place).

That way, you can detect O_DIRECT failure on more than just tmpfs, and
without needing an #if __linux__, and with no nasty races in unlinking a
just-created file.  I'm starting to think Dan's suggestion has merit
after all, if done correctly.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

  parent reply	other threads:[~2013-08-21 19:05 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-21 13:16 [Qemu-devel] [PATCH v2 0/2] osdep: warn if opening a file O_DIRECT on tmpfs fails Stefan Hajnoczi
2013-08-21 13:16 ` [Qemu-devel] [PATCH v2 1/2] libcacard: link against qemu-error.o for error_report() Stefan Hajnoczi
2013-08-21 13:16 ` [Qemu-devel] [PATCH v2 2/2] osdep: warn if opening a file O_DIRECT on tmpfs fails Stefan Hajnoczi
2013-08-21 14:08   ` Markus Armbruster
2013-08-21 19:48     ` Doug Goldstein
2013-08-21 15:13   ` Eric Blake
2013-08-21 19:04   ` Eric Blake [this message]
2013-08-21 14:09 ` [Qemu-devel] [PATCH v2 0/2] " Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52150F21.7030305@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=deepakcs@linux.vnet.ibm.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).