Whamcloud - gitweb
LU-6134 utils: lfs should only open/stat files if needed 22/13822/3
authorAndreas Dilger <andreas.dilger@intel.com>
Fri, 20 Feb 2015 12:25:43 +0000 (05:25 -0700)
committerOleg Drokin <oleg.drokin@intel.com>
Wed, 18 Mar 2015 11:30:41 +0000 (11:30 +0000)
Since (commit 322968acf183) "lfs find" would needlessly open() and
fstat() every file if the --ost, -uid/user, -gid/group, -[amc]time,
or -size options were used, to get the MDT index for each file.
This was causing "lfs find --ost" to fail if an OST was offline, and
added needless overhead that "lfs find" was meant to avoid.

The MDT index is only needed if --mdt is used, so only get it in
that case.  It also wasn't necessary to call fstat() in this case
either because the file type was already known at this point.

Some other minor cleanups related to fetching the MDT index:
- don't use ret in cb_get_dirstripe() as it is isn't needed
- fix cb_get_mdt_index() to avoid a Coverity false positive due to
  initializing rc and having a conditional branch that is always taken
- convert spaces to tabs for related code, other minor style fixes

Signed-off-by: Andreas Dilger <andreas.dilger@intel.com>
Change-Id: Ib41ced742fe5068f504f540479e6b4718d2540e5
Reviewed-on: http://review.whamcloud.com/13822
Reviewed-by: wangdi <di.wang@intel.com>
Tested-by: Jenkins
Reviewed-by: John L. Hammond <john.hammond@intel.com>
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>
lustre/utils/liblustreapi.c

