From: shadow <shadow>
Date: Thu, 30 Jul 2009 10:06:14 +0000 (+0000)
Subject: checking permission should be part of 'open' part of mds_open, not 'lookup'
X-Git-Tag: v1_8_2_01~1^2~196
X-Git-Url: https://git.whamcloud.com/?a=commitdiff_plain;h=0fbe658cae75356c2942d98c59c46a39d662239f;p=fs%2Flustre-release.git

checking permission should be part of 'open' part of mds_open, not 'lookup'
part, so server should be set DISP_OPEN_OPEN disposition before perform
permission check.
Also don not need call revalidate dentry if client already have LOOKUP lock.

Branch b1_8
b=17545
i=green
i=panda
---

diff --git a/lustre/ChangeLog b/lustre/ChangeLog
index b0224f2..1247bec 100644
--- a/lustre/ChangeLog
+++ b/lustre/ChangeLog
@@ -15,6 +15,14 @@ tbd Sun Microsystems, Inc.
 	  more information, please refer to bugzilla 17630.
 
 Severity   : normal
+Bugzilla   : 17545
+Description: open sometimes returns ENOENT instead of EACCES
+Details    : checking permission should be part of open part of mds_open, not
+             lookup part. so server should be set DISP_OPEN_OPEN disposition
+             before starting permission check.
+             Also not need revalidate dentry if client already have LOOKUP lock.
+
+Severity   : normal
 Bugzilla   : 19854
 Description: enable client interface failover
 Details    : When a child reconnects from another NID, properly update export
diff --git a/lustre/llite/dcache.c b/lustre/llite/dcache.c
index 9b2fe52..1d61bc5 100644
--- a/lustre/llite/dcache.c
+++ b/lustre/llite/dcache.c
@@ -369,6 +369,9 @@ int ll_revalidate_it(struct dentry *de, int lookup_flags,
                 GOTO(out_sa, rc);
         }
 
+        if ((de->d_flags & DCACHE_LUSTRE_INVALID) == 0)
+                GOTO(out_sa, rc = 1);
+
         exp = ll_i2mdcexp(de->d_inode);
 
         /* Never execute intents for mount points.
diff --git a/lustre/mds/mds_open.c b/lustre/mds/mds_open.c
index 44f8d7e..ece1672 100644
--- a/lustre/mds/mds_open.c
+++ b/lustre/mds/mds_open.c
@@ -848,7 +848,6 @@ static int mds_finish_open(struct ptlrpc_request *req, struct dentry *dchild,
         if ((rc = mds_lov_prepare_objids(obd,lmm)) != 0)
                 RETURN(rc);
 
-        ldlm_reply_set_disposition(rep, DISP_OPEN_OPEN);
         mfd = mds_dentry_open(dchild, mds->mds_vfsmnt, flags, req);
         if (IS_ERR(mfd))
                 RETURN(PTR_ERR(mfd));
@@ -878,6 +877,7 @@ static int mds_open_by_fid(struct ptlrpc_request *req, struct ll_fid *fid,
         struct inode *inodes[PTLRPC_NUM_VERSIONS] = { NULL };
         ENTRY;
 
+        ldlm_reply_set_disposition(rep, DISP_LOOKUP_EXECD);
         fidlen = ll_fid2str(fidname, fid->id, fid->generation);
         dchild = mds_lookup(obd, fidname, mds->mds_pending_dir, fidlen);
         if (IS_ERR(dchild)) {
@@ -929,8 +929,8 @@ static int mds_open_by_fid(struct ptlrpc_request *req, struct ll_fid *fid,
         }
 
         mds_pack_inode2body(body, dchild->d_inode);
-        ldlm_reply_set_disposition(rep, DISP_LOOKUP_EXECD);
         ldlm_reply_set_disposition(rep, DISP_LOOKUP_POS);
+        ldlm_reply_set_disposition(rep, DISP_OPEN_OPEN);
 
         rc = mds_finish_open(req, dchild, body, flags, &handle, rec, rep, NULL);
         rc = mds_finish_transno(mds, inodes, handle,
@@ -1293,7 +1293,19 @@ int mds_open(struct mds_update_record *rec, int offset,
 
 found_child:
         mds_pack_inode2body(body, dchild->d_inode);
+        if (!created && (rec->ur_flags & MDS_OPEN_CREAT) &&
+            (rec->ur_flags & MDS_OPEN_EXCL)) {
+                /* File already exists, we didn't just create it, and we
+                 * were passed O_EXCL; err-or. */
+                GOTO(cleanup, rc = -EEXIST); // returns a lock to the client
+        }
+
+        /* if we are following special file, don't open */
+        if (!S_ISREG(dchild->d_inode->i_mode) &&
+            !S_ISDIR(dchild->d_inode->i_mode))
+                GOTO(cleanup_no_trans, rc = 0);
 
