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 <rhenwood@whamcloud.com>
Change-Id: I39e355b28fb1337f5b3a53f9e7265f4e969ddd2d
Reviewed-on: http://review.whamcloud.com/1765
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: John Hammond <jhammond@tacc.utexas.edu>
Tested-by: Hudson
Tested-by: Maloo <whamcloud.maloo@gmail.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/doc/lctl.8 b/lustre/doc/lctl.8
index 53ea4ee..dca672d 100644 (file)
--- 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 
 .PP
 .SS Device Operations
 .TP 

-.BI list_param " [-F|-R] <param_path ...>"
+.BI list_param " [-F|-R] <param_search ...>"

 List the Lustre or LNet parameter name
 List the Lustre or LNet parameter name

-.br

 .B -F
 Add '/', '@' or '=' for dirs, symlinks and writeable files, respectively.
 .br
 .B -R
 .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:
 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
 .br
   debug=
 .br

-.B     
+.B

 # lctl list_param -R mdt
 .br
   mdt
 # lctl list_param -R mdt
 .br
   mdt


@@ -142,8 +141,8 @@ is unspecified, all the parameters will be shown.
 .br
   ...
 .TP
 .br
   ...
 .TP

-.BI get_param " [-n|-N|-F] <param_path ...>"
-Get the value of Lustre or LNET parameter from the specified path.
+.BI get_param " [-n|-N|-F] <parameter ...>"
+Get the value of Lustre or LNET parameter.

 .br
 .B -n
 Print only the value and not parameter name.
 .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
 .br
 lctl "get_param -NF" is equivalent to "list_param -F".
 .TP

-.BI set_param " [-n] <param_path=value ...>"
-Set the value of Lustre or LNET parameter from the specified path.
+.BI set_param " [-n] <parameter=value ...>"
+Set the value of Lustre or LNET parameter.

 .br
 .B -n
 Disable printing of the key name when printing values.
 .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 (file)
--- a/lustre/tests/sanity.sh
+++ b/lustre/tests/sanity.sh
@@ -4192,7 +4192,7 @@ set_checksums()
        return 0
 }
 
        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"
                         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
         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"
 
         [ $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 (file)
--- 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;
 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 jt_lcfg_listparam(int argc, char **argv)
 {

-        int fp;

         int rc = 0, i;
         struct param_opts popt;
         char pattern[PATH_MAX];
         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);
 
 
                 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)
 
                 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 jt_lcfg_getparam(int argc, char **argv)
 {

-        int fp;

         int rc = 0, i;
         struct param_opts popt;
         char pattern[PATH_MAX];
         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);
 
 
                 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);
 
                 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 jt_lcfg_setparam(int argc, char **argv)
 {

-        int fp;

         int rc = 0, i;
         struct param_opts popt;
         char pattern[PATH_MAX];
         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);
 
 
                 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;
 
                 rc = setparam_display(&popt, pattern, value);
                 path = NULL;


@@ -1026,3 +1023,4 @@ int jt_lcfg_setparam(int argc, char **argv)
 
         return 0;
 }
 
         return 0;
 }

+