diff mbox

[v2,2/2] fuse: permit O_DIRECT flag in open()

Message ID 1311764179-20326-2-git-send-email-avati@gluster.com (mailing list archive)
State New, archived
Headers show

Commit Message

Anand Avati July 27, 2011, 10:56 a.m. UTC
FUSE currently disallows O_DIRECT flag in open(). It is tricky dealing with
setting/unsetting of O_DIRECT flag on an open file. There are applications
(primarily VMs and databases) which open files with O_DIRECT flag. These
applications do not work on FUSE due to this limitation.

The approach with this patch is to permit opens with O_DIRECT, but instead
disable setting/unsetting of the O_DIRECT flag no matter how the file was
opened. This limitation is for more practical than disallowing O_DIRECT
altogether.

Signed-off-by: Anand Avati <avati@gluster.com>
---
 fs/fuse/file.c |   36 ++++++++++++++++++++++++++++++++----
 1 files changed, 32 insertions(+), 4 deletions(-)

Comments

Christoph Hellwig July 27, 2011, 9:05 p.m. UTC | #1
On Wed, Jul 27, 2011 at 03:56:19AM -0700, Anand Avati wrote:
> FUSE currently disallows O_DIRECT flag in open(). It is tricky dealing with
> setting/unsetting of O_DIRECT flag on an open file. There are applications
> (primarily VMs and databases) which open files with O_DIRECT flag. These
> applications do not work on FUSE due to this limitation.
> 
> The approach with this patch is to permit opens with O_DIRECT, but instead
> disable setting/unsetting of the O_DIRECT flag no matter how the file was
> opened. This limitation is for more practical than disallowing O_DIRECT
> altogether.

But it's also entirely incorrect.  Fix your userspace to have a proper
fallback if O_DIRECT opens fail.  It's a feature only supported by a few
filesystems, and very few operating systems.

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 82a6646..355c30f 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -154,6 +154,27 @@  int fuse_do_open(struct fuse_conn *fc, u64 nodeid, struct file *file,
 		return err;
 	}
 
+	if (file->f_flags & O_DIRECT) {
+		if (!(outarg.open_flags & FOPEN_DIRECT_IO)) {
+			/* filesystem process did not acknowledge the O_DIRECT
+			   flag with FOPEN_DIRECT_IO bit in ourarg. Hence
+			   we fail the open().
+
+			   Notify the filesystem process about the failed
+			   open with fuse_sync_release right here itself as
+			   file->private_data is not yet set up for release
+			   notification.
+			*/
+			fuse_sync_release(ff, file->f_flags);
+			return -EINVAL;
+		}
+
+		/* make VFS believe we don't support O_DIRECT till we
+		   implement a_ops->direct_IO
+		*/
+		file->f_flags &= ~O_DIRECT;
+	}
+
 	if (isdir)
 		outarg.open_flags &= ~FOPEN_DIRECT_IO;
 
@@ -193,10 +214,6 @@  int fuse_open_common(struct inode *inode, struct file *file, bool isdir)
 	struct fuse_conn *fc = get_fuse_conn(inode);
 	int err;
 
-	/* VFS checks this, but only _after_ ->open() */
-	if (file->f_flags & O_DIRECT)
-		return -EINVAL;
-
 	err = generic_file_open(inode, file);
 	if (err)
 		return err;
@@ -2132,6 +2149,15 @@  int fuse_notify_poll_wakeup(struct fuse_conn *fc,
 	return 0;
 }
 
+
+static int fuse_check_flags(struct file *filp, int flags)
+{
+	if ((filp->f_flags ^ flags) & O_DIRECT)
+		return -EINVAL;
+	return 0;
+}
+
+
 static const struct file_operations fuse_file_operations = {
 	.llseek		= fuse_file_llseek,
 	.read		= do_sync_read,
@@ -2149,6 +2175,7 @@  static const struct file_operations fuse_file_operations = {
 	.unlocked_ioctl	= fuse_file_ioctl,
 	.compat_ioctl	= fuse_file_compat_ioctl,
 	.poll		= fuse_file_poll,
+	.check_flags	= fuse_check_flags,
 };
 
 static const struct file_operations fuse_direct_io_file_operations = {
@@ -2165,6 +2192,7 @@  static const struct file_operations fuse_direct_io_file_operations = {
 	.unlocked_ioctl	= fuse_file_ioctl,
 	.compat_ioctl	= fuse_file_compat_ioctl,
 	.poll		= fuse_file_poll,
+	.check_flags	= fuse_check_flags,
 	/* no splice_read */
 };