+        ldlm_reply_set_disposition(rep, DISP_OPEN_OPEN);
         if (S_ISREG(dchild->d_inode->i_mode)) {
                 /* Check permissions etc */
                 rc = ll_permission(dchild->d_inode, acc_mode, NULL);
@@ -1310,36 +1322,23 @@ found_child:
                     ((rec->ur_flags & MDS_OPEN_APPEND) == 0 ||
                      (rec->ur_flags & MDS_OPEN_TRUNC) != 0))
                         GOTO(cleanup, rc = -EPERM);
-        }
-
-        if (!created && (rec->ur_flags & MDS_OPEN_CREAT) &&
-            (rec->ur_flags & MDS_OPEN_EXCL)) {
-                /* File already exists, we didn't just create it, and we
-                 * were passed O_EXCL; err-or. */
-                GOTO(cleanup, rc = -EEXIST); // returns a lock to the client
-        }
-
-        /* if we are following a symlink, don't open */
-        if (S_ISLNK(dchild->d_inode->i_mode))
-                GOTO(cleanup_no_trans, rc = 0);
-
-        if (S_ISDIR(dchild->d_inode->i_mode)) {
-                if (rec->ur_flags & MDS_OPEN_CREAT ||
-                    rec->ur_flags & FMODE_WRITE) {
-                        /* we are trying to create or write a exist dir */
-                        GOTO(cleanup, rc = -EISDIR);
-                }
-                if (rec->ur_flags & MDS_FMODE_EXEC) {
-                        /* we are trying to exec a directory */
-                        GOTO(cleanup, rc = -EACCES);
-                }
-                if (ll_permission(dchild->d_inode, acc_mode, NULL)) {
-                        ldlm_reply_set_disposition(rep, DISP_OPEN_OPEN);
-                        GOTO(cleanup, rc = -EACCES);
+        } else {
+                if (S_ISDIR(dchild->d_inode->i_mode)) {
+                        if (rec->ur_flags & MDS_OPEN_CREAT ||
+                            rec->ur_flags & FMODE_WRITE) {
+                                /* we are trying to create or write a exist dir*/
+                                GOTO(cleanup, rc = -EISDIR);
+                        }
+                        if (rec->ur_flags & MDS_FMODE_EXEC) {
+                                /* we are trying to exec a directory */
+                                GOTO(cleanup, rc = -EACCES);
+                        }
+                        if (ll_permission(dchild->d_inode, acc_mode, NULL))
+                                GOTO(cleanup, rc = -EACCES);
+                } else if (rec->ur_flags & MDS_OPEN_DIRECTORY) {
+                        GOTO(cleanup, rc = -ENOTDIR);
                 }
-        } else if (rec->ur_flags & MDS_OPEN_DIRECTORY) {
-                GOTO(cleanup, rc = -ENOTDIR);
-        }
+	}
 
         if (OBD_FAIL_CHECK(OBD_FAIL_MDS_OPEN_CREATE)) {
                 obd_fail_loc = OBD_FAIL_LDLM_REPLY | OBD_FAIL_ONCE;