Whamcloud - gitweb
LU-4221 osd: add case LCFG_PARAM to osd_process_config 38/8238/8
authorEmoly Liu <emoly.liu@intel.com>
Mon, 9 Dec 2013 07:46:04 +0000 (15:46 +0800)
committerOleg Drokin <oleg.drokin@intel.com>
Fri, 27 Dec 2013 18:21:27 +0000 (18:21 +0000)
Some proc parameters were moved from ofd to osd module and only
their symlinks were kept in ofd for interoperability/compatibility.
To process this kind of config params passed by ofd, this patch is
to do the following fixes:
- add case LCFG_PARAM to osd_process_config() to process parameters
  with prefix both PARAM_OSD and PARAM_OST.
- since these parameters are not included by the static lprocfs var
  list, a pre-check is added for them to avoid "unknown param" error
  message confuses the uses. If they are matched in this check, they
  will be passed to the osd directly.
- get rid of lprocfs_osd_init_vars() and use struct lprocfs_vars
  lprocfs_osd_{obd,module}_vars[] instead.
- improve the error messages in class_process_proc_param() and
  class_process_proc_seq_param() a little.
- add conf-sanity.sh test_28a to verify the patch and skip this test
  for ZFS OSTs since ZFS has no such kind of parameters.

Signed-off-by: Emoly Liu <emoly.liu@intel.com>
Change-Id: I8f64dcc29017c3ed9da010ff0a3e40c02327f05c
Reviewed-on: http://review.whamcloud.com/8238
Tested-by: Jenkins
Tested-by: Andreas Dilger <andreas.dilger@intel.com>
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Reviewed-by: James Simmons <uja.ornl@gmail.com>
lustre/include/lustre_param.h
lustre/mgs/mgs_llog.c
lustre/obdclass/obd_config.c
lustre/ofd/ofd_dev.c
lustre/osd-ldiskfs/osd_handler.c
lustre/osd-ldiskfs/osd_internal.h
lustre/osd-ldiskfs/osd_lproc.c
lustre/osd-zfs/osd_handler.c
lustre/tests/conf-sanity.sh

