From 9e3616004d5c1eead6372edd8af4875d401fe724 Mon Sep 17 00:00:00 2001 From: Emoly Liu Date: Tue, 7 Jan 2014 22:56:21 +0800 Subject: [PATCH] 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. This is a backport of commit b1491d26271f074dc6f99cde037403337c0b2151 in http://review.whamcloud.com/8238 . Signed-off-by: Emoly Liu Signed-off-by: Michael MacDonald Signed-off-by: Hongchao Zhang Signed-off-by: Jian Yu Change-Id: I8b8d4244f90bd9e16acdccedd09da73fbb5e501b Reviewed-on: http://review.whamcloud.com/8618 Tested-by: Jenkins Tested-by: Maloo Tested-by: Michael MacDonald Reviewed-by: James Simmons Reviewed-by: Oleg Drokin --- lustre/include/lustre_param.h | 1 + lustre/mgs/mgs_llog.c | 15 ++++---- lustre/obdclass/obd_config.c | 28 +++++++-------- lustre/ofd/ofd_dev.c | 44 ++++++++++++++++++++++- lustre/osd-ldiskfs/osd_handler.c | 42 +++++++++++++--------- lustre/osd-ldiskfs/osd_internal.h | 3 +- lustre/osd-ldiskfs/osd_lproc.c | 10 ++---- lustre/osd-zfs/osd_handler.c | 21 ++++++++--- lustre/tests/conf-sanity.sh | 74 +++++++++++++++++++++++++++++++++++++++ 9 files changed, 186 insertions(+), 52 deletions(-) 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 b6cdef7..98fcc8b 100644 --- a/lustre/mgs/mgs_llog.c +++ b/lustre/mgs/mgs_llog.c @@ -3488,17 +3488,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 43b79b8..b549364 100644 --- a/lustre/obdclass/obd_config.c +++ b/lustre/obdclass/obd_config.c @@ -1301,7 +1301,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); @@ -1332,19 +1335,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), diff --git a/lustre/ofd/ofd_dev.c b/lustre/ofd/ofd_dev.c index 75ac1a9..16f9e29 100644 --- a/lustre/ofd/ofd_dev.c +++ b/lustre/ofd/ofd_dev.c @@ -187,6 +187,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) @@ -234,12 +268,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 555c3fa..03fddae 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" @@ -5687,23 +5689,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, @@ -5836,17 +5847,16 @@ 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) return rc; - rc = class_register_type(&osd_obd_device_ops, NULL, lvars.module_vars, + rc = class_register_type(&osd_obd_device_ops, NULL, + lprocfs_osd_module_vars, LUSTRE_OSD_LDISKFS_NAME, &osd_device_type); if (rc) lu_kmem_fini(ldiskfs_caches); diff --git a/lustre/osd-ldiskfs/osd_internal.h b/lustre/osd-ldiskfs/osd_internal.h index cb390c4..c80a916 100644 --- a/lustre/osd-ldiskfs/osd_internal.h +++ b/lustre/osd-ldiskfs/osd_internal.h @@ -628,7 +628,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 5171863..e0d4d99 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", @@ -545,9 +544,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 de11dc1..331368f 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" @@ -740,21 +741,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 8552663..407bece 100644 --- a/lustre/tests/conf-sanity.sh +++ b/lustre/tests/conf-sanity.sh @@ -1064,6 +1064,80 @@ test_28() { } run_test 28 "permanent parameter setting" +test_28a() { # LU-4221 + [ $(lustre_version_code ost1) -eq $(version_code 2.5.0) -o \ + $(lustre_version_code ost1) -ge $(version_code 2.5.52) ] || + { skip "Need OST version >= 2.5.52 or = 2.5.0" && + 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="auto_scrub" + 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 -- 1.8.3.1