From: Emoly Liu Date: Mon, 9 Dec 2013 07:46:04 +0000 (+0800) Subject: LU-4221 osd: add case LCFG_PARAM to osd_process_config X-Git-Tag: 2.5.53~15 X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=b1491d26271f074dc6f99cde037403337c0b2151 LU-4221 osd: add case LCFG_PARAM to osd_process_config 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 Change-Id: I8f64dcc29017c3ed9da010ff0a3e40c02327f05c Reviewed-on: http://review.whamcloud.com/8238 Tested-by: Jenkins Tested-by: Andreas Dilger Tested-by: Maloo Reviewed-by: Andreas Dilger Reviewed-by: James Simmons --- diff --git a/lustre/include/lustre_param.h b/lustre/include/lustre_param.h index 87d0f48..7df6194 100644 --- a/lustre/include/lustre_param.h +++ b/lustre/include/lustre_param.h @@ -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." diff --git a/lustre/mgs/mgs_llog.c b/lustre/mgs/mgs_llog.c index b610b86..9b0201f 100644 --- a/lustre/mgs/mgs_llog.c +++ b/lustre/mgs/mgs_llog.c @@ -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; diff --git a/lustre/obdclass/obd_config.c b/lustre/obdclass/obd_config.c index 47f42b1..286fb8a 100644 --- a/lustre/obdclass/obd_config.c +++ b/lustre/obdclass/obd_config.c @@ -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", diff --git a/lustre/ofd/ofd_dev.c b/lustre/ofd/ofd_dev.c index 2bdbde6..91e28b4 100644 --- a/lustre/ofd/ofd_dev.c +++ b/lustre/ofd/ofd_dev.c @@ -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, ¶m) == 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: { diff --git a/lustre/osd-ldiskfs/osd_handler.c b/lustre/osd-ldiskfs/osd_handler.c index c13c682..4644b9d 100644 --- a/lustre/osd-ldiskfs/osd_handler.c +++ b/lustre/osd-ldiskfs/osd_handler.c @@ -62,6 +62,8 @@ /* struct ptlrpc_thread */ #include #include +/* process_config */ +#include #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) diff --git a/lustre/osd-ldiskfs/osd_internal.h b/lustre/osd-ldiskfs/osd_internal.h index 8d5ca40..e9dccbc 100644 --- a/lustre/osd-ldiskfs/osd_internal.h +++ b/lustre/osd-ldiskfs/osd_internal.h @@ -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); diff --git a/lustre/osd-ldiskfs/osd_lproc.c b/lustre/osd-ldiskfs/osd_lproc.c index 83bb586..b9b4e3d 100644 --- a/lustre/osd-ldiskfs/osd_lproc.c +++ b/lustre/osd-ldiskfs/osd_lproc.c @@ -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 diff --git a/lustre/osd-zfs/osd_handler.c b/lustre/osd-zfs/osd_handler.c index 77f4799..292b1a8 100644 --- a/lustre/osd-zfs/osd_handler.c +++ b/lustre/osd-zfs/osd_handler.c @@ -54,6 +54,7 @@ #include #include #include +#include #include #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) diff --git a/lustre/tests/conf-sanity.sh b/lustre/tests/conf-sanity.sh index cc0f3dc..c91c1d7 100644 --- a/lustre/tests/conf-sanity.sh +++ b/lustre/tests/conf-sanity.sh @@ -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