index 87d0f48..7df6194 100644 (file)
@@ -101,6 +101,7 @@ int do_lcfg(char *cfgname, lnet_nid_t nid, int cmd,
 
 /* Prefixes for parameters handled by obd's proc methods (XXX_process_config) */
 #define PARAM_OST              "ost."
+#define PARAM_OSD              "osd."
 #define PARAM_OSC              "osc."
 #define PARAM_MDT              "mdt."
 #define PARAM_HSM              "mdt.hsm."
index b610b86..9b0201f 100644 (file)
@@ -3491,17 +3491,18 @@ static int mgs_write_log_param(const struct lu_env *env,
                 GOTO(end, rc);
         }
 
-        /* All mdd., ost. params in proc */
-        if ((class_match_param(ptr, PARAM_MDD, NULL) == 0) ||
-            (class_match_param(ptr, PARAM_OST, NULL) == 0)) {
-                CDEBUG(D_MGS, "%.3s param %s\n", ptr, ptr + 4);
+       /* All mdd., ost. and osd. params in proc */
+       if ((class_match_param(ptr, PARAM_MDD, NULL) == 0) ||
+           (class_match_param(ptr, PARAM_OST, NULL) == 0) ||
+           (class_match_param(ptr, PARAM_OSD, NULL) == 0)) {
+               CDEBUG(D_MGS, "%.3s param %s\n", ptr, ptr + 4);
                if (mgs_log_is_empty(env, mgs, mti->mti_svname))
-                        GOTO(end, rc = -ENODEV);
+                       GOTO(end, rc = -ENODEV);
 
                rc = mgs_wlp_lcfg(env, mgs, fsdb, mti, mti->mti_svname,
                                  &mgi->mgi_bufs, mti->mti_svname, ptr);
-                GOTO(end, rc);
-        }
+               GOTO(end, rc);
+       }
 
         LCONSOLE_WARN("Ignoring unrecognized param '%s'\n", ptr);
         rc2 = -ENOSYS;
index 47f42b1..286fb8a 100644 (file)
@@ -1302,7 +1302,10 @@ int class_process_proc_param(char *prefix, struct lprocfs_vars *lvars,
         for (i = 1; i < lcfg->lcfg_bufcount; i++) {
                 key = lustre_cfg_buf(lcfg, i);
                 /* Strip off prefix */
-                class_match_param(key, prefix, &key);
+               if (class_match_param(key, prefix, &key))
+                       /* If the prefix doesn't match, return error so we
+                        * can pass it down the stack */
+                       RETURN(-ENOSYS);
                 sval = strchr(key, '=');
                 if (!sval || (*(sval + 1) == 0)) {
                         CERROR("Can't parse param %s (missing '=')\n", key);
@@ -1334,19 +1337,16 @@ int class_process_proc_param(char *prefix, struct lprocfs_vars *lvars,
                         }
                         j++;
                 }
-                if (!matched) {
-                        /* If the prefix doesn't match, return error so we
-                           can pass it down the stack */
-                        if (strnchr(key, keylen, '.'))
-                            RETURN(-ENOSYS);
-                        CERROR("%s: unknown param %s\n",
-                               (char *)lustre_cfg_string(lcfg, 0), key);
-                        /* rc = -EINVAL;        continue parsing other params */
-                        skip++;
-                } else if (rc < 0) {
-                        CERROR("writing proc entry %s err %d\n",
-                               var->name, rc);
-                        rc = 0;
+               if (!matched) {
+                       CERROR("%.*s: %s unknown param %s\n",
+                              (int)strlen(prefix) - 1, prefix,
+                              (char *)lustre_cfg_string(lcfg, 0), key);
+                       /* rc = -EINVAL;        continue parsing other params */
+                       skip++;
+               } else if (rc < 0) {
+                       CERROR("%s: error writing proc entry '%s': rc = %d\n",
+                              prefix, var->name, rc);
+                       rc = 0;
                } else {
                        CDEBUG(D_CONFIG, "%s.%.*s: Set parameter %.*s=%s\n",
                                         lustre_cfg_string(lcfg, 0),
@@ -1397,7 +1397,10 @@ int class_process_proc_seq_param(char *prefix, struct lprocfs_seq_vars *lvars,
        for (i = 1; i < lcfg->lcfg_bufcount; i++) {
                key = lustre_cfg_buf(lcfg, i);
                /* Strip off prefix */
-               class_match_param(key, prefix, &key);
+               if (class_match_param(key, prefix, &key))
+                       /* If the prefix doesn't match, return error so we
+                        * can pass it down the stack */
+                       RETURN(-ENOSYS);
                sval = strchr(key, '=');
                if (!sval || (*(sval + 1) == 0)) {
                        CERROR("Can't parse param %s (missing '=')\n", key);
@@ -1430,17 +1433,14 @@ int class_process_proc_seq_param(char *prefix, struct lprocfs_seq_vars *lvars,
                        j++;
                }
                if (!matched) {
-                       /* If the prefix doesn't match, return error so we
-                          can pass it down the stack */
-                       if (strnchr(key, keylen, '.'))
-                           RETURN(-ENOSYS);
-                       CERROR("%s: unknown param %s\n",
+                       CERROR("%.*s: %s unknown param %s\n",
+                              (int)strlen(prefix) - 1, prefix,
                               (char *)lustre_cfg_string(lcfg, 0), key);
                        /* rc = -EINVAL;        continue parsing other params */
                        skip++;
                } else if (rc < 0) {
-                       CERROR("writing proc entry %s err %d\n",
-                              var->name, rc);
+                       CERROR("%s: error writing proc entry '%s': rc = %d\n",
+                              prefix, var->name, rc);
                        rc = 0;
                } else {
                        CDEBUG(D_CONFIG, "%s.%.*s: Set parameter %.*s=%s\n",
index 2bdbde6..91e28b4 100644 (file)
@@ -193,6 +193,40 @@ static struct cfg_interop_param ofd_interop_param[] = {
        { NULL }
 };
 
+/* Some parameters were moved from ofd to osd and only their
+ * symlinks were kept in ofd by LU-3106. They are:
+ * -writehthrough_cache_enable
+ * -readcache_max_filese
+ * -read_cache_enable
+ * -brw_stats
+ * Since they are not included by the static lprocfs var list,
+ * a pre-check is added for them to avoid "unknown param" error
+ * message confuses the customer. If they are matched in this
+ * check, they will be passed to the osd directly.
+ */
+static bool match_symlink_param(char *param)
+{
+       char *sval;
+       int paramlen;
+
+       if (class_match_param(param, PARAM_OST, &param) == 0) {
+               sval = strchr(param, '=');
+               if (sval != NULL) {
+                       paramlen = sval - param;
+                       if (strncmp(param, "writethrough_cache_enable",
+                                   paramlen) == 0 ||
+                           strncmp(param, "readcache_max_filesize",
+                                   paramlen) == 0 ||
+                           strncmp(param, "read_cache_enable",
+                                   paramlen) == 0 ||
+                           strncmp(param, "brw_stats", paramlen) == 0)
+                               return true;
+               }
+       }
+
+       return false;
+}
+
 /* used by MGS to process specific configurations */
 static int ofd_process_config(const struct lu_env *env, struct lu_device *d,
                              struct lustre_cfg *cfg)
@@ -240,12 +274,20 @@ static int ofd_process_config(const struct lu_env *env, struct lu_device *d,
                        }
                }
 
+               if (match_symlink_param(param)) {
+                       rc = next->ld_ops->ldo_process_config(env, next, cfg);
+                       break;
+               }
+
                lprocfs_ofd_init_vars(&lvars);
                rc = class_process_proc_param(PARAM_OST, lvars.obd_vars, cfg,
                                              d->ld_obd);
-               if (rc > 0 || rc == -ENOSYS)
+               if (rc > 0 || rc == -ENOSYS) {
+                       CDEBUG(D_CONFIG, "pass param %s down the stack.\n",
+                              param);
                        /* we don't understand; pass it on */
                        rc = next->ld_ops->ldo_process_config(env, next, cfg);
+               }
                break;
        }
        case LCFG_SPTLRPC_CONF: {
index c13c682..4644b9d 100644 (file)
@@ -62,6 +62,8 @@
 /* struct ptlrpc_thread */
 #include <lustre_net.h>
 #include <lustre_fid.h>
+/* process_config */
+#include <lustre_param.h>
 
 #include "osd_internal.h"
 #include "osd_dynlocks.h"
@@ -5671,23 +5673,32 @@ static struct lu_device *osd_device_free(const struct lu_env *env,
 static int osd_process_config(const struct lu_env *env,
                               struct lu_device *d, struct lustre_cfg *cfg)
 {
-        struct osd_device *o = osd_dev(d);
-        int err;
-        ENTRY;
+       struct osd_device               *o = osd_dev(d);
+       int                             rc;
+       ENTRY;
 
-        switch(cfg->lcfg_command) {
-        case LCFG_SETUP:
-                err = osd_mount(env, o, cfg);
-                break;
-        case LCFG_CLEANUP:
+       switch (cfg->lcfg_command) {
+       case LCFG_SETUP:
+               rc = osd_mount(env, o, cfg);
+               break;
+       case LCFG_CLEANUP:
                lu_dev_del_linkage(d->ld_site, d);
-               err = osd_shutdown(env, o);
+               rc = osd_shutdown(env, o);
                break;
-        default:
-                err = -ENOSYS;
-        }
+       case LCFG_PARAM:
+               LASSERT(&o->od_dt_dev);
+               rc = class_process_proc_param(PARAM_OSD, lprocfs_osd_obd_vars,
+                                             cfg, &o->od_dt_dev);
+               if (rc > 0 || rc == -ENOSYS)
+                       rc = class_process_proc_param(PARAM_OST,
+                                                     lprocfs_osd_obd_vars,
+                                                     cfg, &o->od_dt_dev);
+               break;
+       default:
+               rc = -ENOSYS;
+       }
 
-        RETURN(err);
+       RETURN(rc);
 }
 
 static int osd_recovery_complete(const struct lu_env *env,
@@ -5820,11 +5831,9 @@ static struct obd_ops osd_obd_device_ops = {
 
 static int __init osd_mod_init(void)
 {
-        struct lprocfs_static_vars lvars;
        int rc;
 
        osd_oi_mod_init();
-       lprocfs_osd_init_vars(&lvars);
 
        rc = lu_kmem_init(ldiskfs_caches);
        if (rc)
@@ -5832,7 +5841,7 @@ static int __init osd_mod_init(void)
 
        rc = class_register_type(&osd_obd_device_ops, NULL, NULL,
 #ifndef HAVE_ONLY_PROCFS_SEQ
-                               lvars.module_vars,
+                               lprocfs_osd_module_vars,
 #endif
                                LUSTRE_OSD_LDISKFS_NAME, &osd_device_type);
        if (rc)
index 8d5ca40..e9dccbc 100644 (file)
@@ -623,7 +623,8 @@ static inline int __osd_xattr_set(struct osd_thread_info *info,
 
 #ifdef LPROCFS
 /* osd_lproc.c */
-void lprocfs_osd_init_vars(struct lprocfs_static_vars *lvars);
+extern struct lprocfs_vars lprocfs_osd_obd_vars[];
+extern struct lprocfs_vars lprocfs_osd_module_vars[];
 int osd_procfs_init(struct osd_device *osd, const char *name);
 int osd_procfs_fini(struct osd_device *osd);
 void osd_brw_stats_update(struct osd_device *osd, struct osd_iobuf *iobuf);
index 83bb586..b9b4e3d 100644 (file)
@@ -239,7 +239,6 @@ out:
 
 int osd_procfs_init(struct osd_device *osd, const char *name)
 {
-        struct lprocfs_static_vars lvars;
         struct obd_type     *type;
         int                  rc;
         ENTRY;
@@ -252,9 +251,9 @@ int osd_procfs_init(struct osd_device *osd, const char *name)
         LASSERT(type != NULL);
 
         /* Find the type procroot and add the proc entry for this device */
-        lprocfs_osd_init_vars(&lvars);
         osd->od_proc_entry = lprocfs_register(name, type->typ_procroot,
-                                              lvars.obd_vars, &osd->od_dt_dev);
+                                             lprocfs_osd_obd_vars,
+                                             &osd->od_dt_dev);
         if (IS_ERR(osd->od_proc_entry)) {
                 rc = PTR_ERR(osd->od_proc_entry);
                 CERROR("Error %d setting up lprocfs for %s\n",
@@ -579,9 +578,4 @@ struct lprocfs_vars lprocfs_osd_module_vars[] = {
         { 0 }
 };
 
-void lprocfs_osd_init_vars(struct lprocfs_static_vars *lvars)
-{
-        lvars->module_vars = lprocfs_osd_module_vars;
-        lvars->obd_vars = lprocfs_osd_obd_vars;
-}
 #endif
index 77f4799..292b1a8 100644 (file)
@@ -54,6 +54,7 @@
 #include <obd_class.h>
 #include <lustre_disk.h>
 #include <lustre_fid.h>
+#include <lustre_param.h>
 #include <md_object.h>
 
 #include "osd_internal.h"
@@ -745,21 +746,31 @@ static int osd_process_config(const struct lu_env *env,
                              struct lu_device *d, struct lustre_cfg *cfg)
 {
        struct osd_device       *o = osd_dev(d);
-       int                      err;
+       int                     rc;
        ENTRY;
 
        switch(cfg->lcfg_command) {
        case LCFG_SETUP:
-               err = osd_mount(env, o, cfg);
+               rc = osd_mount(env, o, cfg);
                break;
        case LCFG_CLEANUP:
-               err = osd_shutdown(env, o);
+               rc = osd_shutdown(env, o);
+               break;
+       case LCFG_PARAM: {
+               LASSERT(&o->od_dt_dev);
+               rc = class_process_proc_param(PARAM_OSD, lprocfs_osd_obd_vars,
+                                             cfg, &o->od_dt_dev);
+               if (rc > 0 || rc == -ENOSYS)
+                       rc = class_process_proc_param(PARAM_OST,
+                                                     lprocfs_osd_obd_vars,
+                                                     cfg, &o->od_dt_dev);
                break;
+       }
        default:
-               err = -ENOTTY;
+               rc = -ENOTTY;
        }
 
-       RETURN(err);
+       RETURN(rc);
 }
 
 static int osd_recovery_complete(const struct lu_env *env, struct lu_device *d)
index cc0f3dc..c91c1d7 100644 (file)
@@ -1062,6 +1062,78 @@ test_28() {
 }
 run_test 28 "permanent parameter setting"
 
+test_28a() { # LU-4221
+       [[ $(lustre_version_code ost1) -ge $(version_code 2.5.52) ]] ||
+               { skip "Need OST version at least 2.5.52" && return 0; }
+       [ "$(facet_fstype ost1)" = "zfs" ] &&
+               skip "LU-4221: no such proc params for ZFS OSTs" && return
+
+       local name
+       local param
+       local cmd
+       local old
+       local new
+       local device="$FSNAME-OST0000"
+
+       setup
+
+       # In this test we will set three kinds of proc parameters with
+       # lctl conf_param:
+       # 1. the ones moved from the OFD to the OSD, and only their
+       #    symlinks kept in obdfilter
+       # 2. non-symlink ones in the OFD
+       # 3. non-symlink ones in the OSD
+
+       # Check 1.
+       # prepare a symlink parameter in the OFD
+       name="writethrough_cache_enable"
+       param="$device.ost.$name"
+       cmd="$LCTL get_param -n obdfilter.$device.$name"
+
+       # conf_param the symlink parameter in the OFD
+       old=$(do_facet ost1 $cmd)
+       new=$(((old + 1) % 2))
+       set_conf_param_and_check ost1 "$cmd" "$param" $new ||
+               error "lctl conf_param $device.ost.$param=$new failed"
+
+       # conf_param the target parameter in the OSD
+       param="$device.osd.$name"
+       cmd="$LCTL get_param -n osd-*.$device.$name"
+       set_conf_param_and_check ost1 "$cmd" "$param" $old ||
+               error "lctl conf_param $device.osd.$param=$old failed"
+
+       # Check 2.
+       # prepare a non-symlink parameter in the OFD
+       name="client_cache_seconds"
+       param="$device.ost.$name"
+       cmd="$LCTL get_param -n obdfilter.$device.$name"
+
+       # conf_param the parameter in the OFD
+       old=$(do_facet ost1 $cmd)
+       new=$((old * 2))
+       set_conf_param_and_check ost1 "$cmd" "$param" $new ||
+               error "lctl conf_param $device.ost.$param=$new failed"
+       set_conf_param_and_check ost1 "$cmd" "$param" $old ||
+               error "lctl conf_param $device.ost.$param=$old failed"
+
+       # Check 3.
+       # prepare a non-symlink parameter in the OSD
+       name="lma_self_repair"
+       param="$device.osd.$name"
+       cmd="$LCTL get_param -n osd-*.$device.$name"
+
+       # conf_param the parameter in the OSD
+       old=$(do_facet ost1 $cmd)
+       new=$(((old + 1) % 2))
+       set_conf_param_and_check ost1 "$cmd" "$param" $new ||
+               error "lctl conf_param $device.osd.$param=$new failed"
+       set_conf_param_and_check ost1 "$cmd" "$param" $old ||
+               error "lctl conf_param $device.osd.$param=$old failed"
+
+       cleanup
+}
+run_test 28a "set symlink parameters permanently with conf_param"
+
 test_29() {
        [ "$OSTCOUNT" -lt "2" ] && skip_env "$OSTCOUNT < 2, skipping" && return
         setup > /dev/null 2>&1