Whamcloud - gitweb
LU-14826 mdt: getattr_name("..") under striped directory 68/44168/6
authorLai Siyao <lai.siyao@whamcloud.com>
Thu, 8 Jul 2021 14:25:51 +0000 (10:25 -0400)
committerOleg Drokin <green@whamcloud.com>
Thu, 22 Jul 2021 04:53:47 +0000 (04:53 +0000)
For getattr_name(".."), it should return FID of the master object for
striped directories. This includes changes on both client and server:
* lmv_getattr_name() should use master object FID if it's looking up
  "..".
* mdt_raw_lookup() should check parent object is sub stripe, if so
  it needs to lookup again to get master object FID. For old client
  without above change this needs to be checked twice.

This is needed by NFS export, because ll_get_parent() find parent by
getattr_name("..").

Reenable check_fhandle_syscall and update sanityn test_102.

Signed-off-by: Lai Siyao <lai.siyao@whamcloud.com>
Change-Id: I72c951293e41656ce3778750147402d7f8ca4cec
Reviewed-on: https://review.whamcloud.com/44168
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: James Simmons <jsimmons@infradead.org>
lustre/lmv/lmv_obd.c
lustre/mdt/mdt_handler.c
lustre/tests/check_fhandle_syscalls.c
lustre/tests/sanityn.sh

