LU-8125 nrs: pol_arg should be copied after the policy starts

NRS policy->pol_arg should be copied to a given policy argument
after the policy starts, otherwise a wrong argument will be saved
and it will cause LBUG when we run "lctl get_param" to get NRS
policy.
sanityn.sh test_77h is added to verify this patch.

Signed-off-by: Emoly Liu <emoly.liu@intel.com>
Change-Id: I5a015bb44fa88f69a4d5347cc2721eb72a57c8ad
Reviewed-on: http://review.whamcloud.com/20164
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Li Xi <lixi@ddn.com>
Reviewed-by: Henri Doreau <henri.doreau@cea.fr>
Reviewed-by: Jean-Baptiste Riaux <riaux.jb@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>

diff --git a/lustre/ptlrpc/nrs.c b/lustre/ptlrpc/nrs.c
index 67626eb..8d637f3 100644 (file)
--- a/lustre/ptlrpc/nrs.c
+++ b/lustre/ptlrpc/nrs.c
@@ -264,14 +264,6 @@ static int nrs_policy_start_locked(struct ptlrpc_nrs_policy *policy, char *arg)
                RETURN(-ENODEV);
        }
 
                RETURN(-ENODEV);
        }
 

-       if (arg != NULL) {
-               if (strlen(arg) + 1 > sizeof(policy->pol_arg)) {
-                       CERROR("NRS: arg '%s' is too long\n", arg);
-                       GOTO(out, rc = -E2BIG);
-               }
-               strncpy(policy->pol_arg, arg, sizeof(policy->pol_arg));
-       }
-

        /**
         * Serialize policy starting across the NRS head
         */
        /**
         * Serialize policy starting across the NRS head
         */


@@ -294,6 +286,14 @@ static int nrs_policy_start_locked(struct ptlrpc_nrs_policy *policy, char *arg)
                }
        }
 
                }
        }
 

+       if (arg != NULL) {
+               if (strlcpy(policy->pol_arg, arg, sizeof(policy->pol_arg)) >=
+                   sizeof(policy->pol_arg)) {
+                       CERROR("NRS: arg '%s' is too long\n", arg);
+                       GOTO(out, rc = -E2BIG);
+               }
+       }
+

        policy->pol_state = NRS_POL_STATE_STARTED;
 
        if (policy->pol_flags & PTLRPC_NRS_FL_FALLBACK) {
        policy->pol_state = NRS_POL_STATE_STARTED;
 
        if (policy->pol_flags & PTLRPC_NRS_FL_FALLBACK) {

diff --git a/lustre/tests/sanityn.sh b/lustre/tests/sanityn.sh
index fc9a971..61de75c 100755 (executable)
--- a/lustre/tests/sanityn.sh
+++ b/lustre/tests/sanityn.sh
@@ -3211,6 +3211,43 @@ test_77g() {
 }
 run_test 77g "Change TBF type directly"
 
 }
 run_test 77g "Change TBF type directly"
 

+test_77h() {
+       [ $(lustre_version_code ost1) -ge $(version_code 2.8.55) ] ||
+               { skip "Need OST version at least 2.8.55"; return 0; }
+
+       local old_policy=$(do_facet ost1 \
+               lctl get_param ost.OSS.ost_io.nrs_policies)
+       local new_policy
+
+       do_facet ost1 lctl set_param \
+               ost.OSS.ost_io.nrs_policies="abc"
+       [ $? -eq 0 ] && error "should return error"
+
+       do_facet ost1 lctl set_param \
+               ost.OSS.ost_io.nrs_policies="tbf\ abc"
+       [ $? -eq 0 ] && error "should return error"
+
+       do_facet ost1 lctl set_param \
+               ost.OSS.ost_io.nrs_policies="tbf\ reg"
+       [ $? -eq 0 ] && error "should return error"
+
+       do_facet ost1 lctl set_param \
+               ost.OSS.ost_io.nrs_policies="tbf\ reg\ abc"
+       [ $? -eq 0 ] && error "should return error"
+
+       do_facet ost1 lctl set_param \
+               ost.OSS.ost_io.nrs_policies="tbf\ abc\ efg"
+       [ $? -eq 0 ] && error "should return error"
+
+       new_policy=$(do_facet ost1 lctl get_param ost.OSS.ost_io.nrs_policies)
+       [ $? -eq 0 ] || error "shouldn't LBUG"
+
+       [ "$old_policy" = "$new_policy" ] || error "NRS policy should be same"
+
+       return 0
+}
+run_test 77h "Wrong policy name should report error, not LBUG"
+

 test_78() { #LU-6673
        local rc
 
 test_78() { #LU-6673
        local rc