Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

config: add the logdir option dynamic reloading support #368

Merged
merged 5 commits into from
Jun 1, 2018
Merged

config: add the logdir option dynamic reloading support #368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4e280e0
config: reset to str default values after the options being set
lxbsz May 7, 2018
4278522
config: reset to boolean default values after the options being set
lxbsz May 7, 2018
6d09e8b
config: fix memory leak of cfg->log_dir_path
lxbsz May 9, 2018
b4caa54
config: add the logdir option dynamic reloading support
lxbsz May 7, 2018
1b6e652
log: do not insert the CDB debug logs to ringbuffer
lxbsz May 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 22 additions & 9 deletions libtcmu_config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,21 +114,30 @@ do { \
} \
} while (0)

#define TCMU_PARSE_CFG_BOOL(cfg, key) \
#define TCMU_PARSE_CFG_BOOL(cfg, key, def) \
do { \
struct tcmu_conf_option *option; \
option = tcmu_get_option(#key); \
if (option) { \
cfg->key = option->opt_bool; \
option->opt_bool = def; \
} \
} while (0)

#define TCMU_PARSE_CFG_STR(cfg, key) \
#define TCMU_PARSE_CFG_STR(cfg, key, def) \
do { \
struct tcmu_conf_option *option; \
char buf[1024]; \
option = tcmu_get_option(#key); \
if (option) { \
cfg->key = strdup(option->opt_str); } \
if (cfg->key) \
free(cfg->key); \
cfg->key = strdup(option->opt_str); \
if (option->opt_str) \
free(option->opt_str); \
sprintf(buf, "%s", def); \
option->opt_str = strdup(buf); \
} \
} while (0);

#define TCMU_FREE_CFG_STR_KEY(cfg, key) \
Expand All @@ -144,9 +153,9 @@ static void tcmu_conf_set_options(struct tcmu_config *cfg, bool reloading)
tcmu_set_log_level(cfg->log_level);
}

/* set log_dir path option */
TCMU_PARSE_CFG_STR(cfg, log_dir_path, TCMU_LOG_DIR_DEFAULT);
if (!reloading) {
/* set log_dir path option */
TCMU_PARSE_CFG_STR(cfg, log_dir_path);
/*
* The priority of the logdir setting is:
* 1, --tcmu_log_dir/-l LOG_DIR_PATH
Expand All @@ -155,11 +164,16 @@ static void tcmu_conf_set_options(struct tcmu_config *cfg, bool reloading)
* 4, default /var/log/
*/
if (!tcmu_get_logdir())
tcmu_logdir_create(cfg->log_dir_path);
tcmu_logdir_create(cfg->log_dir_path, false);
else
tcmu_warn("The logdir option from the tcmu.conf will be ignored\n");
} else {
tcmu_warn("The logdir option is not supported by dynamic reloading for now!\n");
/*
* Here we asume that users want to change the
* log_dir_path without considering the priority
* mentioned above.
*/
tcmu_logdir_resetup(cfg->log_dir_path);
}

/* add your new config options */
Expand All @@ -172,6 +186,7 @@ static void tcmu_conf_free_str_keys(struct tcmu_config *cfg)
* For example:
* TCMU_FREE_CFG_STR_KEY(cfg, 'STR KEY');
*/
TCMU_FREE_CFG_STR_KEY(cfg, log_dir_path);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if you do the above then you need to fix tcmu_destroy_config:

        tcmu_conf_free_str_keys(cfg);
        if (cfg->log_dir_path)
                free(cfg->log_dir_path);

because tcmu_conf_free_str_keys call will delete cfg->log-dir_path and then the call above will try to delete the same pointer.

I actually thought the leak was if tcmu_conf_set_options is called twice. The first call would do:

cfg->key = strdup(option->opt_str); \

but then the second call would see the cfg->key is set and we would end up allocating memory again without freeing and leak the old memory at cfg->key. I thought we needed a

if (!cfg->key)
     cfg->key = strdup(option->opt_str);

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, updated all.
Thanks,

}

#define TCMU_MAX_CFG_FILE_SIZE (2 * 1024 * 1024)
Expand Down Expand Up @@ -538,8 +553,6 @@ void tcmu_destroy_config(struct tcmu_config *cfg)
}

tcmu_conf_free_str_keys(cfg);
if (cfg->log_dir_path)
free(cfg->log_dir_path);
free(cfg->path);
free(cfg);
}
Loading