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

dyn-logger: make sure the logLevel changing log visible always #177

Closed
wants to merge 1 commit into from
Closed

dyn-logger: make sure the logLevel changing log visible always #177

wants to merge 1 commit into from

Conversation

lxbsz
Copy link
Collaborator

@lxbsz lxbsz commented Mar 14, 2019

Fixes: bz#1661406
Signed-off-by: Xiubo Li [email protected]

@ghost ghost assigned lxbsz Mar 14, 2019
@ghost ghost added the in progress label Mar 14, 2019
@lxbsz
Copy link
Collaborator Author

lxbsz commented Mar 14, 2019

The test logs:

[2019-03-14 02:28:15.911589] INFO: event->mask: 0x20
 [at dyn-config.c+509 :<glusterBlockDynConfigStart>]
[2019-03-14 02:28:15.911863] INFO: event->mask: 0x10
 [at dyn-config.c+509 :<glusterBlockDynConfigStart>]
[2019-03-14 02:28:15.911986] INFO: event->mask: 0x20
 [at dyn-config.c+509 :<glusterBlockDynConfigStart>]
[2019-03-14 02:28:15.912013] INFO: event->mask: 0x1
 [at dyn-config.c+509 :<glusterBlockDynConfigStart>]
[2019-03-14 02:28:15.912034] INFO: event->mask: 0x10
 [at dyn-config.c+509 :<glusterBlockDynConfigStart>]
[2019-03-14 02:28:23.074942] INFO: event->mask: 0x800
 [at dyn-config.c+509 :<glusterBlockDynConfigStart>]
[2019-03-14 02:28:23.076089] INFO: event->mask: 0x4
 [at dyn-config.c+509 :<glusterBlockDynConfigStart>]
[2019-03-14 02:28:23.076521] INFO: event->mask: 0x400
 [at dyn-config.c+509 :<glusterBlockDynConfigStart>]
[2019-03-14 02:28:23.076919] INFO: event->mask: 0x8000
 [at dyn-config.c+509 :<glusterBlockDynConfigStart>]
[2019-03-14 02:28:23.078437] NONE: logLevel now is DEBUG
 [at utils.c+48 :<glusterBlockSetLogLevel>]
[2019-03-14 02:28:23.078994] INFO: event->mask: 0x20
 [at dyn-config.c+509 :<glusterBlockDynConfigStart>]
[2019-03-14 02:28:23.079045] INFO: event->mask: 0x1
 [at dyn-config.c+509 :<glusterBlockDynConfigStart>]
[2019-03-14 02:28:23.079082] INFO: event->mask: 0x10
 [at dyn-config.c+509 :<glusterBlockDynConfigStart>]
[2019-03-14 02:28:32.350250] INFO: event->mask: 0x20
 [at dyn-config.c+509 :<glusterBlockDynConfigStart>]
[2019-03-14 02:28:32.350593] INFO: event->mask: 0x10
 [at dyn-config.c+509 :<glusterBlockDynConfigStart>]
[2019-03-14 02:28:32.350634] INFO: event->mask: 0x20
 [at dyn-config.c+509 :<glusterBlockDynConfigStart>]
[2019-03-14 02:28:32.350709] INFO: event->mask: 0x1
 [at dyn-config.c+509 :<glusterBlockDynConfigStart>]
[2019-03-14 02:28:32.350949] INFO: event->mask: 0x10
 [at dyn-config.c+509 :<glusterBlockDynConfigStart>]
[2019-03-14 02:28:38.693986] INFO: event->mask: 0x800
 [at dyn-config.c+509 :<glusterBlockDynConfigStart>]
[2019-03-14 02:28:38.696880] INFO: event->mask: 0x4
 [at dyn-config.c+509 :<glusterBlockDynConfigStart>]
[2019-03-14 02:28:38.696966] INFO: event->mask: 0x400
 [at dyn-config.c+509 :<glusterBlockDynConfigStart>]
[2019-03-14 02:28:38.697114] INFO: event->mask: 0x8000
 [at dyn-config.c+509 :<glusterBlockDynConfigStart>]
[2019-03-14 02:28:38.697205] NONE: logLevel now is ERROR
 [at utils.c+48 :<glusterBlockSetLogLevel>]


