LU-17431 nodemap: modify all properties of a dynamic nm

All properties of a dynamic nodemap must be taken into account. So
move the check for dynamic in nodemap_idx_* functions, except for
nodemap_activate which is independent of any nodemap.
Update sanity-sec test_72 to correctly exercise this.

Fixes: db818bd040 ("LU-17431 nodemap: allow modifying properties of a dynamic nm")
Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
Change-Id: I5fb7877669fdf7689b1be1f065006644f0a3f15c
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/58088
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Marc Vef <mvef@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/include/lustre_nodemap.h b/lustre/include/lustre_nodemap.h
index 54c0add..643fa08 100644 (file)
--- a/lustre/include/lustre_nodemap.h
+++ b/lustre/include/lustre_nodemap.h
@@ -135,7 +135,7 @@ struct nm_config_file {
        struct list_head                 ncf_list;
 };
 
-void nodemap_activate(const bool value);
+int nodemap_activate(const bool value);
 int nodemap_add(const char *nodemap_name, bool dynamic);
 int nodemap_del(const char *nodemap_name);
 int nodemap_add_member(struct lnet_nid *nid, struct obd_export *exp);

diff --git a/lustre/ptlrpc/nodemap_handler.c b/lustre/ptlrpc/nodemap_handler.c
index 20fa97d..adf474d 100644 (file)
--- a/lustre/ptlrpc/nodemap_handler.c
+++ b/lustre/ptlrpc/nodemap_handler.c
@@ -550,8 +550,7 @@ int nodemap_add_idmap_helper(struct lu_nodemap *nodemap,
 
                del_map[0] = temp->id_client;
                idmap_delete(id_type, temp, nodemap);
-               if (!nodemap->nm_dyn)
-                       rc = nodemap_idx_idmap_del(nodemap, id_type, del_map);
+               rc = nodemap_idx_idmap_del(nodemap, id_type, del_map);
                /* In case there is any corrupted idmap */
                if (!rc || unlikely(rc == -ENOENT)) {
                        temp = idmap_insert(id_type, idmap, nodemap);
@@ -610,7 +609,7 @@ int nodemap_add_idmap(const char *nodemap_name, enum nodemap_id_type id_type,
                GOTO(out_unlock, rc = -EPERM);
 
        rc = nodemap_add_idmap_helper(nodemap, id_type, map);
-       if (!rc && !nodemap->nm_dyn)
+       if (!rc)
                rc = nodemap_idx_idmap_add(nodemap, id_type, map);
 
 out_unlock:
@@ -661,8 +660,7 @@ int nodemap_del_idmap(const char *nodemap_name, enum nodemap_id_type id_type,
                rc = -EINVAL;
        } else {
                idmap_delete(id_type, idmap, nodemap);
-               if (!nodemap->nm_dyn)
-                       rc = nodemap_idx_idmap_del(nodemap, id_type, map);
+               rc = nodemap_idx_idmap_del(nodemap, id_type, map);
        }
        up_write(&nodemap->nm_idmap_lock);
 
@@ -1057,8 +1055,8 @@ int nodemap_add_range_helper(struct nodemap_config *config,
        up_write(&config->nmc_range_tree_lock);
 
        /* if range_id is non-zero, we are loading from disk */
-       if (range_id == 0 && !nodemap->nm_dyn)
-               rc = nodemap_idx_range_add(range);
+       if (range_id == 0)
+               rc = nodemap_idx_range_add(nodemap, range);
 
        if (config == active_config) {
                nm_member_revoke_locks(config->nmc_default_nodemap);
@@ -1111,9 +1109,9 @@ EXPORT_SYMBOL(nodemap_add_range);
 int nodemap_del_range(const char *name, const struct lnet_nid nid[2],
                      u8 netmask)
 {
-       struct lu_nodemap       *nodemap;
-       struct lu_nid_range     *range;
-       int                     rc = 0;
+       struct lu_nodemap *nodemap;
+       struct lu_nid_range *range;
+       int rc = 0;
 
        mutex_lock(&active_config_lock);
        nodemap = nodemap_lookup(name);
@@ -1138,8 +1136,7 @@ int nodemap_del_range(const char *name, const struct lnet_nid nid[2],
                up_write(&active_config->nmc_range_tree_lock);
                GOTO(out_putref, rc = -EINVAL);
        }
-       if (!nodemap->nm_dyn)
-               rc = nodemap_idx_range_del(range);
+       rc = nodemap_idx_range_del(nodemap, range);
        range_delete(active_config, range);
        nm_member_reclassify_nodemap(nodemap);
        up_write(&active_config->nmc_range_tree_lock);
@@ -2070,8 +2067,7 @@ int nodemap_del(const char *nodemap_name)
        down_write(&active_config->nmc_range_tree_lock);
        list_for_each_entry_safe(range, range_temp, &nodemap->nm_ranges,
                                 rn_list) {
-               if (!nodemap->nm_dyn)
-                       rc2 = nodemap_idx_range_del(range);
+               rc2 = nodemap_idx_range_del(nodemap, range);
                if (rc2 < 0)
                        rc = rc2;
 
@@ -2087,11 +2083,9 @@ int nodemap_del(const char *nodemap_name)
                nodemap_fileset_reset(nodemap);
        }
 
-       if (!nodemap->nm_dyn) {
-               rc2 = nodemap_idx_nodemap_del(nodemap);
-               if (rc2 < 0)
-                       rc = rc2;
-       }
+       rc2 = nodemap_idx_nodemap_del(nodemap);
+       if (rc2 < 0)
+               rc = rc2;
 
        /*
         * remove procfs here in case nodemap_create called with same name
@@ -2319,16 +2313,25 @@ out:
  *
  * \param      value           1 for on, 0 for off
  */
-void nodemap_activate(const bool value)
+int nodemap_activate(const bool value)
 {
+       int rc = 0;
+
+       if (!nodemap_mgs()) {
+               CERROR("cannot activate for non-existing MGS.\n");
+               return -EINVAL;
+       }
+
        mutex_lock(&active_config_lock);
        active_config->nmc_nodemap_is_active = value;
 
        /* copy active value to global to avoid locking in map functions */
        nodemap_active = value;
-       nodemap_idx_nodemap_activate(value);
+       rc = nodemap_idx_nodemap_activate(value);
        mutex_unlock(&active_config_lock);
        nm_member_revoke_all();
+
+       return rc;
 }
 EXPORT_SYMBOL(nodemap_activate);
 
@@ -2887,14 +2890,14 @@ int server_iocontrol_nodemap(struct obd_device *obd,
                    strcasecmp(param, "y") == 0 ||
                    strcasecmp(param, "true") == 0 ||
                    strcasecmp(param, "t") == 0)
-                       nodemap_activate(1);
+                       rc = nodemap_activate(1);
                else if (strcmp(param, "0") == 0 ||
                         strcasecmp(param, "off") == 0 ||
                         strcasecmp(param, "no") == 0 ||
                         strcasecmp(param, "n") == 0 ||
                         strcasecmp(param, "false") == 0 ||
                         strcasecmp(param, "f") == 0)
-                       nodemap_activate(0);
+                       rc = nodemap_activate(0);
                else
                        rc = -EINVAL;
                break;

diff --git a/lustre/ptlrpc/nodemap_internal.h b/lustre/ptlrpc/nodemap_internal.h
index 51205f7..65b5d8c 100644 (file)
--- a/lustre/ptlrpc/nodemap_internal.h
+++ b/lustre/ptlrpc/nodemap_internal.h
@@ -198,8 +198,10 @@ int nodemap_idx_idmap_add(const struct lu_nodemap *nodemap,
 int nodemap_idx_idmap_del(const struct lu_nodemap *nodemap,
                          enum nodemap_id_type id_type,
                          const __u32 map[2]);
-int nodemap_idx_range_add(const struct lu_nid_range *range);
-int nodemap_idx_range_del(const struct lu_nid_range *range);
+int nodemap_idx_range_add(struct lu_nodemap *nodemap,
+                         const struct lu_nid_range *range);
+int nodemap_idx_range_del(struct lu_nodemap *nodemap,
+                         const struct lu_nid_range *range);
 int nodemap_idx_nodemap_activate(bool value);
 int nodemap_index_read(struct lu_env *env, struct nm_config_file *ncf,
                       struct idx_info *ii, const struct lu_rdpg *rdpg);

diff --git a/lustre/ptlrpc/nodemap_lproc.c b/lustre/ptlrpc/nodemap_lproc.c
index 09bfd0c..33a89bc 100644 (file)
--- a/lustre/ptlrpc/nodemap_lproc.c
+++ b/lustre/ptlrpc/nodemap_lproc.c
@@ -435,9 +435,9 @@ static ssize_t
 nodemap_active_seq_write(struct file *file, const char __user *buffer,
                         size_t count, loff_t *off)
 {
-       char                    active_string[NODEMAP_LDEBUGFS_FLAG_LEN + 1];
-       long unsigned int       active;
-       int                     rc;
+       char active_string[NODEMAP_LDEBUGFS_FLAG_LEN + 1];
+       unsigned long active;
+       int rc;
 
        if (count == 0)
                return 0;
@@ -453,9 +453,9 @@ nodemap_active_seq_write(struct file *file, const char __user *buffer,
        if (rc != 0)
                return -EINVAL;
 
-       nodemap_activate(active);
+       rc = nodemap_activate(active);
 
-       return count;
+       return rc ? rc : count;
 }
 LDEBUGFS_SEQ_FOPS(nodemap_active);
 

diff --git a/lustre/ptlrpc/nodemap_storage.c b/lustre/ptlrpc/nodemap_storage.c
index b65d600..906600e 100644 (file)
--- a/lustre/ptlrpc/nodemap_storage.c
+++ b/lustre/ptlrpc/nodemap_storage.c
@@ -596,18 +596,21 @@ int nodemap_idx_nodemap_update(const struct lu_nodemap *nodemap)
 
 int nodemap_idx_nodemap_del(const struct lu_nodemap *nodemap)
 {
-       struct rb_root           root;
-       struct lu_idmap         *idmap;
-       struct lu_idmap         *temp;
-       struct lu_nid_range     *range;
-       struct lu_nid_range     *range_temp;
-       struct nodemap_key       nk;
-       struct lu_env            env;
-       int                      rc = 0;
-       int                      rc2 = 0;
+       struct rb_root root;
+       struct lu_idmap *idmap;
+       struct lu_idmap *temp;
+       struct lu_nid_range *range;
+       struct lu_nid_range *range_temp;
+       struct nodemap_key nk;
+       struct lu_env env;
+       int rc = 0;
+       int rc2 = 0;
 
        ENTRY;
 
+       if (nodemap->nm_dyn)
+               return 0;
+
        if (!nodemap_mgs()) {
                CERROR("cannot del nodemap config from non-existing MGS.\n");
                return -EINVAL;
@@ -703,6 +706,9 @@ int nodemap_idx_cluster_roles_del(const struct lu_nodemap *nodemap)
 
        ENTRY;
 
+       if (nodemap->nm_dyn)
+               return 0;
+
        if (!nodemap_mgs()) {
                CERROR("cannot add nodemap config to non-existing MGS.\n");
                return -EINVAL;
@@ -733,6 +739,9 @@ int nodemap_idx_offset_del(const struct lu_nodemap *nodemap)
 
        ENTRY;
 
+       if (nodemap->nm_dyn)
+               return 0;
+
        if (!nodemap_mgs()) {
                CERROR("cannot add nodemap config to non-existing MGS.\n");
                return -EINVAL;
@@ -1167,7 +1176,8 @@ int nodemap_idx_fileset_clear(const struct lu_nodemap *nodemap)
        return rc;
 }
 
-int nodemap_idx_range_add(const struct lu_nid_range *range)
+int nodemap_idx_range_add(struct lu_nodemap *nodemap,
+                         const struct lu_nid_range *range)
 {
        struct nodemap_key nk;
        union nodemap_rec nr;
@@ -1175,6 +1185,10 @@ int nodemap_idx_range_add(const struct lu_nid_range *range)
        int rc = 0;
 
        ENTRY;
+
+       if (nodemap->nm_dyn)
+               return 0;
+
        if (!nodemap_mgs()) {
                CERROR("cannot add nodemap config to non-existing MGS.\n");
                return -EINVAL;
@@ -1198,13 +1212,18 @@ free_env:
        RETURN(rc);
 }
 
-int nodemap_idx_range_del(const struct lu_nid_range *range)
+int nodemap_idx_range_del(struct lu_nodemap *nodemap,
+                         const struct lu_nid_range *range)
 {
-       struct nodemap_key       nk;
-       struct lu_env            env;
-       int                      rc = 0;
+       struct nodemap_key nk;
+       struct lu_env env;
+       int rc = 0;
+
        ENTRY;
 
+       if (nodemap->nm_dyn)
+               return 0;
+
        if (!nodemap_mgs()) {
                CERROR("cannot del nodemap config from non-existing MGS.\n");
                return -EINVAL;
@@ -1227,12 +1246,16 @@ int nodemap_idx_idmap_add(const struct lu_nodemap *nodemap,
                          enum nodemap_id_type id_type,
                          const u32 map[2])
 {
-       struct nodemap_key       nk;
-       union nodemap_rec        nr;
-       struct lu_env            env;
-       int                      rc = 0;
+       struct nodemap_key nk;
+       union nodemap_rec nr;
+       struct lu_env env;
+       int rc = 0;
+
        ENTRY;
 
+       if (nodemap->nm_dyn)
+               return 0;
+
        if (!nodemap_mgs()) {
                CERROR("cannot add idmap to non-existing MGS.\n");
                return -EINVAL;
@@ -1255,11 +1278,15 @@ int nodemap_idx_idmap_del(const struct lu_nodemap *nodemap,
                          enum nodemap_id_type id_type,
                          const u32 map[2])
 {
-       struct nodemap_key       nk;
-       struct lu_env            env;
-       int                      rc = 0;
+       struct nodemap_key nk;
+       struct lu_env env;
+       int rc = 0;
+
        ENTRY;
 
+       if (nodemap->nm_dyn)
+               return 0;
+
        if (!nodemap_mgs()) {
                CERROR("cannot del idmap from non-existing MGS.\n");
                return -EINVAL;
@@ -1279,14 +1306,15 @@ int nodemap_idx_idmap_del(const struct lu_nodemap *nodemap,
 
 static int nodemap_idx_global_add_update(bool value, enum nm_add_update update)
 {
-       struct nodemap_key       nk;
-       union nodemap_rec        nr;
-       struct lu_env            env;
-       int                      rc = 0;
+       struct nodemap_key nk;
+       union nodemap_rec nr;
+       struct lu_env env;
+       int rc = 0;
+
        ENTRY;
 
        if (!nodemap_mgs()) {
-               CERROR("cannot add nodemap config to non-existing MGS.\n");
+               CERROR("cannot do global for non-existing MGS.\n");
                return -EINVAL;
        }
 

diff --git a/lustre/tests/sanity-sec.sh b/lustre/tests/sanity-sec.sh
index 75857d0..98d18c8 100755 (executable)
--- a/lustre/tests/sanity-sec.sh
+++ b/lustre/tests/sanity-sec.sh
@@ -7114,6 +7114,7 @@ test_72() {
        local properties="audit_mode deny_unknown forbid_encryption \
                          readonly_mount"
        local sepol="1:mls:31:40afb76d077c441b69af58cccaaa2ca63641ed6e21b0a887dc21a684f508b78f"
+       local rbac_val
        local val
 
        (( OST1_VERSION >= $(version_code 2.15.64) )) ||
@@ -7139,6 +7140,8 @@ test_72() {
                error "add_idmap for $mgsnm on MGS failed"
        wait_nm_sync $mgsnm idmap
 
+       rbac_val=$(do_facet mgs $LCTL get_param -n nodemap.$mgsnm.rbac)
+
        stack_trap "do_facet ost1 $LCTL nodemap_del $nm || true" EXIT
        do_facet ost1 $LCTL nodemap_add $nm &&
                error "static nodemap on server should fail"
@@ -7207,6 +7210,11 @@ test_72() {
                        error "dynamic modify of $prop failed"
        val=$(do_facet ost1 $LCTL get_param -n nodemap.$nm.$prop)
        [[ "x$val" == "xfile_perms" ]] || error "incorrect $prop $val"
+       do_facet ost1 $LCTL nodemap_modify --name $nm \
+               --property $prop --value all ||
+                       error "dynamic modify of $prop failed"
+       val=$(do_facet ost1 $LCTL get_param -n nodemap.$nm.$prop)
+       [[ "x$val" == "x$rbac_val" ]] || error "incorrect $prop $val"
        prop=squash_uid
        do_facet ost1 $LCTL nodemap_modify --name $nm \
                --property $prop --value 77 ||
@@ -7237,6 +7245,24 @@ test_72() {
                        error "dynamic modify of $prop failed"
        val=$(do_facet ost1 $LCTL get_param -n nodemap.$nm.$prop)
        [[ "x$val" == "x$sepol" ]] || error "incorrect $prop $val"
+       prop=offset
+       do_facet ost1 $LCTL nodemap_add_offset --name $nm \
+               --offset 100000 --limit 200000 ||
+                       error "dynamic modify of $prop failed"
+       val=$(do_facet ost1 $LCTL get_param -n nodemap.$nm.$prop |
+               awk '$1 == "start_uid:" {print $2}' | sed s+,++)
+       [[ "x$val" == "x100000" ]] || error "incorrect $prop start_uid $val"
+       val=$(do_facet ost1 $LCTL get_param -n nodemap.$nm.$prop |
+               awk '$1 == "limit_uid:" {print $2}' | sed s+,++)
+       [[ "x$val" == "x200000" ]] || error "incorrect $prop limit_uid $val"
+       do_facet ost1 $LCTL nodemap_del_offset --name $nm ||
+                       error "dynamic del of $prop failed"
+       val=$(do_facet ost1 $LCTL get_param -n nodemap.$nm.$prop |
+               awk '$1 == "start_uid:" {print $2}' | sed s+,++)
+       [[ "x$val" == "x0" ]] || error "incorrect $prop start_uid $val"
+       val=$(do_facet ost1 $LCTL get_param -n nodemap.$nm.$prop |
+               awk '$1 == "limit_uid:" {print $2}' | sed s+,++)
+       [[ "x$val" == "x0" ]] || error "incorrect $prop limit_uid $val"
 
        val=$(do_facet ost1 $LCTL nodemap_test_id --nid $startnid \
                --idtype uid --id $clid)