From 51ed079112c9a9eeac37eaf680637a4d04dd3cc9 Mon Sep 17 00:00:00 2001 From: Richard Henwood Date: Fri, 2 Dec 2011 21:44:45 -0500 Subject: [PATCH] LU-888 lctl: remove perilous lctl {get,set,list}_param behavior. This patch stops the {get,set)_param from potentially reading and writing to file in the current working directory. Now lctl {get,set,list}_param visits /proc/{fs,sys}/{lnet,lustre} for parameter values. Specifying a file path to {get,set,list}_param is deprecated behavoir. Using a path with lctl {get,set,list}_param fails if the path does not begin with '/proc/'. If the path begins with '/proc/' a warning is printed the command executes using the given path. A new helper function lprocfs_param_pattern is introduced to provide checking and constructing the proc path. Test suite has been searched for {get,set,list}_param lctl calls. All specified parameters have been checked to ensure they are not using the deprecated path interface. lctl man page is updated to remove ambiguity around using paths to specifiy parameters. Signed-off-by: Richard Henwood Change-Id: I39e355b28fb1337f5b3a53f9e7265f4e969ddd2d Reviewed-on: http://review.whamcloud.com/1765 Reviewed-by: Andreas Dilger Reviewed-by: John Hammond Tested-by: Hudson Tested-by: Maloo Reviewed-by: Oleg Drokin --- lustre/doc/lctl.8 | 17 +++++++------- lustre/tests/sanity.sh | 4 ++-- lustre/utils/lustre_cfg.c | 58 +++++++++++++++++++++++------------------------ 3 files changed, 38 insertions(+), 41 deletions(-) diff --git a/lustre/doc/lctl.8 b/lustre/doc/lctl.8 index 53ea4ee..dca672d 100644 --- a/lustre/doc/lctl.8 +++ b/lustre/doc/lctl.8 @@ -92,15 +92,14 @@ Show all the local Lustre OBDs. AKA .PP .SS Device Operations .TP -.BI list_param " [-F|-R] " +.BI list_param " [-F|-R] " List the Lustre or LNet parameter name -.br .B -F Add '/', '@' or '=' for dirs, symlinks and writeable files, respectively. .br .B -R -Recursively list all parameters under the specified path. If -.I param_path +Recursively list all parameters under the specified parameter search string. If +.I param_search is unspecified, all the parameters will be shown. .br .B Examples: @@ -121,7 +120,7 @@ is unspecified, all the parameters will be shown. .br debug= .br -.B +.B # lctl list_param -R mdt .br mdt @@ -142,8 +141,8 @@ is unspecified, all the parameters will be shown. .br ... .TP -.BI get_param " [-n|-N|-F] " -Get the value of Lustre or LNET parameter from the specified path. +.BI get_param " [-n|-N|-F] " +Get the value of Lustre or LNET parameter. .br .B -n Print only the value and not parameter name. @@ -181,8 +180,8 @@ When -N specified, add '/', '@' or '=' for directories, symlinks and writeable f .br lctl "get_param -NF" is equivalent to "list_param -F". .TP -.BI set_param " [-n] " -Set the value of Lustre or LNET parameter from the specified path. +.BI set_param " [-n] " +Set the value of Lustre or LNET parameter. .br .B -n Disable printing of the key name when printing values. diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index 23e4f8f..f1235f1 100644 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -4192,7 +4192,7 @@ set_checksums() return 0 } -export ORIG_CSUM_TYPE="`lctl get_param -n osc/*osc-[^mM]*/checksum_type | +export ORIG_CSUM_TYPE="`lctl get_param -n osc.*osc-[^mM]*.checksum_type | sed 's/.*\[\(.*\)\].*/\1/g' | head -n1`" CKSUM_TYPES=${CKSUM_TYPES:-"crc32 adler"} [ "$ORIG_CSUM_TYPE" = "crc32c" ] && CKSUM_TYPES="$CKSUM_TYPES crc32c" @@ -6176,7 +6176,7 @@ test_124a() { done echo "" lctl set_param -n $NSDIR.pool.lock_volume_factor $OLD_LVF - local LRU_SIZE_A=`lctl get_param -n $NSDIR/lru_size` + local LRU_SIZE_A=`lctl get_param -n $NSDIR.lru_size` [ $LRU_SIZE_B -gt $LRU_SIZE_A ] || { error "No locks dropped in ${SLEEP}s. LRU size: $LRU_SIZE_A" diff --git a/lustre/utils/lustre_cfg.c b/lustre/utils/lustre_cfg.c index 2113286..f034eac 100644 --- a/lustre/utils/lustre_cfg.c +++ b/lustre/utils/lustre_cfg.c @@ -659,6 +659,30 @@ static void clean_path(char *path) } } +/* Supporting file paths creates perilous behavoir: LU-888. + * Path support is deprecated. + * If a path is supplied it must begin with /proc. */ +static void lprocfs_param_pattern(const char *cmd, const char *path, char *buf, + size_t buf_size) +{ + /* test path to see if it begins with '/proc/' */ + if (strncmp(path, "/proc/", strlen("/proc/")) == 0) { + static int warned; + if (!warned) { + fprintf(stderr, "%s: specifying parameters via " + "full paths is deprecated.\n", cmd); +#if LUSTRE_VERSION_CODE >= OBD_OCD_VERSION(2,6,50,0) +#warning "remove deprecated full path tunable access" +#endif + warned = 1; + } + snprintf(buf, buf_size, "%s", path); + } else { + snprintf(buf, buf_size, "/proc/{fs,sys}/{lnet,lustre}/%s", + path); + } +} + struct param_opts { int only_path:1; int show_path:1; @@ -735,7 +759,6 @@ static int listparam_display(struct param_opts *popt, char *pattern) int jt_lcfg_listparam(int argc, char **argv) { - int fp; int rc = 0, i; struct param_opts popt; char pattern[PATH_MAX]; @@ -754,15 +777,7 @@ int jt_lcfg_listparam(int argc, char **argv) clean_path(path); - /* If the entire path is specified as input */ - fp = open(path, O_RDONLY); - if (fp < 0) { - snprintf(pattern, PATH_MAX, "/proc/{fs,sys}/{lnet,lustre}/%s", - path); - } else { - strcpy(pattern, path); - close(fp); - } + lprocfs_param_pattern(argv[0], path, pattern, sizeof(pattern)); rc = listparam_display(&popt, pattern); if (rc < 0) @@ -876,7 +891,6 @@ static int getparam_display(struct param_opts *popt, char *pattern) int jt_lcfg_getparam(int argc, char **argv) { - int fp; int rc = 0, i; struct param_opts popt; char pattern[PATH_MAX]; @@ -891,15 +905,7 @@ int jt_lcfg_getparam(int argc, char **argv) clean_path(path); - /* If the entire path is specified as input */ - fp = open(path, O_RDONLY); - if (fp < 0) { - snprintf(pattern, PATH_MAX, "/proc/{fs,sys}/{lnet,lustre}/%s", - path); - } else { - strcpy(pattern, path); - close(fp); - } + lprocfs_param_pattern(argv[0], path, pattern, sizeof(pattern)); if (popt.only_path) rc = listparam_display(&popt, pattern); @@ -979,7 +985,6 @@ static int setparam_display(struct param_opts *popt, char *pattern, char *value) int jt_lcfg_setparam(int argc, char **argv) { - int fp; int rc = 0, i; struct param_opts popt; char pattern[PATH_MAX]; @@ -1007,15 +1012,7 @@ int jt_lcfg_setparam(int argc, char **argv) clean_path(path); - /* If the entire path is specified as input */ - fp = open(path, O_RDONLY); - if (fp < 0) { - snprintf(pattern, PATH_MAX, "/proc/{fs,sys}/{lnet,lustre}/%s", - path); - } else { - strcpy(pattern, path); - close(fp); - } + lprocfs_param_pattern(argv[0], path, pattern, sizeof(pattern)); rc = setparam_display(&popt, pattern, value); path = NULL; @@ -1026,3 +1023,4 @@ int jt_lcfg_setparam(int argc, char **argv) return 0; } + -- 1.8.3.1