LU-10703 nodemap: save and clear fileset correctly

This patch is to fix the following two issues:
- When processing the nodemap_idx_type "NODEMAP_CLUSTER_IDX" in
  nodemap_process_keyrec(), fileset should be saved, otherwise,
  it will be changed to empty every time when client is notified
  to fetch nodemap logs (mgc_process_recover_nodemap_log()->
  nodemap_process_idx_pages()->nodemap_process_keyrec()).
- Allow 'fileset=clear' in addition to 'fileset=""' to clear
  fileset because either 'lctl set_param -P *.*.fileset=""' or
  'lctl nodemap_set_fileset --fileset ""' can only work on MGS,
  while on other non-MGS servers, they both will invoke upcall
  "/usr/sbin/lctl set_param nodemap.default.fileset=" by function
  process_param2_config(), which will cause "no value" error and
  won't clear fileset. 'fileset=""' is still kept for compatibility
  reason.

Also, sanity-sec.sh test_27a is modified and test_27b is added to
verify this patch.

Change-Id: I23236a4f1b67ac555713d6b3f059df699fdc91dc
Signed-off-by: Emoly Liu <emoly.liu@intel.com>
Reviewed-on: https://review.whamcloud.com/31450
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Reviewed-by: Sebastien Buisson <sbuisson@ddn.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>

