[CIFS] Avoid open on possible directories since Samba now rejects them

[Steve French]

The small fix below has passed testing (connectathon, dbench, and
some freelance tests so far, now running others) to at least three
server types.   Jeff has also tested it.  Fix now in the cifs-2.6.git
tree.   This or equivalent is a must fix for 2.6.30 to handle the
change to Samba that jra just made.


Small change (mostly formatting) to limit lookup based open calls to
file create only.

After discussion yesteday on samba-technical about the posix lookup
regression,  and looking at a problem with cifs posix open to one
particular Samba version, Jeff and JRA realized that Samba server's
behavior changed in this area (posix open behavior on files vs.
directories).   To make this behavior consistent, JRA just made a
fix to Samba server to alter how it handles open of directories (now
returning the equivalent of EISDIR instead of success). Since we don't
know at lookup time whether the inode is a directory or file (and
thus whether posix open will succeed with most current Samba server),
this change avoids the posix open code on lookup open (just issues
posix open on creates).    This gets the semantic benefits we want
(atomicity, posix byte range locks, improved write semantics on newly
created files) and file create still is fast, and we avoid the problem
that Jeff noticed yesterday with "openat" (and some open directory
calls) of non-cached directories to one version of Samba server, and
will work with future Samba versions (which include the fix jra just
pushed into Samba server).  I confirmed this approach with jra
yesterday and with Shirish today.

Posix open is only called (at lookup time) for file create now.
For opens (rather than creates), because we do not know if it
is a file or directory yet, and current Samba no longer allows
us to do posix open on dirs, we could end up wasting an open call
on what turns out to be a dir. For file opens, we wait to call posix
open till cifs_open.  It could be added here (lookup) in the future
but the performance tradeoff of the extra network request when EISDIR
or EACCES is returned would have to be weighed against the 50%
reduction in network traffic in the other paths.

    Reviewed-by: Shirish Pargaonkar <shirishp@xxxxxxxxxx>
    Tested-by: Jeff Layton <jlayton@xxxxxxxxxx>
    CC: Jeremy Allison <jra@xxxxxxxxx>
    Signed-off-by: Steve French <sfrench@xxxxxxxxxx>

diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index f49d684..3758965 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -657,31 +657,36 @@ cifs_lookup(struct inode *parent_dir_inode,
struct dentry *direntry,
 	}
 	cFYI(1, ("Full path: %s inode = 0x%p", full_path, direntry->d_inode));

