LU-1069 mdc: Avoid doing permission check again.

inode_setattr() replaced by simple_setattr(). The latter
will invoke inode_change_ok(). It causes a -EPERM error when
we want to update inode timestamps.

Signed-off-by: yang sheng <ys@whamcloud.com>
Change-Id: I29c58d7027df1bf94db58df82e5214fd12ea5c87
Reviewed-on: http://review.whamcloud.com/2145
Tested-by: Hudson
Tested-by: Maloo <whamcloud.maloo@gmail.com>
Reviewed-by: Fan Yong <yong.fan@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/include/linux/lustre_compat25.h b/lustre/include/linux/lustre_compat25.h
index 470b2b7..d6e7cfe 100644 (file)
--- a/lustre/include/linux/lustre_compat25.h
+++ b/lustre/include/linux/lustre_compat25.h
@@ -930,5 +930,11 @@ static inline int ll_quota_off(struct super_block *sb, int off, int remount)
 # define ext2_find_next_zero_bit  find_next_zero_bit_le
 #endif
 
 # define ext2_find_next_zero_bit  find_next_zero_bit_le
 #endif
 

+#ifdef ATTR_TIMES_SET
+# define TIMES_SET_FLAGS (ATTR_MTIME_SET | ATTR_ATIME_SET | ATTR_TIMES_SET)
+#else
+# define TIMES_SET_FLAGS (ATTR_MTIME_SET | ATTR_ATIME_SET)
+#endif
+

 #endif /* __KERNEL__ */
 #endif /* _COMPAT25_H */
 #endif /* __KERNEL__ */
 #endif /* _COMPAT25_H */

diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c
index 4e49985..815cf6c 100644 (file)
--- a/lustre/llite/llite_lib.c
+++ b/lustre/llite/llite_lib.c
@@ -1200,7 +1200,7 @@ int ll_md_setattr(struct dentry *dentry, struct md_op_data *op_data,
         struct inode *inode = dentry->d_inode;
         struct ll_sb_info *sbi = ll_i2sbi(inode);
         struct ptlrpc_request *request = NULL;
         struct inode *inode = dentry->d_inode;
         struct ll_sb_info *sbi = ll_i2sbi(inode);
         struct ptlrpc_request *request = NULL;

-        int rc;
+        int rc, ia_valid;

         ENTRY;
 
         op_data = ll_prep_md_op_data(op_data, inode, NULL, NULL, 0, 0,
         ENTRY;
 
         op_data = ll_prep_md_op_data(op_data, inode, NULL, NULL, 0, 0,


@@ -1217,8 +1217,12 @@ int ll_md_setattr(struct dentry *dentry, struct md_op_data *op_data,
                         /* Unlinked special device node? Or just a race?
                          * Pretend we done everything. */
                         if (!S_ISREG(inode->i_mode) &&
                         /* Unlinked special device node? Or just a race?
                          * Pretend we done everything. */
                         if (!S_ISREG(inode->i_mode) &&

-                            !S_ISDIR(inode->i_mode))
+                            !S_ISDIR(inode->i_mode)) {
+                                ia_valid = op_data->op_attr.ia_valid;
+                                op_data->op_attr.ia_valid &= ~TIMES_SET_FLAGS;

                                 rc = simple_setattr(dentry, &op_data->op_attr);
                                 rc = simple_setattr(dentry, &op_data->op_attr);

+                                op_data->op_attr.ia_valid = ia_valid;
+                        }

                 } else if (rc != -EPERM && rc != -EACCES && rc != -ETXTBSY) {
                         CERROR("md_setattr fails: rc = %d\n", rc);
                 }
                 } else if (rc != -EPERM && rc != -EACCES && rc != -ETXTBSY) {
                         CERROR("md_setattr fails: rc = %d\n", rc);
                 }


@@ -1232,12 +1236,16 @@ int ll_md_setattr(struct dentry *dentry, struct md_op_data *op_data,
                 RETURN(rc);
         }
 
                 RETURN(rc);
         }
 

-        /* We call inode_setattr to adjust timestamps.
+        /* We want to adjust timestamps.

          * If there is at least some data in file, we cleared ATTR_SIZE
          * If there is at least some data in file, we cleared ATTR_SIZE

-         * above to avoid invoking vmtruncate, otherwise it is important
-         * to call vmtruncate in inode_setattr to update inode->i_size
+         * to avoid update size, otherwise it is important to do.(SOM case)

          * (bug 6196) */
          * (bug 6196) */

+        ia_valid = op_data->op_attr.ia_valid;
+        /* Since we set ATTR_*_SET flags above, and already done permission
+         * check, So don't let inode_change_ok() check it again.  */
+        op_data->op_attr.ia_valid &= ~TIMES_SET_FLAGS;

         rc = simple_setattr(dentry, &op_data->op_attr);
         rc = simple_setattr(dentry, &op_data->op_attr);

+        op_data->op_attr.ia_valid = ia_valid;

 
         /* Extract epoch data if obtained. */
         op_data->op_handle = md.body->handle;
 
         /* Extract epoch data if obtained. */
         op_data->op_handle = md.body->handle;


@@ -1340,7 +1348,7 @@ int ll_setattr_raw(struct dentry *dentry, struct iattr *attr)
         }
 
         /* POSIX: check before ATTR_*TIME_SET set (from inode_change_ok) */
         }
 
         /* POSIX: check before ATTR_*TIME_SET set (from inode_change_ok) */

