| Message ID | 20191106094855.13859-1-liuhangbin@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | move all teamd_log_dbg to teamd_log_dbgx | expand |
Hi Jiri, Would you please help review this patch set when have time? Thanks Hangbin On Wed, Nov 06, 2019 at 05:48:49PM +0800, Hangbin Liu wrote: > Hi Jiri, > > I'm not sure if I should split the patch or just post one directly. Please > tell me if you feel the commit message are repeated and want only one patch. > > Recently some users reported that they start to see debug messages in their > syslogs even with daemon_verbosity_level = LOG_INFO and without -g option. > > Actually this issue is there at the begining, the user would see the debug > messages if they run teamd with -d option. The reason that most users did > not notice this is because they are using libteam via NetworkManager, and > NetworkManager run libteam in frontend. > > But after commit e47d5db53873 ("teamd: add an option to force log > output to stdout, stderr or syslog"), NetworkManager will set > TEAM_LOG_OUTPUT=syslog in the environment. At the same time libdaemon > does not filter log levels if we use syslog(see function daemon_logv in > libdaemon). Then all the users would see the debug messages suddenly and > feels annoying. > > And here is the quote for daemon_set_verbosity() from libdaemon/dlog.h > """ > Allows to decide which messages to output on standard output/error > streams. All messages are logged to syslog and this setting does > not influence that. > """ > > Since we should not limit how our user(NM) used libteam. And libdaemon > is intend to not filter logs if use syslog. We'd better filter the > debug message ourselves, like via -g option. So I would prefer to > move all teamd_log_dbg to teamd_log_dbgx. After that, the user could > decide whether to enable debug or not by themselves with -g option. > > Hangbin Liu (6): > teamd/teamd.c: move teamd_log_dbg to teamd_log_dbgx > teamd/teamd_runner_activebackup.c: move teamd_log_dbg to > teamd_log_dbgx > teamd/teamd_balancer.c: move teamd_log_dbg to teamd_log_dbgx > teamd/teamd_runner_lacp.c: move teamd_log_dbg to teamd_log_dbgx > teamd/teamd_link_watch.c: move teamd_log_dbg to teamd_log_dbgx > teamd: move teamd_log_dbg to teamd_log_dbgx > > teamd/teamd.c | 40 ++++++++++++++-------------- > teamd/teamd_balancer.c | 21 ++++++++------- > teamd/teamd_dbus.c | 6 ++--- > teamd/teamd_hash_func.c | 2 +- > teamd/teamd_link_watch.c | 14 +++++----- > teamd/teamd_lw_arp_ping.c | 12 ++++----- > teamd/teamd_lw_ethtool.c | 4 +-- > teamd/teamd_lw_nsna_ping.c | 2 +- > teamd/teamd_lw_psr.c | 12 ++++----- > teamd/teamd_lw_tipc.c | 8 +++--- > teamd/teamd_per_port.c | 6 ++--- > teamd/teamd_runner_activebackup.c | 18 ++++++------- > teamd/teamd_runner_lacp.c | 44 +++++++++++++++---------------- > teamd/teamd_usock.c | 12 ++++----- > teamd/teamd_zmq.c | 10 +++---- > 15 files changed, 107 insertions(+), 104 deletions(-) > > -- > 2.19.2 >
Hi Jiri, Would you please help review this patch set? Some customers are complaining about the debug messages. Thanks Hangbin On Wed, Nov 06, 2019 at 05:48:49PM +0800, Hangbin Liu wrote: > Hi Jiri, > > I'm not sure if I should split the patch or just post one directly. Please > tell me if you feel the commit message are repeated and want only one patch. > > Recently some users reported that they start to see debug messages in their > syslogs even with daemon_verbosity_level = LOG_INFO and without -g option. > > Actually this issue is there at the begining, the user would see the debug > messages if they run teamd with -d option. The reason that most users did > not notice this is because they are using libteam via NetworkManager, and > NetworkManager run libteam in frontend. > > But after commit e47d5db53873 ("teamd: add an option to force log > output to stdout, stderr or syslog"), NetworkManager will set > TEAM_LOG_OUTPUT=syslog in the environment. At the same time libdaemon > does not filter log levels if we use syslog(see function daemon_logv in > libdaemon). Then all the users would see the debug messages suddenly and > feels annoying. > > And here is the quote for daemon_set_verbosity() from libdaemon/dlog.h > """ > Allows to decide which messages to output on standard output/error > streams. All messages are logged to syslog and this setting does > not influence that. > """ > > Since we should not limit how our user(NM) used libteam. And libdaemon > is intend to not filter logs if use syslog. We'd better filter the > debug message ourselves, like via -g option. So I would prefer to > move all teamd_log_dbg to teamd_log_dbgx. After that, the user could > decide whether to enable debug or not by themselves with -g option. > > Hangbin Liu (6): > teamd/teamd.c: move teamd_log_dbg to teamd_log_dbgx > teamd/teamd_runner_activebackup.c: move teamd_log_dbg to > teamd_log_dbgx > teamd/teamd_balancer.c: move teamd_log_dbg to teamd_log_dbgx > teamd/teamd_runner_lacp.c: move teamd_log_dbg to teamd_log_dbgx > teamd/teamd_link_watch.c: move teamd_log_dbg to teamd_log_dbgx > teamd: move teamd_log_dbg to teamd_log_dbgx > > teamd/teamd.c | 40 ++++++++++++++-------------- > teamd/teamd_balancer.c | 21 ++++++++------- > teamd/teamd_dbus.c | 6 ++--- > teamd/teamd_hash_func.c | 2 +- > teamd/teamd_link_watch.c | 14 +++++----- > teamd/teamd_lw_arp_ping.c | 12 ++++----- > teamd/teamd_lw_ethtool.c | 4 +-- > teamd/teamd_lw_nsna_ping.c | 2 +- > teamd/teamd_lw_psr.c | 12 ++++----- > teamd/teamd_lw_tipc.c | 8 +++--- > teamd/teamd_per_port.c | 6 ++--- > teamd/teamd_runner_activebackup.c | 18 ++++++------- > teamd/teamd_runner_lacp.c | 44 +++++++++++++++---------------- > teamd/teamd_usock.c | 12 ++++----- > teamd/teamd_zmq.c | 10 +++---- > 15 files changed, 107 insertions(+), 104 deletions(-) > > -- > 2.19.2 >
Wed, Nov 06, 2019 at 10:48:49AM CET, liuhangbin@gmail.com wrote: >Hi Jiri, > >I'm not sure if I should split the patch or just post one directly. Please >tell me if you feel the commit message are repeated and want only one patch. > >Recently some users reported that they start to see debug messages in their >syslogs even with daemon_verbosity_level = LOG_INFO and without -g option. > >Actually this issue is there at the begining, the user would see the debug >messages if they run teamd with -d option. The reason that most users did >not notice this is because they are using libteam via NetworkManager, and >NetworkManager run libteam in frontend. > >But after commit e47d5db53873 ("teamd: add an option to force log >output to stdout, stderr or syslog"), NetworkManager will set >TEAM_LOG_OUTPUT=syslog in the environment. At the same time libdaemon >does not filter log levels if we use syslog(see function daemon_logv in >libdaemon). Then all the users would see the debug messages suddenly and >feels annoying. > >And here is the quote for daemon_set_verbosity() from libdaemon/dlog.h >""" >Allows to decide which messages to output on standard output/error >streams. All messages are logged to syslog and this setting does >not influence that. >""" > >Since we should not limit how our user(NM) used libteam. And libdaemon >is intend to not filter logs if use syslog. We'd better filter the >debug message ourselves, like via -g option. So I would prefer to >move all teamd_log_dbg to teamd_log_dbgx. After that, the user could >decide whether to enable debug or not by themselves with -g option. So after this patchset, there is no user of teamd_log_dbg(), am I correct? I don't see you would remove it. I would prefer to keep teamd_log_dbg() and only extend it with ctx arg, having it as: #define teamd_log_dbg(ctx, args...) teamd_log_dbgx(ctx, 1, ##args) You can do it all in a single patch. Sorry for the late reply :/
Hi Jiri, I'm not sure if I should split the patch or just post one directly. Please tell me if you feel the commit message are repeated and want only one patch. Recently some users reported that they start to see debug messages in their syslogs even with daemon_verbosity_level = LOG_INFO and without -g option. Actually this issue is there at the begining, the user would see the debug messages if they run teamd with -d option. The reason that most users did not notice this is because they are using libteam via NetworkManager, and NetworkManager run libteam in frontend. But after commit e47d5db53873 ("teamd: add an option to force log output to stdout, stderr or syslog"), NetworkManager will set TEAM_LOG_OUTPUT=syslog in the environment. At the same time libdaemon does not filter log levels if we use syslog(see function daemon_logv in libdaemon). Then all the users would see the debug messages suddenly and feels annoying. And here is the quote for daemon_set_verbosity() from libdaemon/dlog.h """ Allows to decide which messages to output on standard output/error streams. All messages are logged to syslog and this setting does not influence that. """ Since we should not limit how our user(NM) used libteam. And libdaemon is intend to not filter logs if use syslog. We'd better filter the debug message ourselves, like via -g option. So I would prefer to move all teamd_log_dbg to teamd_log_dbgx. After that, the user could decide whether to enable debug or not by themselves with -g option. Hangbin Liu (6): teamd/teamd.c: move teamd_log_dbg to teamd_log_dbgx teamd/teamd_runner_activebackup.c: move teamd_log_dbg to teamd_log_dbgx teamd/teamd_balancer.c: move teamd_log_dbg to teamd_log_dbgx teamd/teamd_runner_lacp.c: move teamd_log_dbg to teamd_log_dbgx teamd/teamd_link_watch.c: move teamd_log_dbg to teamd_log_dbgx teamd: move teamd_log_dbg to teamd_log_dbgx teamd/teamd.c | 40 ++++++++++++++-------------- teamd/teamd_balancer.c | 21 ++++++++------- teamd/teamd_dbus.c | 6 ++--- teamd/teamd_hash_func.c | 2 +- teamd/teamd_link_watch.c | 14 +++++----- teamd/teamd_lw_arp_ping.c | 12 ++++----- teamd/teamd_lw_ethtool.c | 4 +-- teamd/teamd_lw_nsna_ping.c | 2 +- teamd/teamd_lw_psr.c | 12 ++++----- teamd/teamd_lw_tipc.c | 8 +++--- teamd/teamd_per_port.c | 6 ++--- teamd/teamd_runner_activebackup.c | 18 ++++++------- teamd/teamd_runner_lacp.c | 44 +++++++++++++++---------------- teamd/teamd_usock.c | 12 ++++----- teamd/teamd_zmq.c | 10 +++---- 15 files changed, 107 insertions(+), 104 deletions(-)