LU-8962 lfs: Handle non-lustre and multiple args

This patch addresses:

01: Handle multiple filesystems provided to 'lfs df'
02: Correctly report 'EOPNOTSUPP' for filesystems which
    are non-Lustre.
03: Make changes to test-framework.sh to handle modified
    return value from 'lfs df'. This changes For compatibility
    reason, ignores and masquerades EOPNOTSUPP as success.

The final return value is 0 for _all_ success or
value of the first failure for even a single failure
seen during the argument processing

sanity/56e Test-case added.

Signed-off-by: Arshad Hussain <arshad.hussain@aeoncomputing.com>
Change-Id: I73287d21792d89b8cde672acdaf9c9caf829522f
Reviewed-on: https://review.whamcloud.com/42126
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Jian Yu <yujian@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/tests/conf-sanity.sh b/lustre/tests/conf-sanity.sh
index 258b3ff..e066261 100644 (file)
--- a/lustre/tests/conf-sanity.sh
+++ b/lustre/tests/conf-sanity.sh
@@ -5036,8 +5036,9 @@ test_69() {
        if [ $num_create -gt 0 ]; then
                # Check the number of inodes available on OST0
                local files=0
-               local ifree=$($LFS df -i $MOUNT | awk '/OST0000/ { print $4 }')
-               log "On OST0, $ifree inodes available. Want $num_create."
+               local ifree=$($LFS df -i $MOUNT |
+                       awk '/OST0000/ { print $4 }'; exit ${PIPESTATUS[0]})
+               log "On OST0, $ifree inodes available. Want $num_create. rc=$?"
 
                $LFS setstripe -i 0 $DIR/$tdir ||
                        error "$LFS setstripe -i 0 $DIR/$tdir failed"
@@ -5073,8 +5074,9 @@ test_69() {
        local idx=$($LFS getstripe -i $DIR/$tdir/$tfile-last)
        [ $idx -ne 0 ] && error "$DIR/$tdir/$tfile-last on $idx not 0" || true
 
-       local iused=$($LFS df -i $MOUNT | awk '/OST0000/ { print $3 }')
-       log "On OST0, $iused used inodes"
+       local iused=$($LFS df -i $MOUNT |
+               awk '/OST0000/ { print $3 }'; exit ${PIPESTATUS[0]})
+       log "On OST0, $iused used inodes rc=$?"
        [ $iused -ge $((ost_max_pre/2 + 1000)) ] &&
                error "OST replacement created too many inodes; $iused"
        cleanup || error "cleanup failed with $?"
@@ -6021,7 +6023,8 @@ test_82a() { # LU-4665
        mount_client $MOUNT || error "mount client $MOUNT failed"
        wait_osts_up
 
-       $LFS df $MOUNT || error "$LFS df $MOUNT failed"
+       $LFS df $MOUNT
+       check_lfs_df_ret_val $? || error "$LFS df $MOUNT failed"
        mkdir $DIR/$tdir || error "mkdir $DIR/$tdir failed"
 
        stack_trap "do_nodes $(comma_list $(mdts_nodes)) \
@@ -6136,7 +6139,8 @@ test_82b() { # LU-4665
        mount_client $MOUNT || error "mount client $MOUNT failed"
 
        wait_osts_up
-       $LFS df $MOUNT || error "$LFS df $MOUNT failed"
+       $LFS df $MOUNT
+       check_lfs_df_ret_val $? || error "$LFS df $MOUNT failed"
        mkdir $DIR/$tdir || error "mkdir $DIR/$tdir failed"
 
        # Create a new pool and add OSTs into it.
@@ -8743,9 +8747,10 @@ test_122b() {
 
        # Check the number of inodes available on OST0
        local files=0
-       local ifree=$($LFS df -i $MOUNT | awk '/OST0000/ { print $4 }')
+       local ifree=$($LFS df -i $MOUNT |
+               awk '/OST0000/ { print $4 }'; exit ${PIPESTATUS[0]})
 
-       log "On OST0, $ifree inodes available. Want $num_create."
+       log "On OST0, $ifree inodes available. Want $num_create. rc=$?"
 
        if [ $ifree -lt 10000 ]; then
                files=$(( ifree - 50 ))

diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh
index 3a9f141..266600a 100755 (executable)
--- a/lustre/tests/sanity.sh
+++ b/lustre/tests/sanity.sh
@@ -6131,6 +6131,47 @@ test_56d() {
 }
 run_test 56d "'lfs df -v' prints only configured devices"
 
+test_56e() {
+       err_enoent=2 # No such file or directory
+       err_eopnotsupp=95 # Operation not supported
+
+       enoent_mnt=/pmt1 # Invalid dentry. Path not present
+       notsup_mnt=/tmp  # Valid dentry, but Not a lustreFS
+
+       # Check for handling of path not exists
+       output=$($LFS df $enoent_mnt 2>&1)
+       ret=$?
+
+       fs=$(echo $output | awk -F: '{print $2}' | awk '{print $3}' | tr -d \')
+       [[ $fs = $enoent_mnt && $ret -eq $err_enoent ]] ||
+               error "expect failure $err_enoent, not $ret"
+
+       # Check for handling of non-Lustre FS
+       output=$($LFS df $notsup_mnt)
+       ret=$?
+
+       fs=$(echo $output | awk '{print $1}' | awk -F: '{print $2}')
+       [[ $fs = $notsup_mnt && $ret -eq $err_eopnotsupp ]] ||
+               error "expect success $err_eopnotsupp, not $ret"
+
+       # Check for multiple LustreFS argument
+       output=$($LFS df $MOUNT $MOUNT $MOUNT | grep -c "filesystem_summary:")
+       ret=$?
+
+       [[ $output -eq 3 && $ret -eq 0 ]] ||
+               error "expect success 3, not $output, rc = $ret"
+
+       # Check for correct non-Lustre FS handling among multiple
+       # LustreFS argument
+       output=$($LFS df $MOUNT $notsup_mnt $MOUNT |
+               grep -c "filesystem_summary:"; exit ${PIPESTATUS[0]})
+       ret=$?
+
+       [[ $output -eq 2 && $ret -eq $err_eopnotsupp ]] ||
+               error "expect success 2, not $output, rc = $ret"
+}
+run_test 56e "'lfs df' Handle non LustreFS & multiple LustreFS"
+
 NUMFILES=3
 NUMDIRS=3
 setup_56() {

diff --git a/lustre/tests/test-framework.sh b/lustre/tests/test-framework.sh
index f10815b..6eb29a3 100755 (executable)
--- a/lustre/tests/test-framework.sh
+++ b/lustre/tests/test-framework.sh
@@ -538,6 +538,17 @@ module_loaded () {
        /sbin/lsmod | grep -q "^\<$1\>"
 }
 
+check_lfs_df_ret_val() {
+       # Ignore only EOPNOTSUPP (which is 95; Operation not supported) error
+       # returned by 'lfs df' for valid dentry but not a lustrefs.
+       #
+       # 'lfs df' historically always returned success(0) instead of
+       # EOPNOTSUPP. This function for compatibility reason, ignores and
+       # masquerades EOPNOTSUPP as success.
+       [[ $1 -eq 95 ]] && return 0
+       return $1
+}
+
 PRLFS=false
 lustre_insmod() {
        local module=$1
@@ -2218,6 +2229,7 @@ restore_quota() {
 # This will allow fixing the "lfs df" summary line in the future.
 lfs_df() {
        $LFS df $* | sed -e 's/filesystem /filesystem_/'
+       check_lfs_df_ret_val $?
 }
 
 # Get free inodes on the MDT specified by mdt index, free indoes on
@@ -3522,12 +3534,20 @@ wait_remote_prog () {
 
 lfs_df_check() {
        local clients=${1:-$CLIENTS}
+       local rc
 
        if [ -z "$clients" ]; then
-               $LFS df $MOUNT
+               $LFS df $MOUNT > /dev/null
+               rc=$?
        else
                $PDSH $clients "$LFS df $MOUNT" > /dev/null
+               rc=$?
        fi
+
+       check_lfs_df_ret_val $rc
+       rc=$?
+
+       return $rc
 }
 
 clients_up() {

diff --git a/lustre/utils/lfs.c b/lustre/utils/lfs.c
index 5d6616a..c68eb66 100644 (file)
--- a/lustre/utils/lfs.c
+++ b/lustre/utils/lfs.c
@@ -7248,7 +7248,7 @@ static int lfs_df(int argc, char **argv)
        char mntdir[PATH_MAX] = {'\0'}, path[PATH_MAX] = {'\0'};
        enum mntdf_flags flags = MNTDF_SHOW;
        int ops = LL_STATFS_LMV | LL_STATFS_LOV;
-       int c, rc = 0, index = 0;
+       int c, rc = 0, rc1 = 0, index = 0, arg_idx = 0;
        char fsname[PATH_MAX] = "", *pool_name = NULL;
        struct option long_opts[] = {
        { .val = 'h',   .name = "human-readable", .has_arg = no_argument },
@@ -7285,26 +7285,75 @@ static int lfs_df(int argc, char **argv)
                        return CMD_HELP;
                }
        }
-       if (optind < argc && !realpath(argv[optind], path)) {
-               rc = -errno;
-               fprintf(stderr, "%s: invalid path '%s': %s\n",
-                       progname, argv[optind], strerror(-rc));
+
+       /* Handle case where path is not specified */
+       if (optind == argc) {
+               while (!llapi_search_mounts(path, index++, mntdir, fsname)) {
+                       /* Check if we have a mount point */
+                       if (mntdir[0] == '\0')
+                               continue;
+
+                       rc = mntdf(mntdir, fsname, pool_name, flags, ops, NULL);
+                       if (rc || path[0] != '\0')
+                               break;
+
+                       fsname[0] = '\0'; /* avoid matching in next loop */
+                       mntdir[0] = '\0'; /* avoid matching in next loop */
+                       path[0] = '\0'; /* clean for next loop */
+               }
                return rc;
        }
 
-       while (!llapi_search_mounts(path, index++, mntdir, fsname)) {
-               /* Check if we have a mount point */
-               if (mntdir[0] == '\0')
+       /* Loop through all the remaining arguments. These are Lustre FS
+        * paths.
+        */
+       for (arg_idx = optind; arg_idx <= argc - 1; arg_idx++) {
+               bool valid = false;
+
+               fsname[0] = '\0'; /* start clean */
+               mntdir[0] = '\0'; /* start clean */
+               path[0] = '\0';   /* start clean */
+
+               /* path does not exists at all */
+               if (!realpath(argv[arg_idx], path)) {
+                       rc = -errno;
+                       fprintf(stderr, "error: invalid path '%s': %s\n",
+                               argv[arg_idx], strerror(-rc));
+                       /* save first seen error */
+                       if (!rc1)
+                               rc1 = rc;
+
                        continue;
+               }
 
-               rc = mntdf(mntdir, fsname, pool_name, flags, ops, NULL);
-               if (rc || path[0] != '\0')
-                       break;
-               fsname[0] = '\0'; /* avoid matching in next loop */
-               mntdir[0] = '\0'; /* avoid matching in next loop */
+               /* path exists but may not be a Lustre filesystem */
+               while (!llapi_search_mounts(path, index++, mntdir, fsname)) {
+                       /* Check if we have a mount point */
+                       if (mntdir[0] == '\0')
+                               continue;
+
+                       rc = mntdf(mntdir, fsname, pool_name, flags, ops, NULL);
+                       if (rc || path[0] != '\0') {
+                               valid = true;
+
+                               /* save first seen error */
+                               if (!rc1)
+                                       rc1 = rc;
+                               break;
+                       }
+               }
+
+               if (!valid) {
+                       llapi_printf(LLAPI_MSG_ERROR,
+                                    "%s:%s Not a Lustre filesystem\n",
+                                    argv[0], argv[arg_idx]);
+                       /* save first seen error */
+                       if (!rc1)
+                               rc1 = -EOPNOTSUPP;
+               }
        }
 
-       return rc;
+       return rc1;
 }
 
 static int print_instance(const char *mntdir, char *buf, size_t buflen,