From 976a14cd8c5c805b10294a921eec7a943d80bf47 Mon Sep 17 00:00:00 2001 From: Sebastien Buisson Date: Fri, 14 Feb 2025 17:41:54 +0100 Subject: [PATCH] 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 Change-Id: I5fb7877669fdf7689b1be1f065006644f0a3f15c Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/58088 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Marc Vef Reviewed-by: Andreas Dilger Reviewed-by: Oleg Drokin --- lustre/include/lustre_nodemap.h | 2 +- lustre/ptlrpc/nodemap_handler.c | 49 ++++++++++++------------ lustre/ptlrpc/nodemap_internal.h | 6 ++- lustre/ptlrpc/nodemap_lproc.c | 10 ++--- lustre/ptlrpc/nodemap_storage.c | 80 +++++++++++++++++++++++++++------------- lustre/tests/sanity-sec.sh | 26 +++++++++++++ 6 files changed, 116 insertions(+), 57 deletions(-) diff --git a/lustre/include/lustre_nodemap.h b/lustre/include/lustre_nodemap.h index 54c0add..643fa085a 100644 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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) -- 1.8.3.1