From 29cfdc2f1e7a17be7605338cd5d12abd0314fbe5 Mon Sep 17 00:00:00 2001 From: Alexander Boyko Date: Fri, 2 Sep 2022 04:41:32 -0400 Subject: [PATCH] LU-16134 utils: adds lctl apply_yaml Commnad set_param -F is used to parse yaml file with settings, and makes set_param -P for it. Many type of settings are based on a specific device and have conf_param type. When such settings goes to set_param -P, all nodes tries to apply it and many errors happen. systemd-udevd[568906]: Process '/usr/sbin/lctl set_param 'osp.kjcf04-OST0003-osc-MDT0000.resend_count=43'' failed with exit code 2. The patch adds functionality for conf_param event of YAML file, and introduces lctl apply_yaml for both types of event. YAML example - {device: testfs-MDT0001, event: conf_param, index: 76, parameter: lod.qos_threshold_rr=100} - { index: 17, event: set_param, device: general, parameter: jobid_var, value: procname_uid } Test-Parameters: trivial HPE-bug-id: LUS-11116 Signed-off-by: Alexander Boyko Change-Id: Iec3b1f14b9ddb85ef3e110bbc4467d0d6c80c136 Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/48419 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Andriy Skulysh Reviewed-by: Oleg Drokin Reviewed-by: Artem Blagodarenko --- lustre/tests/conf-sanity.sh | 48 +++++++++++++++---- lustre/utils/lctl.c | 3 ++ lustre/utils/lustre_cfg.c | 111 ++++++++++++++++++++++++++++++++------------ lustre/utils/obdctl.h | 1 + 4 files changed, 125 insertions(+), 38 deletions(-) diff --git a/lustre/tests/conf-sanity.sh b/lustre/tests/conf-sanity.sh index eaeae51..41518f6 100644 --- a/lustre/tests/conf-sanity.sh +++ b/lustre/tests/conf-sanity.sh @@ -9897,18 +9897,18 @@ test_123ai() { #LU-16167 } run_test 123ai "llog_print display all non skipped records" -test_123F() { +test_123_prep() { remote_mgs_nodsh && skip "remote MGS with nodsh" [ -d $MOUNT/.lustre ] || setup - local yaml_file="$TMP/$tfile.yaml" + yaml_file="$TMP/$tfile.yaml" do_facet mgs rm "$yaml_file" - local cfgfiles=$(do_facet mgs "lctl --device MGS llog_catlist" | + cfgfiles=$(do_facet mgs "lctl --device MGS llog_catlist" | sed 's/config_log://') # set jobid_var to a different value for test - local orig_val=$(do_facet mgs $LCTL get_param jobid_var) + orig_val=$(do_facet mgs $LCTL get_param jobid_var) do_facet mgs $LCTL set_param -P jobid_var="TESTNAME" @@ -9922,12 +9922,10 @@ test_123F() { writeconf_all echo "Remounting" setup_noconfig +} - # Reapply the config from before - echo "Setting configuration parameters" - do_facet mgs "lctl set_param -F $yaml_file" - - local set_val=$(do_facet mgs $LCTL get_param jobid_var) +test_123_restore() { + set_val=$(do_facet mgs $LCTL get_param jobid_var) do_facet mgs $LCTL set_param -P $orig_val @@ -9937,8 +9935,40 @@ test_123F() { do_facet mgs rm "$yaml_file" cleanup } + +test_123F() { + local yaml_file + local cfgfiles + local orig_val + local set_val + + test_123_prep + + # Reapply the config from before + echo "Setting configuration parameters" + do_facet mgs "lctl set_param -F $yaml_file" + + test_123_restore +} run_test 123F "clear and reset all parameters using set_param -F" +test_123G() { + local yaml_file + local cfgfiles + local orig_val + local set_val + + test_123_prep + + # Reapply the config from before + echo "Setting configuration parameters" + do_facet mgs "lctl apply_yaml $yaml_file" + + test_123_restore +} +run_test 123G "clear and reset all parameters using apply_yaml" + + test_124() { [ $MDSCOUNT -lt 2 ] && skip "needs >= 2 MDTs" diff --git a/lustre/utils/lctl.c b/lustre/utils/lctl.c index 3b4f145..d4db99a 100644 --- a/lustre/utils/lctl.c +++ b/lustre/utils/lctl.c @@ -201,6 +201,9 @@ command_t cmdlist[] = { " -P Set the parameter permanently, filesystem-wide.\n" " -d Remove the permanent setting (only with -P option).\n" " -F Read permanent configuration from a YAML file.\n"}, + {"apply_yaml", jt_lcfg_applyyaml, 0, "set/config the Lustre or LNET " + "parameters using configuration from a YAML file.\n" + "usage: apply_yaml file\n"}, {"list_param", jt_lcfg_listparam, 0, "list the Lustre or LNET parameter name\n" "usage: list_param [-F|-R|-D] \n" diff --git a/lustre/utils/lustre_cfg.c b/lustre/utils/lustre_cfg.c index e6d63a6..ad6c78b 100644 --- a/lustre/utils/lustre_cfg.c +++ b/lustre/utils/lustre_cfg.c @@ -1647,6 +1647,10 @@ static int setparam_cmdline(int argc, char **argv, struct param_opts *popt) return -1; } } + if (popt->po_perm && popt->po_file) { + fprintf(stderr, "warning: ignoring -P option\n"); + popt->po_perm = 0; + } return optind; } @@ -1685,17 +1689,16 @@ static struct cfg_stage_data { { PS_NONE, "none" } }; -void conf_to_set_param(enum paramtype confset, const char *param, - const char *device, char *buf, - int bufsize) +static enum paramtype construct_param(enum paramtype confset, const char *param, + const char *device, char *buf, + int bufsize, bool convert) { char *tmp; if (confset == PT_SETPARAM) { strncpy(buf, param, bufsize); - return; + return confset; } - /* * sys.* params are top level, we just need to trim the sys. */ @@ -1703,28 +1706,42 @@ void conf_to_set_param(enum paramtype confset, const char *param, if (tmp) { tmp += 4; strncpy(buf, tmp, bufsize); - return; + return PT_SETPARAM; } - /* - * parameters look like type.parameter, we need to stick the device - * in the middle. Example combine mdt.identity_upcall with device - * lustre-MDT0000 for mdt.lustre-MDT0000.identity_upcall - */ + if (convert) { + /* + * parameters look like type.parameter, we need to stick the + * device in the middle. Example combine mdt.identity_upcall + * with device lustre-MDT0000 for + * mdt.lustre-MDT0000.identity_upcall + */ + + tmp = strchrnul(param, '.'); + snprintf(buf, tmp - param + 1, "%s", param); + buf += tmp - param; + bufsize -= tmp - param; + snprintf(buf, bufsize, ".%s%s", device, tmp); + return PT_SETPARAM; + } + /* create for conf_param */ + if (strlen(device)) { + int rc; - tmp = strchrnul(param, '.'); - snprintf(buf, tmp - param + 1, "%s", param); - buf += tmp - param; - bufsize -= tmp - param; - snprintf(buf, bufsize, ".%s%s", device, tmp); + rc = snprintf(buf, bufsize, "%s.%s", device, param); + if (rc < 0) + return PT_NONE; + return confset; + } + return PT_NONE; } -int lcfg_setparam_yaml(char *func, char *filename) +int lcfg_apply_param_yaml(char *func, char *filename) { FILE *file; yaml_parser_t parser; yaml_token_t token; - int rc = 0; + int rc = 0, rc1 = 0; enum paramtype confset = PT_NONE; int param = PS_NONE; @@ -1732,7 +1749,9 @@ int lcfg_setparam_yaml(char *func, char *filename) char parameter[PARAM_SZ + 1]; char value[PARAM_SZ + 1]; char device[PARAM_SZ + 1]; + bool convert; + convert = !strncmp(func, "set_param", 9); file = fopen(filename, "rb"); yaml_parser_initialize(&parser); yaml_parser_set_input_file(&parser, file); @@ -1744,7 +1763,7 @@ int lcfg_setparam_yaml(char *func, char *filename) * when we have all 3, create param=val and call the * appropriate function for set/conf param */ - while (token.type != YAML_STREAM_END_TOKEN && rc == 0) { + while (token.type != YAML_STREAM_END_TOKEN) { int i; yaml_token_delete(&token); @@ -1781,9 +1800,15 @@ int lcfg_setparam_yaml(char *func, char *filename) continue; if (param & PS_PARAM_FOUND) { - conf_to_set_param(confset, - (char *)token.data.alias.value, - device, parameter, PARAM_SZ); + enum paramtype rc; + + rc = construct_param(confset, + (char *)token.data.alias.value, + device, parameter, PARAM_SZ, convert); + + if (rc == PT_NONE) + printf("error: conf_param without device\n"); + confset = rc; param |= PS_PARAM_SET; param &= ~PS_PARAM_FOUND; @@ -1822,9 +1847,19 @@ int lcfg_setparam_yaml(char *func, char *filename) } snprintf(buf, size, "%s=%s", parameter, value); - printf("set_param: %s\n", buf); - rc = lcfg_setparam_perm(func, buf); - + printf("%s: %s\n", confset == PT_SETPARAM ? + "set_param" : "conf_param", buf); + + if (confset == PT_SETPARAM) + rc = lcfg_setparam_perm(func, buf); + else + rc = lcfg_conf_param(func, buf); + if (rc) { + printf("error: failed to apply parameter rc = %d, tyring next one\n", + rc); + rc1 = rc; + rc = 0; + } confset = PT_NONE; param = PS_NONE; parameter[0] = '\0'; @@ -1837,7 +1872,19 @@ int lcfg_setparam_yaml(char *func, char *filename) yaml_parser_delete(&parser); fclose(file); - return rc; + return rc1; +} + +int jt_lcfg_applyyaml(int argc, char **argv) +{ + int index; + struct param_opts popt = {0}; + + index = setparam_cmdline(argc, argv, &popt); + if (index < 0 || index >= argc) + return CMD_HELP; + + return lcfg_apply_param_yaml(argv[0], argv[index]); } int jt_lcfg_setparam(int argc, char **argv) @@ -1858,9 +1905,15 @@ int jt_lcfg_setparam(int argc, char **argv) */ return jt_lcfg_setparam_perm(argc, argv, &popt); - if (popt.po_file) - return lcfg_setparam_yaml(argv[0], argv[index]); - + if (popt.po_file) { +#if LUSTRE_VERSION >= OBD_OCD_VERSION(2,17,0,0) + fprintf(stderr, "warning: 'lctl set_param -F' is deprecated, use 'lctl apply_yaml' instead\n"); + return -EINVAL; +#else + printf("This option left for backward compatibility, please use 'lctl apply_yaml' instead\n"); + return lcfg_apply_param_yaml(argv[0], argv[index]); +#endif + } for (i = index; i < argc; i++) { int rc2; diff --git a/lustre/utils/obdctl.h b/lustre/utils/obdctl.h index 8ceeb5a..487370b 100644 --- a/lustre/utils/obdctl.h +++ b/lustre/utils/obdctl.h @@ -154,6 +154,7 @@ int jt_lcfg_param(int argc, char **argv); int jt_lcfg_confparam(int argc, char **argv); int jt_lcfg_getparam(int argc, char **argv); int jt_lcfg_setparam(int argc, char **argv); +int jt_lcfg_applyyaml(int argc, char **argv); int jt_lcfg_listparam(int argc, char **argv); int jt_pool_cmd(int argc, char **argv); -- 1.8.3.1