| Message ID | 20230531021936.7366-5-lizhijian@fujitsu.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl/monitor and ndctl/monitor fixes | expand |
On 5/30/23 19:19, Li Zhijian wrote: > Tell people monitor is starting rather only daemon mode will log this > message before. It's a minor fix so that whatever stdout or logfile > is specified, people can see a *normal* message. > > After this patch > # cxl monitor > cxl monitor started. > ^C > # cxl monitor -l standard.log > ^C > # cat standard.log > [1684735993.704815571] [818499] cxl monitor started. > # cxl monitor --daemon -l /var/log/daemon.log > # cat /var/log/daemon.log > [1684736075.817150494] [818509] cxl monitor started. > > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> > --- > V2: commit log updated # Dave > --- > cxl/monitor.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/cxl/monitor.c b/cxl/monitor.c > index 179646562187..0736483cc50a 100644 > --- a/cxl/monitor.c > +++ b/cxl/monitor.c > @@ -205,8 +205,8 @@ int cmd_monitor(int argc, const char **argv, struct cxl_ctx *ctx) > err(&monitor, "daemon start failed\n"); > goto out; > } > - info(&monitor, "cxl monitor daemon started.\n"); > } > + info(&monitor, "cxl monitor started.\n"); > > rc = monitor_event(ctx); >
On Wed, 2023-05-31 at 10:19 +0800, Li Zhijian wrote: > Tell people monitor is starting rather only daemon mode will log this > message before. It's a minor fix so that whatever stdout or logfile > is specified, people can see a *normal* message. > > After this patch > # cxl monitor > cxl monitor started. > ^C > # cxl monitor -l standard.log > ^C > # cat standard.log > [1684735993.704815571] [818499] cxl monitor started. > # cxl monitor --daemon -l /var/log/daemon.log > # cat /var/log/daemon.log > [1684736075.817150494] [818509] cxl monitor started. > > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> > --- > V2: commit log updated # Dave > --- > cxl/monitor.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/cxl/monitor.c b/cxl/monitor.c > index 179646562187..0736483cc50a 100644 > --- a/cxl/monitor.c > +++ b/cxl/monitor.c > @@ -205,8 +205,8 @@ int cmd_monitor(int argc, const char **argv, struct cxl_ctx *ctx) > err(&monitor, "daemon start failed\n"); > goto out; > } > - info(&monitor, "cxl monitor daemon started.\n"); > } > + info(&monitor, "cxl monitor started.\n"); Actually I don't think this message should go in a log file at all. It is okay to print this on stderr only in all cases, but if the monitor log is meant to contain say json, such a message will break that. Regardless of the format, the fact that the file is present is enough to see that the monitor was started. > > rc = monitor_event(ctx); >
on 7/6/2023 5:16 AM, Verma, Vishal L wrote: > On Wed, 2023-05-31 at 10:19 +0800, Li Zhijian wrote: >> Tell people monitor is starting rather only daemon mode will log this >> message before. It's a minor fix so that whatever stdout or logfile >> is specified, people can see a *normal* message. >> >> After this patch >> # cxl monitor >> cxl monitor started. >> ^C >> # cxl monitor -l standard.log >> ^C >> # cat standard.log >> [1684735993.704815571] [818499] cxl monitor started. >> # cxl monitor --daemon -l /var/log/daemon.log >> # cat /var/log/daemon.log >> [1684736075.817150494] [818509] cxl monitor started. >> >> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> >> --- >> V2: commit log updated # Dave >> --- >> cxl/monitor.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/cxl/monitor.c b/cxl/monitor.c >> index 179646562187..0736483cc50a 100644 >> --- a/cxl/monitor.c >> +++ b/cxl/monitor.c >> @@ -205,8 +205,8 @@ int cmd_monitor(int argc, const char **argv, struct cxl_ctx *ctx) >> err(&monitor, "daemon start failed\n"); >> goto out; >> } >> - info(&monitor, "cxl monitor daemon started.\n"); >> } >> + info(&monitor, "cxl monitor started.\n"); > Actually I don't think this message should go in a log file at all. It > is okay to print this on stderr only in all cases, but if the monitor > log is meant to contain say json, such a message will break that. wow, you are right. I'm fine to delete the message. Let's drop this patch so that it's consistent with message of 'ndctl monitor'. Thanks Zhijian > > Regardless of the format, the fact that the file is present is enough > to see that the monitor was started. > >> >> rc = monitor_event(ctx); >>
diff --git a/cxl/monitor.c b/cxl/monitor.c index 179646562187..0736483cc50a 100644 --- a/cxl/monitor.c +++ b/cxl/monitor.c @@ -205,8 +205,8 @@ int cmd_monitor(int argc, const char **argv, struct cxl_ctx *ctx) err(&monitor, "daemon start failed\n"); goto out; } - info(&monitor, "cxl monitor daemon started.\n"); } + info(&monitor, "cxl monitor started.\n"); rc = monitor_event(ctx);
Tell people monitor is starting rather only daemon mode will log this message before. It's a minor fix so that whatever stdout or logfile is specified, people can see a *normal* message. After this patch # cxl monitor cxl monitor started. ^C # cxl monitor -l standard.log ^C # cat standard.log [1684735993.704815571] [818499] cxl monitor started. # cxl monitor --daemon -l /var/log/daemon.log # cat /var/log/daemon.log [1684736075.817150494] [818509] cxl monitor started. Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> --- V2: commit log updated # Dave --- cxl/monitor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)