index ba1b656..717e410 100644 (file)
@@ -1976,7 +1976,11 @@ lmv_getattr_name(struct obd_export *exp,struct md_op_data *op_data,
        ENTRY;
 
 retry:
-       tgt = lmv_locate_tgt(lmv, op_data);
+       if (op_data->op_namelen == 2 &&
+           op_data->op_name[0] == '.' && op_data->op_name[1] == '.')
+               tgt = lmv_fid2tgt(lmv, &op_data->op_fid1);
+       else
+               tgt = lmv_locate_tgt(lmv, op_data);
        if (IS_ERR(tgt))
                RETURN(PTR_ERR(tgt));
 
index 6455708..0a2149b 100644 (file)
@@ -270,6 +270,41 @@ static void mdt_lock_pdo_mode(struct mdt_thread_info *info, struct mdt_object *o
         EXIT;
 }
 
+/**
+ * Check whether \a o is directory stripe object.
+ *
+ * \param[in]  info    thread environment
+ * \param[in]  o       MDT object
+ *
+ * \retval 1   is directory stripe.
+ * \retval 0   isn't directory stripe.
+ * \retval < 1  error code
+ */
+static int mdt_is_dir_stripe(struct mdt_thread_info *info,
+                               struct mdt_object *o)
+{
+       struct md_attr *ma = &info->mti_attr;
+       struct lmv_mds_md_v1 *lmv;
+       int rc;
+
+       rc = mdt_stripe_get(info, o, ma, XATTR_NAME_LMV);
+       if (rc < 0)
+               return rc;
+
+       if (!(ma->ma_valid & MA_LMV))
+               return 0;
+
+       lmv = &ma->ma_lmv->lmv_md_v1;
+
+       if (!lmv_is_sane2(lmv))
+               return -EBADF;
+
+       if (le32_to_cpu(lmv->lmv_magic) == LMV_MAGIC_STRIPE)
+               return 1;
+
+       return 0;
+}
+
 static int mdt_lookup_fileset(struct mdt_thread_info *info, const char *fileset,
                              struct lu_fid *fid)
 {
@@ -1808,30 +1843,62 @@ out:
 
 static int mdt_raw_lookup(struct mdt_thread_info *info,
                          struct mdt_object *parent,
-                         const struct lu_name *lname,
-                         struct ldlm_reply *ldlm_rep)
+                         const struct lu_name *lname)
 {
-       struct lu_fid   *child_fid = &info->mti_tmp_fid1;
-       int              rc;
+       struct lu_fid *fid = &info->mti_tmp_fid1;
+       struct mdt_body *repbody;
+       bool is_dotdot = false;
+       bool is_old_parent_stripe = false;
+       bool is_new_parent_checked = false;
+       int rc;
+
        ENTRY;
 
        LASSERT(!info->mti_cross_ref);
+       /* Always allow to lookup ".." */
+       if (lname->ln_namelen == 2 &&
+           lname->ln_name[0] == '.' && lname->ln_name[1] == '.') {
+               info->mti_spec.sp_permitted = 1;
+               is_dotdot = true;
+               if (mdt_is_dir_stripe(info, parent) == 1)
+                       is_old_parent_stripe = true;
+       }
 
+       mdt_object_get(info->mti_env, parent);
+lookup:
        /* Only got the fid of this obj by name */
-       fid_zero(child_fid);
-       rc = mdo_lookup(info->mti_env, mdt_object_child(info->mti_object),
-                       lname, child_fid, &info->mti_spec);
-       if (rc == 0) {
-               struct mdt_body *repbody;
+       fid_zero(fid);
+       rc = mdo_lookup(info->mti_env, mdt_object_child(parent), lname, fid,
+                       &info->mti_spec);
+       mdt_object_put(info->mti_env, parent);
+       if (rc)
+               RETURN(rc);
+
+       /* getattr_name("..") should return master object FID for striped dir */
+       if (is_dotdot && (is_old_parent_stripe || !is_new_parent_checked)) {
+               parent = mdt_object_find(info->mti_env, info->mti_mdt, fid);
+               if (IS_ERR(parent))
+                       RETURN(PTR_ERR(parent));
 
-               repbody = req_capsule_server_get(info->mti_pill, &RMF_MDT_BODY);
-               repbody->mbo_fid1 = *child_fid;
-               repbody->mbo_valid = OBD_MD_FLID;
-               mdt_set_disposition(info, ldlm_rep, DISP_LOOKUP_POS);
-       } else if (rc == -ENOENT) {
-               mdt_set_disposition(info, ldlm_rep, DISP_LOOKUP_NEG);
+               /* old client getattr_name("..") with stripe FID */
+               if (unlikely(is_old_parent_stripe)) {
+                       is_old_parent_stripe = false;
+                       goto lookup;
+               }
+
+               /* ".." may be a stripe */
+               if (unlikely(mdt_is_dir_stripe(info, parent) == 1)) {
+                       is_new_parent_checked = true;
+                       goto lookup;
+               }
+
+               mdt_object_put(info->mti_env, parent);
        }
 
+       repbody = req_capsule_server_get(info->mti_pill, &RMF_MDT_BODY);
+       repbody->mbo_fid1 = *fid;
+       repbody->mbo_valid = OBD_MD_FLID;
+
        RETURN(rc);
 }
 
@@ -1991,14 +2058,8 @@ static int mdt_getattr_name_lock(struct mdt_thread_info *info,
        }
 
        if (lu_name_is_valid(lname)) {
-               /* Always allow to lookup ".." */
-               if (unlikely(lname->ln_namelen == 2 &&
-                            lname->ln_name[0] == '.' &&
-                            lname->ln_name[1] == '.'))
-                       info->mti_spec.sp_permitted = 1;
-
                if (info->mti_body->mbo_valid == OBD_MD_FLID) {
-                       rc = mdt_raw_lookup(info, parent, lname, ldlm_rep);
+                       rc = mdt_raw_lookup(info, parent, lname);
 
                        RETURN(rc);
                }
@@ -6743,7 +6804,6 @@ static int mdt_path_current(struct mdt_thread_info *info,
        struct lu_name *tmpname = &info->mti_name;
        struct lu_fid *tmpfid = &info->mti_tmp_fid1;
        struct lu_buf *buf = &info->mti_big_buf;
-       struct md_attr *ma = &info->mti_attr;
        struct linkea_data ldata = { NULL };
        bool first = true;
        struct mdt_object *mdt_obj;
@@ -6816,22 +6876,13 @@ static int mdt_path_current(struct mdt_thread_info *info,
                }
 
                /* Check if it is slave stripes */
-               rc = mdt_stripe_get(info, mdt_obj, ma, XATTR_NAME_LMV);
+               rc = mdt_is_dir_stripe(info, mdt_obj);
                mdt_object_put(info->mti_env, mdt_obj);
                if (rc < 0)
                        GOTO(out, rc);
-
-               if (ma->ma_valid & MA_LMV) {
-                       struct lmv_mds_md_v1 *lmv = &ma->ma_lmv->lmv_md_v1;
-
-                       if (!lmv_is_sane2(lmv))
-                               GOTO(out, rc = -EBADF);
-
-                       /* For slave stripes, get its master */
-                       if (le32_to_cpu(lmv->lmv_magic) == LMV_MAGIC_STRIPE) {
-                               fp->gf_fid = *tmpfid;
-                               continue;
-                       }
+               if (rc == 1) {
+                       fp->gf_fid = *tmpfid;
+                       continue;
                }
 
                /* Pack the name in the end of the buffer */
index 4806974..a6e8e01 100644 (file)
@@ -57,19 +57,6 @@ void usage(char *prog)
 }
 
 
-#ifndef HAVE_FHANDLE_SYSCALLS
-
-int main(int argc, char **argv)
-{
-       if (argc != 3)
-               usage(argv[0]);
-
-       fprintf(stderr, "HAVE_FHANDLE_SYSCALLS not defined\n");
-       return 0;
-}
-
-#else
-
 #ifndef HAVE_FHANDLE_GLIBC_SUPPORT
 /* Because the kernel supports this functions doesn't mean that glibc does.
  * Just in case we define what we need */
@@ -127,6 +114,14 @@ open_by_handle_at(int mnt_fd, struct file_handle *fh, int mode)
 }
 #endif
 
+static int debug_mark(const char *msg)
+{
+       char cmd[4096] = "";
+
+       snprintf(cmd, sizeof(cmd), "../utils/lctl mark %s", msg);
+       return system(cmd);
+}
+
 /* verify a file contents */
 int check_access(const char *filename,
                 int mnt_fd, struct file_handle *fh, struct stat *st_orig)
@@ -135,18 +130,25 @@ int check_access(const char *filename,
        struct stat st;
        char *readbuf = NULL;
 
+       debug_mark("before open by handle");
        /* Open the file handle */
-       fd2 = open_by_handle_at(mnt_fd, fh, O_RDONLY);
+       fd2 = open_by_handle_at(mnt_fd, fh, O_RDONLY |
+                               (S_ISDIR(st_orig->st_mode) ? O_DIRECTORY : 0));
+       debug_mark("after open by handle");
        if (fd2 < 0) {
                fprintf(stderr, "open_by_handle_at(%s) error: %s\n", filename,
                        strerror(errno));
+               if (errno == ESTALE)
+                       fprintf(stderr, "second mountpoint not mounted?\n");
                rc = errno;
                goto out_f_handle;
        }
 
        /* Get file size */
        bzero(&st, sizeof(struct stat));
+       debug_mark("before stat");
        rc = fstat(fd2, &st);
+       debug_mark("after stat");
        if (rc < 0) {
                fprintf(stderr, "fstat(%s) error: %s\n", filename,
                        strerror(errno));
@@ -157,14 +159,15 @@ int check_access(const char *filename,
        /* we can't check a ctime due unlink update */
        if (st_orig->st_size != st.st_size ||
            st_orig->st_ino != st.st_ino ||
+           st_orig->st_mode != st.st_mode ||
            st_orig->st_mtime != st.st_mtime) {
-               fprintf(stderr, "stat data does not match between fopen "
-                       "and fhandle case\n");
+               fprintf(stderr,
+                       "stat data mismatch between fopen and fhandle case\n");
                rc = EINVAL;
                goto out_fd2;
        }
 
-       if (st.st_size) {
+       if (st.st_size && S_ISREG(st.st_mode)) {
                len = st.st_blksize;
                readbuf = malloc(len);
                if (readbuf == NULL) {
@@ -217,7 +220,9 @@ int main(int argc, char **argv)
        }
        filename = rindex(file, '/') + 1;
 
+       debug_mark("before first open");
        fd1 = open(file, O_RDONLY);
+       debug_mark("after first open");
        if (fd1 < 0) {
                fprintf(stderr, "open file %s error: %s\n",
                        file, strerror(errno));
@@ -227,7 +232,9 @@ int main(int argc, char **argv)
 
        /* Get file stats using fd1 from traditional open */
        bzero(&st, sizeof(struct stat));
+       debug_mark("before first stat");
        rc = fstat(fd1, &st);
+       debug_mark("after first stat");
        if (rc < 0) {
                fprintf(stderr, "fstat(%s) error: %s\n", file,
                        strerror(errno));
@@ -236,7 +243,9 @@ int main(int argc, char **argv)
        }
 
        /* Open mount point directory */
+       debug_mark("before directory open");
        mnt_fd = open(argv[2], O_DIRECTORY);
+       debug_mark("after directory open");
        if (mnt_fd < 0) {
                fprintf(stderr, "open(%s) error: %s\n)", argv[2],
                        strerror(errno));
@@ -255,8 +264,10 @@ int main(int argc, char **argv)
        fh->handle_bytes = MAX_HANDLE_SZ;
 
        /* Convert name to handle */
+       debug_mark("before get handle");
        ret = name_to_handle_at(AT_FDCWD, file, fh, &mnt_id,
                                AT_SYMLINK_FOLLOW);
+       debug_mark("after get handle");
        if (ret) {
                fprintf(stderr, "name_by_handle_at(%s) error: %s\n", filename,
                        strerror(errno));
@@ -265,8 +276,8 @@ int main(int argc, char **argv)
        }
 
        /* Print out the contents of the file handle */
-       fprintf(stdout, "fh_bytes: %u\nfh_type: %d\nfh_data: ",
-               fh->handle_bytes, fh->handle_type);
+       fprintf(stdout, "file: %s\nfh_bytes: %u\nfh_type: %d\nfh_data: ",
+               file, fh->handle_bytes, fh->handle_type);
        for (i = 0; i < fh->handle_bytes; i++)
                fprintf(stdout, "%02x ", fh->f_handle[i]);
        fprintf(stdout, "\n");
@@ -278,24 +289,32 @@ int main(int argc, char **argv)
        fprintf(stdout, "file's parent FID is "DFID"\n", PFID(parent));
        fprintf(stdout, "file FID is "DFID"\n", PFID(fid));
 
-       fprintf(stdout, "just access via different mount point - ");
+       fprintf(stdout, "access via mount point '%s' - ", argv[2]);
+       fflush(stdout);
        rc = check_access(filename, mnt_fd, fh, &st);
        if (rc != 0)
                goto out_f_handle;
        fprintf(stdout, "OK \n");
+       fflush(stdout);
+
+       if (S_ISREG(st.st_mode)) {
+               fprintf(stdout, "access after unlink - ");
+               fflush(stdout);
+               ret = unlink(file);
+               if (ret < 0) {
+                       fprintf(stderr,
+                               "can't unlink '%s'. check permissions?\n",
+                               file);
+                       goto out_f_handle;
+               }
 
-       fprintf(stdout, "access after unlink - ");
-       ret = unlink(file);
-       if (ret < 0) {
-               fprintf(stderr, "can't unlink a file. check permissions?\n");
-               goto out_f_handle;
+               rc = check_access(filename, mnt_fd, fh, &st);
+               if (rc != 0)
+                       goto out_f_handle;
+               fprintf(stdout, "OK\n");
+               fflush(stdout);
        }
 
-       rc = check_access(filename, mnt_fd, fh, &st);
-       if (rc != 0)
-               goto out_f_handle;
-       fprintf(stdout, "OK\n");
-
        rc = 0;
        fprintf(stdout, "check_fhandle_syscalls test Passed!\n");
 
@@ -308,5 +327,3 @@ out_fd1:
 out:
        return rc;
 }
-
-#endif
index 752f917..0ab469b 100755 (executable)
@@ -5217,8 +5217,28 @@ test_102() {
        echo "Test file_handle syscalls" > $DIR/$tfile ||
                error "write failed"
        check_fhandle_syscalls $DIR/$tfile $DIR2 ||
-               error "check_fhandle_syscalls failed"
-       rm -f $DIR2/$tfile
+               error "check_fhandle_syscalls $tfile failed"
+
+       # test this is working on DNE directories also
+       if (( MDSCOUNT > 1  MDS1_VERSION >= $(version_code 2.14.52) )); then
+               $LFS mkdir -i 1 $DIR/$tdir.remote
+               cancel_lru_locks mdc
+               check_fhandle_syscalls $DIR/$tdir.remote $DIR2 ||
+                       error "check_fhandle_syscalls $tdir.remote failed"
+               $LFS mkdir -c -1 $DIR/$tdir.remote/subdir
+               cancel_lru_locks mdc
+               check_fhandle_syscalls $DIR/$tdir.remote/subdir $DIR2 ||
+                       error "check_fhandle_syscalls $tdir.remote/subdir fail"
+
+               $LFS mkdir -c -1 $DIR/$tdir.stripe
+               cancel_lru_locks mdc
+               check_fhandle_syscalls $DIR/$tdir.stripe $DIR2 ||
+                       error "check_fhandle_syscalls $tdir.stripe failed"
+               $LFS mkdir -c -1 $DIR/$tdir.stripe/subdir
+               cancel_lru_locks mdc
+               check_fhandle_syscalls $DIR/$tdir.stripe/subdir $DIR2 ||
+                       error "check_fhandle_syscalls $tdir.stripe/subdir fail"
+       fi
 }
 run_test 102 "Test open by handle of unlinked file"