+	/* Posix open is only called (at lookup time) for file create now.
+	 * For opens (rather than creates), because we do not know if it
+	 * is a file or directory yet, and current Samba no longer allows
+	 * us to do posix open on dirs, we could end up wasting an open call
+	 * on what turns out to be a dir. For file opens, we wait to call posix
+	 * open till cifs_open.  It could be added here (lookup) in the future
+	 * but the performance tradeoff of the extra network request when EISDIR
+	 * or EACCES is returned would have to be weighed against the 50%
+	 * reduction in network traffic in the other paths.
+	 */
 	if (pTcon->unix_ext) {
 		if (!(nd->flags & (LOOKUP_PARENT | LOOKUP_DIRECTORY)) &&
-		     (nd->flags & LOOKUP_OPEN) && !pTcon->broken_posix_open) {
-			if (!((nd->intent.open.flags & O_CREAT) &&
-					(nd->intent.open.flags & O_EXCL))) {
-				rc = cifs_posix_open(full_path, &newInode,
+		     (nd->flags & LOOKUP_OPEN) && !pTcon->broken_posix_open &&
+		     (nd->intent.open.flags & O_CREAT)) {
+			rc = cifs_posix_open(full_path, &newInode,
 					parent_dir_inode->i_sb,
 					nd->intent.open.create_mode,
 					nd->intent.open.flags, &oplock,
 					&fileHandle, xid);
-				/*
-				 * This code works around a bug in
-				 * samba posix open in samba versions 3.3.1
-				 * and earlier where create works
-				 * but open fails with invalid parameter.
-				 * If either of these error codes are
-				 * returned, follow the normal lookup.
-				 * Otherwise, the error during posix open
-				 * is handled.
-				 */
-				if ((rc != -EINVAL) && (rc != -EOPNOTSUPP))
-					posix_open = true;
-				else
-					pTcon->broken_posix_open = true;
-			}
+			/*
+			 * The check below works around a bug in POSIX
+			 * open in samba versions 3.3.1 and earlier where
+			 * open could incorrectly fail with invalid parameter.
+			 * If either that or op not supported returned, follow
+			 * the normal lookup.
+			 */
+			if ((rc == 0) || (rc == -ENOENT))
+				posix_open = true;
+			else if ((rc == -EINVAL) || (rc != -EOPNOTSUPP))
+				pTcon->broken_posix_open = true;
 		}
 		if (!posix_open)
 			rc = cifs_get_inode_info_unix(&newInode, full_path,

-- 
Thanks,

Steve
commit 8db14ca12569fe885694bd3d5ff84c2d973d3cb0
Author: Steve French <sfrench@xxxxxxxxxx>
Date:   Sat May 23 18:57:25 2009 +0000

    [CIFS] Avoid open on possible directories since Samba now rejects them
    
    Small change (mostly formatting) to limit lookup based open calls to
    file create only.
    
    After discussion yesteday on samba-technical about the posix lookup
    regression,  and looking at a problem with cifs posix open to one
    particular Samba version, Jeff and JRA realized that Samba server's
    behavior changed in this area (posix open behavior on files vs.
    directories).   To make this behavior consistent, JRA just made a
    fix to Samba server to alter how it handles open of directories (now
    returning the equivalent of EISDIR instead of success). Since we don't
    know at lookup time whether the inode is a directory or file (and
    thus whether posix open will succeed with most current Samba server),
    this change avoids the posix open code on lookup open (just issues
    posix open on creates).    This gets the semantic benefits we want
    (atomicity, posix byte range locks, improved write semantics on newly
    created files) and file create still is fast, and we avoid the problem
    that Jeff noticed yesterday with "openat" (and some open directory
    calls) of non-cached directories to one version of Samba server, and
    will work with future Samba versions (which include the fix jra just
    pushed into Samba server).  I confirmed this approach with jra
    yesterday and with Shirish today.
    
    Posix open is only called (at lookup time) for file create now.
    For opens (rather than creates), because we do not know if it
    is a file or directory yet, and current Samba no longer allows
    us to do posix open on dirs, we could end up wasting an open call
    on what turns out to be a dir. For file opens, we wait to call posix
    open till cifs_open.  It could be added here (lookup) in the future
    but the performance tradeoff of the extra network request when EISDIR
    or EACCES is returned would have to be weighed against the 50%
    reduction in network traffic in the other paths.
    
    Reviewed-by: Shirish Pargaonkar <shirishp@xxxxxxxxxx>
    Tested-by: Jeff Layton <jlayton@xxxxxxxxxx>
    CC: Jeremy Allison <jra@xxxxxxxxx>
    Signed-off-by: Steve French <sfrench@xxxxxxxxxx>

diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index f49d684..3758965 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -657,31 +657,36 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
 	}
 	cFYI(1, ("Full path: %s inode = 0x%p", full_path, direntry->d_inode));
 
+	/* Posix open is only called (at lookup time) for file create now.
+	 * For opens (rather than creates), because we do not know if it
+	 * is a file or directory yet, and current Samba no longer allows
+	 * us to do posix open on dirs, we could end up wasting an open call
+	 * on what turns out to be a dir. For file opens, we wait to call posix
+	 * open till cifs_open.  It could be added here (lookup) in the future
+	 * but the performance tradeoff of the extra network request when EISDIR
+	 * or EACCES is returned would have to be weighed against the 50%
+	 * reduction in network traffic in the other paths.
+	 */
 	if (pTcon->unix_ext) {
 		if (!(nd->flags & (LOOKUP_PARENT | LOOKUP_DIRECTORY)) &&
-		     (nd->flags & LOOKUP_OPEN) && !pTcon->broken_posix_open) {
-			if (!((nd->intent.open.flags & O_CREAT) &&
-					(nd->intent.open.flags & O_EXCL))) {
-				rc = cifs_posix_open(full_path, &newInode,
+		     (nd->flags & LOOKUP_OPEN) && !pTcon->broken_posix_open &&
+		     (nd->intent.open.flags & O_CREAT)) {
+			rc = cifs_posix_open(full_path, &newInode,
 					parent_dir_inode->i_sb,
 					nd->intent.open.create_mode,
 					nd->intent.open.flags, &oplock,
 					&fileHandle, xid);
-				/*
-				 * This code works around a bug in
-				 * samba posix open in samba versions 3.3.1
-				 * and earlier where create works
-				 * but open fails with invalid parameter.
-				 * If either of these error codes are
-				 * returned, follow the normal lookup.
-				 * Otherwise, the error during posix open
-				 * is handled.
-				 */
-				if ((rc != -EINVAL) && (rc != -EOPNOTSUPP))
-					posix_open = true;
-				else
-					pTcon->broken_posix_open = true;
-			}
+			/*
+			 * The check below works around a bug in POSIX
+			 * open in samba versions 3.3.1 and earlier where
+			 * open could incorrectly fail with invalid parameter.
+			 * If either that or op not supported returned, follow
+			 * the normal lookup.
+			 */
+			if ((rc == 0) || (rc == -ENOENT))
+				posix_open = true;
+			else if ((rc == -EINVAL) || (rc != -EOPNOTSUPP))
+				pTcon->broken_posix_open = true;
 		}
 		if (!posix_open)
 			rc = cifs_get_inode_info_unix(&newInode, full_path,