index 719de6f..47ae129 100644 (file)
@@ -1616,17 +1616,13 @@ static DIR *opendir_parent(char *path)
 
 static int cb_get_dirstripe(char *path, DIR *d, struct find_param *param)
 {
-       struct lmv_user_md *lmv = (struct lmv_user_md *)param->fp_lmv_md;
-       int ret = 0;
-
-       lmv->lum_stripe_count = param->fp_lmv_stripe_count;
+       param->fp_lmv_md->lum_stripe_count = param->fp_lmv_stripe_count;
        if (param->fp_get_default_lmv)
-               lmv->lum_magic = LMV_USER_MAGIC;
+               param->fp_lmv_md->lum_magic = LMV_USER_MAGIC;
        else
-               lmv->lum_magic = LMV_MAGIC_V1;
-       ret = ioctl(dirfd(d), LL_IOC_LMV_GETSTRIPE, lmv);
+               param->fp_lmv_md->lum_magic = LMV_MAGIC_V1;
 
-       return ret;
+       return ioctl(dirfd(d), LL_IOC_LMV_GETSTRIPE, param->fp_lmv_md);
 }
 
 static int get_lmd_info(char *path, DIR *parent, DIR *dir,
@@ -3000,49 +2996,43 @@ static int cb_find_init(char *path, DIR *parent, DIR **dirp,
        if (decision == 0) {
                ret = get_lmd_info(path, parent, dir, param->fp_lmd,
                                   param->fp_lum_size);
-                if (ret == 0) {
-                        if (dir) {
+               if (ret == 0 && param->fp_mdt_uuid != NULL) {
+                       if (dir != NULL) {
                                ret = llapi_file_fget_mdtidx(dirfd(dir),
-                                               &param->fp_file_mdt_index);
-                        } else {
-                                int fd;
-                                lstat_t tmp_st;
-
-                                ret = lstat_f(path, &tmp_st);
-                                if (ret) {
-                                        ret = -errno;
-                                        llapi_error(LLAPI_MSG_ERROR, ret,
-                                                    "error: %s: lstat failed"
-                                                    "for %s", __func__, path);
-                                        return ret;
-                                }
-                                if (S_ISREG(tmp_st.st_mode)) {
-                                        fd = open(path, O_RDONLY);
-                                        if (fd > 0) {
-                                                ret = llapi_file_fget_mdtidx(fd,
                                                     &param->fp_file_mdt_index);
-                                                close(fd);
-                                        } else {
-                                                ret = fd;
-                                        }
-                                } else {
-                                        /* For special inode, it assumes to
-                                         * reside on the same MDT with the
-                                         * parent */
-                                        fd = dirfd(parent);
+                       } else if (S_ISREG(st->st_mode)) {
+                               int fd;
+
+                               /* FIXME: we could get the MDT index from the
+                                * file's FID in lmd->lmd_lmm.lmm_oi without
+                                * opening the file, once we are sure that
+                                * LFSCK2 (2.6) has fixed up pre-2.0 LOV EAs.
+                                * That would still be an ioctl() to map the
+                                * FID to the MDT, but not an open RPC. */
+                               fd = open(path, O_RDONLY);
+                               if (fd > 0) {
                                        ret = llapi_file_fget_mdtidx(fd,
                                                     &param->fp_file_mdt_index);
-                                }
-                        }
-                }
-                if (ret) {
-                        if (ret == -ENOTTY)
-                                lustre_fs = 0;
-                        if (ret == -ENOENT)
-                                goto decided;
-                        return ret;
-                }
-        }
+                                       close(fd);
+                               } else {
+                                       ret = -errno;
+                               }
+                       } else {
+                               /* For a special file, we assume it resides on
+                                * the same MDT as the parent directory. */
+                               ret = llapi_file_fget_mdtidx(dirfd(parent),
+                                                    &param->fp_file_mdt_index);
+                       }
+               }
+               if (ret != 0) {
+                       if (ret == -ENOTTY)
+                               lustre_fs = 0;
+                       if (ret == -ENOENT)
+                               goto decided;
+
+                       return ret;
+               }
+       }
 
        if (param->fp_type && !checked_type) {
                if ((st->st_mode & S_IFMT) == param->fp_type) {
@@ -3366,64 +3356,58 @@ int llapi_file_fget_mdtidx(int fd, int *mdtidx)
 static int cb_get_mdt_index(char *path, DIR *parent, DIR **dirp, void *data,
                            struct dirent64 *de)
 {
-        struct find_param *param = (struct find_param *)data;
+       struct find_param *param = (struct find_param *)data;
        DIR *d = dirp == NULL ? NULL : *dirp;
-        int ret = 0;
-        int mdtidx;
+       int ret;
+       int mdtidx;
 
-        LASSERT(parent != NULL || d != NULL);
+       LASSERT(parent != NULL || d != NULL);
 
-        if (d) {
-                ret = llapi_file_fget_mdtidx(dirfd(d), &mdtidx);
-        } else if (parent) {
-                int fd;
+       if (d != NULL) {
+               ret = llapi_file_fget_mdtidx(dirfd(d), &mdtidx);
+       } else /* if (parent) */ {
+               int fd;
 
-                fd = open(path, O_RDONLY);
-                if (fd > 0) {
-                        ret = llapi_file_fget_mdtidx(fd, &mdtidx);
-                        close(fd);
-                } else {
-                        ret = -errno;
-                }
-        }
+               fd = open(path, O_RDONLY | O_NOCTTY);
+               if (fd > 0) {
+                       ret = llapi_file_fget_mdtidx(fd, &mdtidx);
+                       close(fd);
+               } else {
+                       ret = -errno;
+               }
+       }
 
-        if (ret) {
-                if (ret == -ENODATA) {
+       if (ret != 0) {
+               if (ret == -ENODATA) {
                        if (!param->fp_obd_uuid)
-                                llapi_printf(LLAPI_MSG_NORMAL,
-                                             "%s has no stripe info\n", path);
-                        goto out;
-                } else if (ret == -ENOENT) {
-                        llapi_error(LLAPI_MSG_WARN, ret,
-                                    "warning: %s: %s does not exist",
-                                    __func__, path);
-                        goto out;
-                } else if (ret == -ENOTTY) {
-                        llapi_error(LLAPI_MSG_ERROR, ret,
-                                    "%s: '%s' not on a Lustre fs?",
-                                    __func__, path);
-                } else {
-                        llapi_error(LLAPI_MSG_ERROR, ret,
-                                    "error: %s: LL_IOC_GET_MDTIDX failed for %s",
-                                    __func__, path);
-                }
-                return ret;
-        }
+                               llapi_printf(LLAPI_MSG_NORMAL,
+                                            "'%s' has no stripe info\n", path);
+                       goto out;
+               } else if (ret == -ENOENT) {
+                       llapi_error(LLAPI_MSG_WARN, ret,
+                                   "warning: %s: '%s' does not exist",
+                                   __func__, path);
+                       goto out;
+               } else if (ret == -ENOTTY) {
+                       llapi_error(LLAPI_MSG_ERROR, ret,
+                                   "%s: '%s' not on a Lustre fs",
+                                   __func__, path);
+               } else {
+                       llapi_error(LLAPI_MSG_ERROR, ret,
+                                   "error: %s: '%s' failed get_mdtidx",
+                                   __func__, path);
+               }
+               return ret;
+       }
 
-       /* The 'LASSERT(parent != NULL || d != NULL);' guarantees
-        * that either 'd' or 'parent' is not null.
-        * So in all cases llapi_file_fget_mdtidx() is called,
-        * thus initializing 'mdtidx'. */
        if (param->fp_quiet || !(param->fp_verbose & VERBOSE_DETAIL))
-               /* coverity[uninit_use_in_call] */
-                llapi_printf(LLAPI_MSG_NORMAL, "%d\n", mdtidx);
-        else
-               /* coverity[uninit_use_in_call] */
-                llapi_printf(LLAPI_MSG_NORMAL, "%s\nmdt_index:\t%d\n",
-                             path, mdtidx);
+               llapi_printf(LLAPI_MSG_NORMAL, "%d\n", mdtidx);
+       else
+               llapi_printf(LLAPI_MSG_NORMAL, "%s\nmdt_index:\t%d\n",
+                            path, mdtidx);
 
 out:
-       /* Do not get down anymore? */
+       /* Do not go down anymore? */
        if (param->fp_depth == param->fp_max_depth)
                return 1;