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.

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

debug-log: add helpful hint msg about previous log_dir #518

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pkalever
Copy link
Contributor

@pkalever pkalever commented Dec 14, 2018

Logs on new log file path look like:

2018-12-14 18:30:31.154 14398 [CRIT] create_file_output:480:
log file path now is '/var/log/gluster-block/tcmu-runner.log'
2018-12-14 18:30:31.154 14398 [CRIT] tcmu_resetup_log_file:748:
you may find previous logs at '/var/log/tcmu-runner.log'

Signed-off-by: Prasanna Kumar Kalever [email protected]

Prasanna Kumar Kalever added 2 commits December 14, 2018 18:26
Logs on new log file path look like:

2018-12-14 18:30:31.154 14398 [CRIT] create_file_output:480:
  log file path now is '/var/log/gluster-block/tcmu-runner.log'
2018-12-14 18:30:31.154 14398 [CRIT] tcmu_resetup_log_file:748:
  you may find previous logs at '/var/log/tcmu-runner.log'

Signed-off-by: Prasanna Kumar Kalever <[email protected]>
@@ -714,13 +714,17 @@ static bool is_same_path(const char* path1, const char* path2)
int tcmu_resetup_log_file(char *log_dir)
{
int ret;
char old_logdir[PATH_MAX] = {'\0', };
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 tcmu_log_dir_set should have been called by the tcmu_log_dir_create caller and not tcmu_log_dir_create. tcmu_log_dir_create just creates the dirs. Calling it in tcmu_log_dir_create leads to the weird dance we do below and also if the resetup fails we were leaving it set to the bad value.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, agree with this.

@lxbsz lxbsz changed the base branch from master to main August 10, 2022 00:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants