Minor Description
There are several easy cleanups that can be done in the mgc config code and its interfaces.
As noted on https://review.whamcloud.com/#/c/27320/:
his looks like it will fix the problem, but the code here could be further improved to avoid potential errors in the future.
lustre/mgc/mgc_request.c
Line 300:
(style) would be better to do this in one check:
if (cfg->cfg_sub_clds & CONFIG_T_SPTLRPC)
lcfg.cfg_instance = NULL;
else if (sb != NULL)
lcfg.cfg_instance = sb;
else
lcfg.cfg_instance = obd;
no need to cast the pointers to void.
Also, blech on having CONFIG_T_{PARAMS,SPTLRPC,BARRIER,...} and CONFIG_SUB_{PARAMS,...}. This is obviously very error prone.
Since CONFIG_T_is part of lustre_idl.h and is an enum, what about adding:
enum config_sub_mask {
CONFIG_SUB_SPTLRPC = BIT(CONFIG_T_SPTLRPC),
CONFIG_SUB_RECOVER = BIT(CONFIG_T_PARAMS),
CONFIG_SUB_PARAMS = BIT(CONFIG_T_PARAMS),
CONFIG_SUB_NODEMAP = BIT(CONFIG_T_NODEMAP),
CONFIG_SUB_BARRIER = BIT(CONFIG_T_BARRIER),
};
and naming the CONFIG_T_* enum, and adding the enums as arguments where they are used so that it is clear which one is intended,
and/or renaming these a bit so that "T" and "SUB" are more self-explanatory?
The definition of struct config_llog_data can be moved to mgc_request.c. Same for cld_is_*() and the declaration of mgc_process_log().
The declarations mgc_fsname2resid() and mgc_logname2resid() can be moved to mgc_internal.h and neither function needs to be exported.
mgc_process_log() can be made static.
The data and datalen parameters of obd_process_config() should be replaces with a struct lustre_cfg * parameter:
static inline int -obd_process_config(struct obd_device *obd, int datalen, void *data) +obd_process_config(struct obd_device *obd, struct lustre_cfg *lcfg)
Most of the char *logname or char *fsname parameters in lustre/mgc/ should be converted to const char *.
Similarly, most of the struct config_llog_instance * parameters should be const qualified.
The void *cfg_instance member of struct config_llog_instance should be const qualified.
The ordering of parameters is inconsistent:
static struct config_llog_data *config_log_find_or_add(struct obd_device *obd, char *logname, struct super_block *sb, int type, struct config_llog_instance *cfg) static struct config_llog_data * config_log_add(struct obd_device *obd, char *logname, struct config_llog_instance *cfg, struct super_block *sb)
Several functions can be simplified if we use a const void *instance parameter (pointing to the sb or obd device) rather than a struct config_llog_instance * parameter. These include config_log_find() and config_log_end().
After this is done, the lustre_cfg for LCFG_LOG_END can be simplified to pass just the sb or obd pointer instead of a whole struct config_llog_instance.
We should also consider removing LCFG_LOG_START and LCFG_LOG_END and replacing their uses with calls to new obd methods. There's no reason to pack in kernel objects into a lustre_cfg for this and it obscures analysis of these functions.