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 <jay@whamcloud.com>
Reviewed-on: http://review.whamcloud.com/1168
Tested-by: Hudson
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Mikhail Pershin <tappro@whamcloud.com>
Tested-by: Maloo <whamcloud.maloo@gmail.com>
Reviewed-by: Yang Sheng <ys@whamcloud.com>

diff --git a/lustre/include/obd_class.h b/lustre/include/obd_class.h
index 3865358..db4cc06 100644 (file)
--- 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 {
 
 /* 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 */
         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 (file)
--- 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.*/
 
         /* 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;
 
         /* 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);
         }
                 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");
 
         if (!osc) {
                 CERROR("no osc\n");

diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c
index fa69404..abff420 100644 (file)
--- 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)
 {
 
 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;
         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;
 
         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.*/
         /* 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 */
         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);
 
         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);
         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);
         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)
 
         /* 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)
         if (dt)

-                OBD_FREE(dt, strlen(dt) + 1);
+                OBD_FREE(dt, strlen(lprof->lp_dt) + instlen);

         if (err)
                 ll_put_super(sb);
         else
         if (err)
                 ll_put_super(sb);
         else


@@ -982,8 +981,8 @@ void ll_put_super(struct super_block *sb)
 
         ll_print_capa_stat(sbi);
 
 
         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);
 
         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);
 
 
         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);
 
 
         cfs_module_put(THIS_MODULE);
 

diff --git a/lustre/mgc/mgc_request.c b/lustre/mgc/mgc_request.c
index cfc2797..d485882 100644 (file)
--- 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;
                                          struct config_llog_instance *cfg)
 {
         struct config_llog_data *cld;

-        char *logid = logname;
+        struct config_llog_data *found = NULL;
+        void *                   instance;

         ENTRY;
 
         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) {
         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);
         cfs_spin_unlock(&config_list_lock);

-        LASSERT(cld->cld_stopping == 0 || cld->cld_is_sptlrpc == 0);
-        RETURN(cld);
+        RETURN(found);

 }
 
 static
 }
 
 static


@@ -201,8 +198,8 @@ struct config_llog_data *do_config_log_add(struct obd_device *obd,
         int                      rc;
         ENTRY;
 
         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)
 
         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;
 
         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
 
         /*
          * 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);
         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);
                 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);
         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);
         /*
 
         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);
 
         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);
                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 (rc)
                         break;
                 cld = config_log_find(logname, cfg);

-                if (IS_ERR(cld)) {
-                        rc = PTR_ERR(cld);
+                if (cld == NULL) {
+                        rc = -ENOENT;

                         break;
                 }
 
                         break;
                 }
 

diff --git a/lustre/obdclass/obd_config.c b/lustre/obdclass/obd_config.c
index a9669fa..0e430d9 100644 (file)
--- 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! "
                     !(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;
                               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);
 
 
                 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) +
                     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);
                         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);
                                 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 */
 
                 /* 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);
                     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].
                  */
                  * 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]);
                     lcfg->lcfg_command == LCFG_SPTLRPC_CONF) {
                         lustre_cfg_bufs_set(&bufs, 2, bufs.lcfg_buf[1],
                                             bufs.lcfg_buflen[1]);