diff --git a/lustre/ptlrpc/nodemap_handler.c b/lustre/ptlrpc/nodemap_handler.c
index 41d8984..576779e 100644 (file)
--- a/lustre/ptlrpc/nodemap_handler.c
+++ b/lustre/ptlrpc/nodemap_handler.c
@@ -924,8 +924,20 @@ static int nodemap_set_fileset_helper(struct nodemap_config *config,
 {
        int rc = 0;
 
-       /* we allow fileset = "" which means clear fileset info */
-       if (fileset == NULL || (fileset[0] != 0 && fileset[0] != '/'))
+       /* Allow 'fileset=clear' in addition to 'fileset=""' to clear fileset
+        * because either command 'lctl set_param -P *.*.fileset=""' or
+        * 'lctl nodemap_set_fileset --fileset ""' can only work correctly
+        * on MGS, while on other servers, both commands will invoke upcall
+        * "/usr/sbin/lctl set_param nodemap.default.fileset=" by function
+        * process_param2_config(), which will cause "no value" error and
+        * won't clear fileset.
+        * 'fileset=""' is still kept for compatibility reason.
+        */
+       if (fileset == NULL)
+               rc = -EINVAL;
+       else if (fileset[0] == '\0' || strcmp(fileset, "clear") == 0)
+               nodemap->nm_fileset[0] = '\0';
+       else if (fileset[0] != '/')
                rc = -EINVAL;
        else if (strlcpy(nodemap->nm_fileset, fileset,
                         sizeof(nodemap->nm_fileset)) >=
@@ -947,8 +959,7 @@ int nodemap_set_fileset(const char *name, const char *fileset)
                GOTO(out, rc = PTR_ERR(nodemap));
        }
 
-       rc = nodemap_set_fileset_helper(active_config, nodemap,
-                                               fileset);
+       rc = nodemap_set_fileset_helper(active_config, nodemap, fileset);
        mutex_unlock(&active_config_lock);
 
        nodemap_putref(nodemap);
@@ -999,6 +1010,7 @@ struct lu_nodemap *nodemap_create(const char *name,
        struct lu_nodemap       *default_nodemap;
        struct cfs_hash         *hash = config->nmc_nodemap_hash;
        int                      rc = 0;
+       ENTRY;
 
        default_nodemap = config->nmc_default_nodemap;
 
@@ -1082,11 +1094,11 @@ struct lu_nodemap *nodemap_create(const char *name,
                nodemap->nm_fileset[0] = '\0';
        }
 
-       return nodemap;
+       RETURN(nodemap);
 
 out:
        CERROR("cannot add nodemap: '%s': rc = %d\n", name, rc);
-       return ERR_PTR(rc);
+       RETURN(ERR_PTR(rc));
 }
 
 /**

diff --git a/lustre/ptlrpc/nodemap_storage.c b/lustre/ptlrpc/nodemap_storage.c
index 65a8884..c3e515f 100644 (file)
--- a/lustre/ptlrpc/nodemap_storage.c
+++ b/lustre/ptlrpc/nodemap_storage.c
@@ -713,7 +713,9 @@ static int nodemap_process_keyrec(struct nodemap_config *config,
                              " nodemap_id=%d. nodemap config file corrupt?\n",
                              nodemap_id);
                break;
-       case NODEMAP_CLUSTER_IDX:
+       case NODEMAP_CLUSTER_IDX: {
+               struct lu_nodemap *old_nm = NULL;
+
                nodemap = cfs_hash_lookup(config->nmc_nodemap_hash,
                                          rec->ncr.ncr_name);
                if (nodemap == NULL) {
@@ -757,6 +759,17 @@ static int nodemap_process_keyrec(struct nodemap_config *config,
                nodemap->nmf_enable_audit =
                                        flags & NM_FL_ENABLE_AUDIT;
 
+               /* The fileset should be saved otherwise it will be empty
+                * every time in case of "NODEMAP_CLUSTER_IDX". */
+               mutex_lock(&active_config_lock);
+               old_nm = nodemap_lookup(rec->ncr.ncr_name);
+               if (!IS_ERR(old_nm) && old_nm->nm_fileset[0] != '\0')
+                       strlcpy(nodemap->nm_fileset, old_nm->nm_fileset,
+                               sizeof(nodemap->nm_fileset));
+               mutex_unlock(&active_config_lock);
+               if (!IS_ERR(old_nm))
+                       nodemap_putref(old_nm);
+
                if (*recent_nodemap == NULL) {
                        *recent_nodemap = nodemap;
                        INIT_LIST_HEAD(&nodemap->nm_list);
@@ -766,6 +779,7 @@ static int nodemap_process_keyrec(struct nodemap_config *config,
                }
                nodemap_putref(nodemap);
                break;
+       }
        case NODEMAP_RANGE_IDX:
                nid[0] = le64_to_cpu(rec->nrr.nrr_start_nid);
                nid[1] = le64_to_cpu(rec->nrr.nrr_end_nid);

diff --git a/lustre/tests/sanity-sec.sh b/lustre/tests/sanity-sec.sh
index b2027ee..a414530 100755 (executable)
--- a/lustre/tests/sanity-sec.sh
+++ b/lustre/tests/sanity-sec.sh
@@ -997,12 +997,9 @@ wait_nm_sync() {
        local nodemap_name=$1
        local key=$2
        local value=$3
-       local proc_param="${nodemap_name}.${key}"
-       [ "$nodemap_name" == "active" ] && proc_param="active"
-
+       local opt=$4
+       local proc_param
        local is_active=$(do_facet mgs $LCTL get_param -n nodemap.active)
-       (( is_active == 0 )) && [ "$proc_param" != "active" ] && return
-
        local max_retries=20
        local is_sync
        local out1=""
@@ -1010,8 +1007,17 @@ wait_nm_sync() {
        local mgs_ip=$(host_nids_address $mgs_HOST $NETTYPE | cut -d' ' -f1)
        local i
 
+       if [ "$nodemap_name" == "active" ]; then
+               proc_param="active"
+       elif [ -z "$key" ]; then
+               proc_param=${nodemap_name}
+       else
+               proc_param="${nodemap_name}.${key}"
+       fi
+       (( is_active == 0 )) && [ "$proc_param" != "active" ] && return
+
        if [ -z "$value" ]; then
-               out1=$(do_facet mgs $LCTL get_param nodemap.${proc_param})
+               out1=$(do_facet mgs $LCTL get_param $opt nodemap.${proc_param})
                echo "On MGS ${mgs_ip}, ${proc_param} = $out1"
        else
                out1=$value;
@@ -1028,7 +1034,7 @@ wait_nm_sync() {
                        [ $node_ip == $mgs_ip ] && continue
                    fi
 
-                   out2=$(do_node $node_ip $LCTL get_param \
+                   out2=$(do_node $node_ip $LCTL get_param $opt \
                                   nodemap.$proc_param 2>/dev/null)
                    echo "On $node ${node_ip}, ${proc_param} = $out2"
                    [ "$out1" != "$out2" ] && is_sync=false && break
@@ -1857,7 +1863,6 @@ run_test 26 "test transferring very large nodemap"
 
 nodemap_exercise_fileset() {
        local nm="$1"
-       local fileset_on_mgs=""
        local loop=0
 
        # setup
@@ -1910,12 +1915,12 @@ nodemap_exercise_fileset() {
                error "subdir of fileset not taken into account"
 
        # remove fileset info from nodemap
-       do_facet mgs $LCTL nodemap_set_fileset --name $nm --fileset \'\' ||
+       do_facet mgs $LCTL nodemap_set_fileset --name $nm --fileset clear ||
                error "unable to delete fileset info on $nm nodemap"
        wait_update_facet mgs "$LCTL get_param nodemap.${nm}.fileset" \
                          "nodemap.${nm}.fileset=" ||
                error "fileset info still not cleared on $nm nodemap"
-       do_facet mgs $LCTL set_param -P nodemap.${nm}.fileset=\'\' ||
+       do_facet mgs $LCTL set_param -P nodemap.${nm}.fileset=clear ||
                error "unable to reset fileset info on $nm nodemap"
        wait_nm_sync $nm fileset "nodemap.${nm}.fileset="
 
@@ -1952,7 +1957,10 @@ nodemap_exercise_fileset() {
        fi
 }
 
-test_27() {
+test_27a() {
+       [ $(lustre_version_code $SINGLEMDS) -lt $(version_code 2.10.59) ] &&
+               skip "Need MDS >= 2.10.59" && return
+
        for nm in "default" "c0"; do
                local subdir="subdir_${nm}"
                local subsubdir="subsubdir_${nm}"
@@ -1961,7 +1969,46 @@ test_27() {
                nodemap_exercise_fileset "$nm"
        done
 }
-run_test 27 "test fileset in various nodemaps"
+run_test 27a "test fileset in various nodemaps"
+
+test_27b() { #LU-10703
+       [ $(lustre_version_code $SINGLEMDS) -lt $(version_code 2.10.59) ] &&
+               skip "Need MDS >= 2.10.59" && return
+       [[ $MDSCOUNT -lt 2 ]] && skip "needs >= 2 MDTs" && return
+
+       nodemap_test_setup
+       trap nodemap_test_cleanup EXIT
+
+       # Add the nodemaps and set their filesets
+       for i in $(seq 1 $MDSCOUNT); do
+               do_facet mgs $LCTL nodemap_del nm$i 2>/dev/null
+               do_facet mgs $LCTL nodemap_add nm$i ||
+                       error "add nodemap nm$i failed"
+               wait_nm_sync nm$i "" "" "-N"
+
+               if ! combined_mgs_mds; then
+                       do_facet mgs \
+                               $LCTL set_param nodemap.nm$i.fileset=/dir$i ||
+                               error "set nm$i.fileset=/dir$i failed on MGS"
+               fi
+               do_facet mgs $LCTL set_param -P nodemap.nm$i.fileset=/dir$i ||
+                       error "set nm$i.fileset=/dir$i failed on servers"
+               wait_nm_sync nm$i fileset "nodemap.nm$i.fileset=/dir$i"
+       done
+
+       # Check if all the filesets are correct
+       for i in $(seq 1 $MDSCOUNT); do
+               fileset=$(do_facet mds$i \
+                         $LCTL get_param -n nodemap.nm$i.fileset)
+               [ "$fileset" = "/dir$i" ] ||
+                       error "nm$i.fileset $fileset != /dir$i on mds$i"
+               do_facet mgs $LCTL nodemap_del nm$i ||
+                       error "delete nodemap nm$i failed"
+       done
+
+       nodemap_test_cleanup
+}
+run_test 27b "The new nodemap won't clear the old nodemap's fileset"
 
 test_28() {
        if ! $SHARED_KEY; then