-        if (ia_valid & (ATTR_MTIME_SET | ATTR_ATIME_SET)) {
+        if (ia_valid & TIMES_SET_FLAGS) {

                 if (cfs_curproc_fsuid() != inode->i_uid &&
                     !cfs_capable(CFS_CAP_FOWNER))
                         RETURN(-EPERM);
                 if (cfs_curproc_fsuid() != inode->i_uid &&
                     !cfs_capable(CFS_CAP_FOWNER))
                         RETURN(-EPERM);


@@ -1377,14 +1385,15 @@ int ll_setattr_raw(struct dentry *dentry, struct iattr *attr)
         if (op_data == NULL)
                 RETURN(-ENOMEM);
 
         if (op_data == NULL)
                 RETURN(-ENOMEM);
 

-        UNLOCK_INODE_MUTEX(inode);
-        if (ia_valid & ATTR_SIZE)
-                UP_WRITE_I_ALLOC_SEM(inode);
-        if (!S_ISDIR(inode->i_mode))
+        if (!S_ISDIR(inode->i_mode)) {
+                if (ia_valid & ATTR_SIZE)
+                        UP_WRITE_I_ALLOC_SEM(inode);
+                UNLOCK_INODE_MUTEX(inode);

                 cfs_down_write(&lli->lli_trunc_sem);
                 cfs_down_write(&lli->lli_trunc_sem);

-        LOCK_INODE_MUTEX(inode);
-        if (ia_valid & ATTR_SIZE)
-                DOWN_WRITE_I_ALLOC_SEM(inode);
+                LOCK_INODE_MUTEX(inode);
+                if (ia_valid & ATTR_SIZE)
+                        DOWN_WRITE_I_ALLOC_SEM(inode);
+        }

 
         memcpy(&op_data->op_attr, attr, sizeof(*attr));
 
 
         memcpy(&op_data->op_attr, attr, sizeof(*attr));
 

diff --git a/lustre/lvfs/fsfilt_ext3.c b/lustre/lvfs/fsfilt_ext3.c
index 5fbed73..08e09da 100644 (file)
--- a/lustre/lvfs/fsfilt_ext3.c
+++ b/lustre/lvfs/fsfilt_ext3.c
@@ -656,13 +656,15 @@ static int fsfilt_ext3_setattr(struct dentry *dentry, void *handle,
 
         /* We set these flags on the client, but have already checked perms
          * so don't confuse inode_change_ok. */
 
         /* We set these flags on the client, but have already checked perms
          * so don't confuse inode_change_ok. */

-        iattr->ia_valid &= ~(ATTR_MTIME_SET | ATTR_ATIME_SET);
+        iattr->ia_valid &= ~TIMES_SET_FLAGS;

 
         if (inode->i_op->setattr) {
                 rc = inode->i_op->setattr(dentry, iattr);
         } else {
 
         if (inode->i_op->setattr) {
                 rc = inode->i_op->setattr(dentry, iattr);
         } else {

+#ifndef HAVE_SIMPLE_SETATTR /* simple_setattr() already call it */

                 rc = inode_change_ok(inode, iattr);
                 if (!rc)
                 rc = inode_change_ok(inode, iattr);
                 if (!rc)

+#endif

                         rc = simple_setattr(dentry, iattr);
         }
 
                         rc = simple_setattr(dentry, iattr);
         }