diff mbox series

[ndctl,v3,4/6] cxl/monitor: always log started message

Message ID 20230531021936.7366-5-lizhijian@fujitsu.com (mailing list archive)
State Superseded
Headers show
Series cxl/monitor and ndctl/monitor fixes | expand

Commit Message

Zhijian Li (Fujitsu) May 31, 2023, 2:19 a.m. UTC
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(-)

Comments

Dave Jiang July 5, 2023, 6:21 p.m. UTC | #1
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);
>
Verma, Vishal L July 5, 2023, 9:16 p.m. UTC | #2
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);
>
Zhijian Li (Fujitsu) July 10, 2023, 1:05 p.m. UTC | #3
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 mbox series

Patch

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);