Hi,
just a few comments, mostly code style.
2013-11-26 16:04, Peng Tao:
> @@ -1011,23 +1083,17 @@ int mgc_set_info_async(const struct lu_env *env,
> struct obd_export *exp,
> }
> if (KEY_IS(KEY_SET_FS)) {
> struct super_block *sb = (struct super_block *)val;
> - struct lustre_sb_info *lsi;
> +
> if (vallen != sizeof(struct super_block))
> return -EINVAL;
> - lsi = s2lsi(sb);
> - rc = mgc_fs_setup(exp->exp_obd, sb, lsi->lsi_srv_mnt);
> - if (rc) {
> - CERROR("set_fs got %d\n", rc);
> - }
> +
> + rc = mgc_fs_setup(exp->exp_obd, sb);
> return rc;
Why set rc when you return it on the next statement?
Wouldn't 'return mgc_fs_setup(exp->exp_obd, sb);' be better?
> }
> if (KEY_IS(KEY_CLEAR_FS)) {
> if (vallen != 0)
> return -EINVAL;
> rc = mgc_fs_cleanup(exp->exp_obd);
> - if (rc) {
> - CERROR("clear_fs got %d\n", rc);
> - }
> return rc;
Same here possibly.
> + /* Copy the setup log locally if we can. Don't mess around if we're
> + * running an MGS though (logs are already local). */
> + if (lctxt && lsi && IS_SERVER(lsi) && !IS_MGS(lsi) &&
> + cli->cl_mgc_configs_dir != NULL &&
> + lu2dt_dev(cli->cl_mgc_configs_dir->do_lu.lo_dev) ==
> + lsi->lsi_dt_dev) {
I think that is *really* ugly!
(The last comparison is separated into two lines)
--
Regards,
Levente Kurusa
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
