From: Jinshan Xiong Date: Sat, 30 Jul 2011 07:24:51 +0000 (-0700) Subject: LU-556: Fix config_log_find() in mgc_request.c X-Git-Tag: 2.1.0-RC0~18 X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=cbc4ca2e8dc37d54bb7d3c9b02ab20b63e60f592 LU-556: Fix config_log_find() in mgc_request.c This issue was imported by lu-411. Actually after checking the code, I found we can make things a bit simpler. Change-Id: Iad212782a5f0357b73014baf7766c77637fb47db Signed-off-by: Jinshan Xiong Reviewed-on: http://review.whamcloud.com/1168 Tested-by: Hudson Reviewed-by: Andreas Dilger Reviewed-by: Mikhail Pershin Tested-by: Maloo Reviewed-by: Yang Sheng --- diff --git a/lustre/include/obd_class.h b/lustre/include/obd_class.h index 3865358..db4cc06 100644 --- a/lustre/include/obd_class.h +++ b/lustre/include/obd_class.h @@ -144,8 +144,8 @@ static inline void lprocfs_echo_init_vars(struct lprocfs_static_vars *lvars) /* Passed as data param to class_config_parse_llog */ struct config_llog_instance { - char * cfg_obdname; - char cfg_instance[sizeof(void*) * 2 + 1]; + char *cfg_obdname; + void *cfg_instance; struct super_block *cfg_sb; struct obd_uuid cfg_uuid; int cfg_last_idx; /* for partial llog processing */ diff --git a/lustre/liblustre/super.c b/lustre/liblustre/super.c index 4274240..08a0457 100644 --- a/lustre/liblustre/super.c +++ b/lustre/liblustre/super.c @@ -1879,7 +1879,7 @@ llu_fsswop_mount(const char *source, /* generate a string unique to this super, let's try the address of the super itself.*/ - snprintf(cfg.cfg_instance, sizeof(cfg.cfg_instance), "%p", sbi); + cfg.cfg_instance = sbi; /* retrive & parse config log */ cfg.cfg_uuid = sbi->ll_sb_uuid; @@ -1894,11 +1894,11 @@ llu_fsswop_mount(const char *source, CERROR("No profile found: %s\n", zconf_profile); GOTO(out_free, err = -EINVAL); } - OBD_ALLOC(osc, strlen(lprof->lp_dt) + strlen(cfg.cfg_instance) + 2); - sprintf(osc, "%s-%s", lprof->lp_dt, cfg.cfg_instance); + OBD_ALLOC(osc, strlen(lprof->lp_dt) + sizeof(cfg.cfg_instance)*2 + 1); + sprintf(osc, "%s-%p", lprof->lp_dt, cfg.cfg_instance); - OBD_ALLOC(mdc, strlen(lprof->lp_md) + strlen(cfg.cfg_instance) + 2); - sprintf(mdc, "%s-%s", lprof->lp_md, cfg.cfg_instance); + OBD_ALLOC(mdc, strlen(lprof->lp_md) + sizeof(cfg.cfg_instance)*2 + 1); + sprintf(mdc, "%s-%p", lprof->lp_md, cfg.cfg_instance); if (!osc) { CERROR("no osc\n"); diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c index fa69404..abff420 100644 --- a/lustre/llite/llite_lib.c +++ b/lustre/llite/llite_lib.c @@ -870,12 +870,13 @@ static inline int ll_bdi_register(struct backing_dev_info *bdi) int ll_fill_super(struct super_block *sb, struct vfsmount *mnt) { - struct lustre_profile *lprof; + struct lustre_profile *lprof = NULL; struct lustre_sb_info *lsi = s2lsi(sb); struct ll_sb_info *sbi; char *dt = NULL, *md = NULL; char *profilenm = get_profile_name(sb); struct config_llog_instance *cfg; + const int instlen = sizeof(cfg->cfg_instance) * 2 + 1; int err; ENTRY; @@ -915,7 +916,7 @@ int ll_fill_super(struct super_block *sb, struct vfsmount *mnt) /* Generate a string unique to this super, in case some joker tries to mount the same fs at two mount points. Use the address of the super itself.*/ - snprintf(cfg->cfg_instance, sizeof(cfg->cfg_instance), "%p", sb); + cfg->cfg_instance = sb; cfg->cfg_uuid = lsi->lsi_llsbi->ll_sb_uuid; /* set up client obds */ @@ -936,26 +937,24 @@ int ll_fill_super(struct super_block *sb, struct vfsmount *mnt) CDEBUG(D_CONFIG, "Found profile %s: mdc=%s osc=%s\n", profilenm, lprof->lp_md, lprof->lp_dt); - OBD_ALLOC(dt, strlen(lprof->lp_dt) + - strlen(cfg->cfg_instance) + 2); + OBD_ALLOC(dt, strlen(lprof->lp_dt) + instlen); if (!dt) GOTO(out_free, err = -ENOMEM); - sprintf(dt, "%s-%s", lprof->lp_dt, cfg->cfg_instance); + sprintf(dt, "%s-%p", lprof->lp_dt, cfg->cfg_instance); - OBD_ALLOC(md, strlen(lprof->lp_md) + - strlen(cfg->cfg_instance) + 2); + OBD_ALLOC(md, strlen(lprof->lp_md) + instlen); if (!md) GOTO(out_free, err = -ENOMEM); - sprintf(md, "%s-%s", lprof->lp_md, cfg->cfg_instance); + sprintf(md, "%s-%p", lprof->lp_md, cfg->cfg_instance); /* connections, registrations, sb setup */ err = client_common_fill_super(sb, md, dt, mnt); out_free: if (md) - OBD_FREE(md, strlen(md) + 1); + OBD_FREE(md, strlen(lprof->lp_md) + instlen); if (dt) - OBD_FREE(dt, strlen(dt) + 1); + OBD_FREE(dt, strlen(lprof->lp_dt) + instlen); if (err) ll_put_super(sb); else @@ -982,8 +981,8 @@ void ll_put_super(struct super_block *sb) ll_print_capa_stat(sbi); - snprintf(cfg.cfg_instance, sizeof(cfg.cfg_instance), "%p", sb); - lustre_end_log(sb, NULL, &cfg); + cfg.cfg_instance = sb; + lustre_end_log(sb, profilenm, &cfg); if (sbi->ll_md_exp) { obd = class_exp2obd(sbi->ll_md_exp); @@ -1026,7 +1025,7 @@ void ll_put_super(struct super_block *sb) cl_env_cache_purge(~0); - LCONSOLE_WARN("client %s umount complete\n", cfg.cfg_instance); + LCONSOLE_WARN("client %p umount complete\n", cfg.cfg_instance); cfs_module_put(THIS_MODULE); diff --git a/lustre/mgc/mgc_request.c b/lustre/mgc/mgc_request.c index cfc2797..d485882 100644 --- a/lustre/mgc/mgc_request.c +++ b/lustre/mgc/mgc_request.c @@ -160,34 +160,31 @@ struct config_llog_data *config_log_find(char *logname, struct config_llog_instance *cfg) { struct config_llog_data *cld; - char *logid = logname; + struct config_llog_data *found = NULL; + void * instance; ENTRY; - if (cfg) - logid = cfg->cfg_instance; - - if (!logid) { - CERROR("No log specified\n"); - RETURN(ERR_PTR(-EINVAL)); - } + LASSERT(logname != NULL); + instance = cfg ? cfg->cfg_instance : NULL; cfs_spin_lock(&config_list_lock); cfs_list_for_each_entry(cld, &config_llog_list, cld_list_chain) { - char *name = cld->cld_logname; - if (cfg) - name = cld->cld_cfg.cfg_instance; - if (strcmp(logid, name) == 0) - goto out_found; - } - cfs_spin_unlock(&config_list_lock); + /* check if instance equals */ + if (instance != cld->cld_cfg.cfg_instance) + continue; - CDEBUG(D_CONFIG, "can't get log %s\n", logid); - RETURN(ERR_PTR(-ENOENT)); -out_found: - cfs_atomic_inc(&cld->cld_refcount); + /* instance may be NULL, should check name */ + if (strcmp(logname, cld->cld_logname) == 0) { + found = cld; + break; + } + } + if (found) { + cfs_atomic_inc(&found->cld_refcount); + LASSERT(found->cld_stopping == 0 || found->cld_is_sptlrpc == 0); + } cfs_spin_unlock(&config_list_lock); - LASSERT(cld->cld_stopping == 0 || cld->cld_is_sptlrpc == 0); - RETURN(cld); + RETURN(found); } static @@ -201,8 +198,8 @@ struct config_llog_data *do_config_log_add(struct obd_device *obd, int rc; ENTRY; - CDEBUG(D_MGC, "do adding config log %s:%s\n", logname, - cfg ? cfg->cfg_instance : "NULL"); + CDEBUG(D_MGC, "do adding config log %s:%p\n", logname, + cfg ? cfg->cfg_instance : 0); OBD_ALLOC(cld, sizeof(*cld)); if (!cld) @@ -264,7 +261,7 @@ static int config_log_add(struct obd_device *obd, char *logname, char *ptr; ENTRY; - CDEBUG(D_MGC, "adding config log %s:%s\n", logname, cfg->cfg_instance); + CDEBUG(D_MGC, "adding config log %s:%p\n", logname, cfg->cfg_instance); /* * for each regular log, the depended sptlrpc log name is @@ -280,7 +277,7 @@ static int config_log_add(struct obd_device *obd, char *logname, strcpy(seclogname + (ptr - logname), "-sptlrpc"); sptlrpc_cld = config_log_find(seclogname, NULL); - if (IS_ERR(sptlrpc_cld)) { + if (sptlrpc_cld == NULL) { sptlrpc_cld = do_config_log_add(obd, seclogname, 1, NULL, NULL); if (IS_ERR(sptlrpc_cld)) { CERROR("can't create sptlrpc log: %s\n", seclogname); @@ -311,8 +308,8 @@ static int config_log_end(char *logname, struct config_llog_instance *cfg) ENTRY; cld = config_log_find(logname, cfg); - if (IS_ERR(cld)) - RETURN(PTR_ERR(cld)); + if (cld == NULL) + RETURN(-ENOENT); cfs_mutex_lock(&cld->cld_lock); /* @@ -1269,7 +1266,7 @@ int mgc_process_log(struct obd_device *mgc, if (cld->cld_cfg.cfg_sb) lsi = s2lsi(cld->cld_cfg.cfg_sb); - CDEBUG(D_MGC, "Process log %s:%s from %d\n", cld->cld_logname, + CDEBUG(D_MGC, "Process log %s:%p from %d\n", cld->cld_logname, cld->cld_cfg.cfg_instance, cld->cld_cfg.cfg_last_idx + 1); ctxt = llog_get_context(mgc, LLOG_CONFIG_REPL_CTXT); @@ -1426,8 +1423,8 @@ static int mgc_process_config(struct obd_device *obd, obd_count len, void *buf) if (rc) break; cld = config_log_find(logname, cfg); - if (IS_ERR(cld)) { - rc = PTR_ERR(cld); + if (cld == NULL) { + rc = -ENOENT; break; } diff --git a/lustre/obdclass/obd_config.c b/lustre/obdclass/obd_config.c index a9669fa..0e430d9 100644 --- a/lustre/obdclass/obd_config.c +++ b/lustre/obdclass/obd_config.c @@ -1236,7 +1236,7 @@ static int class_config_llog_handler(struct llog_handle * handle, !(clli->cfg_flags & CFG_F_MARKER) && (lcfg->lcfg_command != LCFG_MARKER)) { CWARN("Config not inside markers, ignoring! " - "(inst: %s, uuid: %s, flags: %#x)\n", + "(inst: %p, uuid: %s, flags: %#x)\n", clli->cfg_instance, clli->cfg_uuid.uuid, clli->cfg_flags); clli->cfg_flags |= CFG_F_SKIP; @@ -1279,15 +1279,15 @@ static int class_config_llog_handler(struct llog_handle * handle, lustre_cfg_bufs_init(&bufs, lcfg); - if (clli && clli->cfg_instance[0] != '\0' && + if (clli && clli->cfg_instance && LUSTRE_CFG_BUFLEN(lcfg, 0) > 0){ inst = 1; inst_len = LUSTRE_CFG_BUFLEN(lcfg, 0) + - strlen(clli->cfg_instance) + 1; + sizeof(clli->cfg_instance) * 2 + 1; OBD_ALLOC(inst_name, inst_len); if (inst_name == NULL) GOTO(out, rc = -ENOMEM); - sprintf(inst_name, "%s-%s", + sprintf(inst_name, "%s-%p", lustre_cfg_string(lcfg, 0), clli->cfg_instance); lustre_cfg_bufs_set_string(&bufs, 0, inst_name); @@ -1297,7 +1297,7 @@ static int class_config_llog_handler(struct llog_handle * handle, /* we override the llog's uuid for clients, to insure they are unique */ - if (clli && clli->cfg_instance[0] != '\0' && + if (clli && clli->cfg_instance != NULL && lcfg->lcfg_command == LCFG_ATTACH) { lustre_cfg_bufs_set_string(&bufs, 2, clli->cfg_uuid.uuid); @@ -1309,7 +1309,7 @@ static int class_config_llog_handler(struct llog_handle * handle, * moving them to index [1] and [2], and insert MGC's * obdname at index [0]. */ - if (clli && clli->cfg_instance[0] == '\0' && + if (clli && clli->cfg_instance == NULL && lcfg->lcfg_command == LCFG_SPTLRPC_CONF) { lustre_cfg_bufs_set(&bufs, 2, bufs.lcfg_buf[1], bufs.lcfg_buflen[1]);