[2019-03-14 02:32:51.334017] NONE: logLevel now is WARNING
 [at utils.c+48 :<glusterBlockSetLogLevel>]

@lxbsz
Copy link
Collaborator Author

lxbsz commented Mar 14, 2019

@pkalever @amarts Please review, Thanks.

@amarts amarts requested a review from pkalever March 14, 2019 13:05
amarts
amarts reviewed Mar 18, 2019
View reviewed changes
Copy link
Member

@amarts amarts left a comment

Choose a reason for hiding this comment

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

LGTM

Reviewed By: Amar Tumballi

@pkalever
Copy link
Contributor

@lxbsz Can we fix https://bugzilla.redhat.com/show_bug.cgi?id=1661406 in this PR ?

Thanks!

@pkalever
Copy link
Contributor

@lxbsz Can we fix https://bugzilla.redhat.com/show_bug.cgi?id=1661406 in this PR ?

@lxbsz what are your thoughts about this ?

@lxbsz
Copy link
Collaborator Author

lxbsz commented Mar 20, 2019 •
edited
Loading

@lxbsz Can we fix https://bugzilla.redhat.com/show_bug.cgi?id=1661406 in this PR ?

@lxbsz what are your thoughts about this ?

The change here will make sure that the logLevel changing output log could be seen all the time.

Before this the logLevel changing output log only be seen when the logLevel is DEBUG, WARN and INFO, because itself is under the INFO logLevel.

And from my last comment you can see that when switching the logLevel from ERROR to WARN the log is displayed:

[2019-03-14 02:28:38.697205] NONE: logLevel now is ERROR
 [at utils.c+48 :<glusterBlockSetLogLevel>]
[2019-03-14 02:32:51.334017] NONE: logLevel now is WARNING
 [at utils.c+48 :<glusterBlockSetLogLevel>]

Isn't that what we need to fix that BZ ?

@pkalever
Copy link
Contributor

Isn't that what we need to fix that BZ ?

My memory is bad, I though we have a plan about syncing gluster-block with fixes in https://github.com/open-iscsi/tcmu-runner/pull/506/commits (namely, 1. directory level watch, 2. checking if we are honoring defaults when commented post changing value, 3. retry configfile open if initial one fails .. etc)

Not sure, If I had discussed this with you or Neha.

@lxbsz
Copy link
Collaborator Author

lxbsz commented Mar 21, 2019

Isn't that what we need to fix that BZ ?

My memory is bad, I though we have a plan about syncing gluster-block with fixes in https://github.com/open-iscsi/tcmu-runner/pull/506/commits (namely, 1. directory level watch, 2. checking if we are honoring defaults when commented post changing value, 3. retry configfile open if initial one fails .. etc)

Not sure, If I had discussed this with you or Neha.

Sorry Prasanna, I also couldn't remember of this and may miss this discussion. And I am okay with this above.

Thanks.

@pkalever
Copy link
Contributor

Sorry Prasanna, I also couldn't remember of this and may miss this discussion. And I am okay with this above.

Sure @lxbsz, please go ahead and make changes.

Thanks!

@lxbsz
Copy link
Collaborator Author

lxbsz commented Mar 25, 2019

Sorry Prasanna, I also couldn't remember of this and may miss this discussion. And I am okay with this above.

Sure @lxbsz, please go ahead and make changes.

Thanks!

This is already fixed in #163. Please see 94d6d9a

Thanks.

pkalever
pkalever requested changes Mar 28, 2019
View reviewed changes
utils/utils.c
@@ -44,7 +44,7 @@ glusterBlockSetLogLevel(unsigned int logLevel)
LOCK(gbConf.lock);
gbConf.logLevel = logLevel;
UNLOCK(gbConf.lock);
LOG("mgmt", GB_LOG_INFO,
LOG("mgmt", GB_LOG_NONE,
Copy link
Contributor

Choose a reason for hiding this comment

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

@lxbsz Looks like you are fixing this part in 94d6d9a with GB_LOG_CRIT already. Can we close this PR ?

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, right.
Since this is duplicated to @163, I will close this PR now.
Thanks Prasanna.

@lxbsz lxbsz closed this Mar 28, 2019
@ghost ghost removed the in progress label Mar 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amarts amarts amarts left review comments

@pkalever pkalever pkalever requested changes

Assignees

@lxbsz lxbsz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lxbsz @pkalever @amarts