RE: [ndctl PATCH 2/4] ndctl, monitor: set default log destination to syslog if "--daemon" is specified

2018-08-08 Thread Qi, Fuli 
> -Original Message-
> From: Masayoshi Mizuma [mailto:msys.miz...@gmail.com]
> Sent: Wednesday, August 8, 2018 10:35 PM
> To: Qi, Fuli/斉 福利 ; linux-nvdimm@lists.01.org
> Subject: Re: [ndctl PATCH 2/4] ndctl, monitor: set default log destination to 
> syslog if
> "--daemon" is specified
> 
> HiQi,
> 
> On 08/07/2018 09:31 PM, Qi, Fuli wrote:
> ...
> >> On 08/07/2018 09:17 AM, QI Fuli wrote:
> >>> When running monitor as a daemon, if the log destination is "standard"
> >>> or a relative path for log file, the messages will not be able to be 
> >>> logged.
> >>> Sometimes, users may not notice that the default log destination is 
> >>> "standard"
> >>> when they start monitor daemon by systemctl, so they will lose messages.
> >>> This patch is used to fix the unfriendly interface. When running
> >>> monitor as a daemon, the default log destination will be changed to
> >>> syslog. Also, the messages will be forwarded to syslog if the log
> >>> destination is
> >> a relative path for log file.
> >>>
> >>> Signed-off-by: QI Fuli 
> >>> ---
> >>>  Documentation/ndctl/ndctl-monitor.txt | 16 +++-
> >>>  ndctl/monitor.c   |  5 -
> >>>  ndctl/monitor.conf|  2 ++
> >>>  3 files changed, 21 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/Documentation/ndctl/ndctl-monitor.txt
> >>> b/Documentation/ndctl/ndctl-monitor.txt
> >>> index 1cba9ea..9a8d76b 100644
> >>> --- a/Documentation/ndctl/ndctl-monitor.txt
> >>> +++ b/Documentation/ndctl/ndctl-monitor.txt
> >>> @@ -67,7 +67,21 @@ OPTIONS
> >>>
> >>>  -l ::
> >>>  --log=::
> >>> - Output notifications to , syslog or standard output.
> >>> + Send log messages to the specified destination.
> >>> ++
> >>> +--
> >>> +::
> >>> + Send log messages to specified . When fopen() is not able
> >>> + to open , log messages will be forwarded to syslog.
> >>> +syslog::
> >>> + Send messages to syslog.
> >>> +standard::
> >>> + Send messages to standard output.
> >>> +--
> >>> ++
> >>> +The default log destination is 'syslog' if "--daemon" is specified,
> >>> +otherwise 'standard'. Note that standard and relative path for
> >>> + will not work if "--daemon" is specified.
> >>>
> >>>  -c::
> >>>  --config-file=::
> >>> diff --git a/ndctl/monitor.c b/ndctl/monitor.c index
> >>> bf1f1d3..2f3d751
> >>> 100644
> >>> --- a/ndctl/monitor.c
> >>> +++ b/ndctl/monitor.c
> >>> @@ -93,7 +93,8 @@ static void log_file(struct ndctl_ctx *ctx, int
> >>> priority, const
> >> char *file,
> >>>   f = fopen(monitor.log, "a+");
> >>>   if (!f) {
> >>>   ndctl_set_log_fn(ctx, log_syslog);
> >>> - err(ctx, "open logfile %s failed\n", monitor.log);
> >>> + err(ctx, "open logfile %s failed, forward messages to syslog\n",
> >>> + monitor.log);
> >>>   did_fail = 1;
> >>>   notice(ctx, "%s\n", buf);
> >>>   goto end;
> >>> @@ -644,6 +645,8 @@ int cmd_monitor(int argc, const char **argv, void 
> >>> *ctx)
> >>>   }
> >>>
> >>>   if (monitor.daemon) {
> >>
> >> Why don't you add './standard' check? Like as:
> >>
> >> if (strncmp(monitor.log, "./standard", 10) == 0)
> >> error("daemon doesn't work for 'standard' log 
> >> option");
> >> goto out;
> >>
> > Hi Masa,
> >
> > Thank you for your comment.
> >
> > When running monitor as a daemon, the messages will not be able to be 
> > logged in
> following cases.
> > a) Users set the log destination to standard by using [--log] option or 
> > setting value
> of "log" in config file.
> 
> > b) The log destination is standard by default.
> 
> Ummm... is this right behavior...? If the monitor running as a daemon, 
> shouldn't the
> default log destination be standard...?

No, If the monitor runs as a daemon, the default log destination should not be

Re: [ndctl PATCH 2/4] ndctl, monitor: set default log destination to syslog if "--daemon" is specified

2018-08-08 Thread Masayoshi Mizuma 
HiQi,

On 08/07/2018 09:31 PM, Qi, Fuli wrote:
...
>> On 08/07/2018 09:17 AM, QI Fuli wrote:
>>> When running monitor as a daemon, if the log destination is "standard"
>>> or a relative path for log file, the messages will not be able to be logged.
>>> Sometimes, users may not notice that the default log destination is 
>>> "standard"
>>> when they start monitor daemon by systemctl, so they will lose messages.
>>> This patch is used to fix the unfriendly interface. When running
>>> monitor as a daemon, the default log destination will be changed to
>>> syslog. Also, the messages will be forwarded to syslog if the log 
>>> destination is
>> a relative path for log file.
>>>
>>> Signed-off-by: QI Fuli 
>>> ---
>>>  Documentation/ndctl/ndctl-monitor.txt | 16 +++-
>>>  ndctl/monitor.c   |  5 -
>>>  ndctl/monitor.conf|  2 ++
>>>  3 files changed, 21 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/ndctl/ndctl-monitor.txt
>>> b/Documentation/ndctl/ndctl-monitor.txt
>>> index 1cba9ea..9a8d76b 100644
>>> --- a/Documentation/ndctl/ndctl-monitor.txt
>>> +++ b/Documentation/ndctl/ndctl-monitor.txt
>>> @@ -67,7 +67,21 @@ OPTIONS
>>>
>>>  -l ::
>>>  --log=::
>>> -   Output notifications to , syslog or standard output.
>>> +   Send log messages to the specified destination.
>>> ++
>>> +--
>>> +::
>>> +   Send log messages to specified . When fopen() is not able
>>> +   to open , log messages will be forwarded to syslog.
>>> +syslog::
>>> +   Send messages to syslog.
>>> +standard::
>>> +   Send messages to standard output.
>>> +--
>>> ++
>>> +The default log destination is 'syslog' if "--daemon" is specified,
>>> +otherwise 'standard'. Note that standard and relative path for 
>>> +will not work if "--daemon" is specified.
>>>
>>>  -c::
>>>  --config-file=::
>>> diff --git a/ndctl/monitor.c b/ndctl/monitor.c index bf1f1d3..2f3d751
>>> 100644
>>> --- a/ndctl/monitor.c
>>> +++ b/ndctl/monitor.c
>>> @@ -93,7 +93,8 @@ static void log_file(struct ndctl_ctx *ctx, int priority, 
>>> const
>> char *file,
>>> f = fopen(monitor.log, "a+");
>>> if (!f) {
>>> ndctl_set_log_fn(ctx, log_syslog);
>>> -   err(ctx, "open logfile %s failed\n", monitor.log);
>>> +   err(ctx, "open logfile %s failed, forward messages to syslog\n",
>>> +   monitor.log);
>>> did_fail = 1;
>>> notice(ctx, "%s\n", buf);
>>> goto end;
>>> @@ -644,6 +645,8 @@ int cmd_monitor(int argc, const char **argv, void *ctx)
>>> }
>>>
>>> if (monitor.daemon) {
>>
>> Why don't you add './standard' check? Like as:
>>
>> if (strncmp(monitor.log, "./standard", 10) == 0)
>> error("daemon doesn't work for 'standard' log 
>> option");
>> goto out;
>>
> Hi Masa,
> 
> Thank you for your comment.
> 
> When running monitor as a daemon, the messages will not be able to be logged 
> in following cases.
> a) Users set the log destination to standard by using [--log] option or 
> setting value of "log" in config file.

> b) The log destination is standard by default.

Ummm... is this right behavior...? If the monitor running as a daemon,
shouldn't the default log destination be standard...? 
How about the following change?

diff --git a/ndctl/monitor.c b/ndctl/monitor.c
index f10384b..3778334 100644
--- a/ndctl/monitor.c
+++ b/ndctl/monitor.c
@@ -611,8 +611,11 @@ int cmd_monitor(int argc, const char **argv, void *ctx)
if (argc)
usage_with_options(u, options);
 
-   /* default to log_standard */
-   ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_standard);
+   /* Set default log destination */
+   if (monitor.daemon)
+   ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_syslog);
+   else
+   ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_standard);
 
if (monitor.verbose)
ndctl_set_log_priority((struct ndctl_ctx *)ctx, LOG_DEBUG);

> c) Users set the log destination to a relative path of log file by using 
> [--log] option or setting value of "log" in config file.
> 
> The './standard' check will only works for case a).

I'm sorry, but I'm not clear about c)... The './standard' check should not have
the effect in case of c), right?
We should check the following three cases for monitor daemon, right?

1. '--log standard' option, or
2. In config file 'log = standard'
3. Both 1. and 2. is not set (default behavior).

If I miss something, sorry about that...

Thanks,
Masa

> 
> Also, it would be more friendly to set a default log destination to monitor 
> daemon.
> 
> Thanks,
> QI
> 
 >> Thanks,
>> Masa
>>
>>> +   if (!monitor.log || strncmp(monitor.log, "./", 2) == 0)
>>> +   ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_syslog);
>>
>>> if (daemon(0, 0) != 0) {
>>> err((struct ndctl_ctx *)ctx, "daemon start

RE: [ndctl PATCH 2/4] ndctl, monitor: set default log destination to syslog if "--daemon" is specified

2018-08-07 Thread Qi, Fuli 
> -Original Message-
> From: Masayoshi Mizuma [mailto:msys.miz...@gmail.com]
> Sent: Wednesday, August 8, 2018 4:30 AM
> To: Qi, Fuli/斉 福利 ; linux-nvdimm@lists.01.org
> Subject: Re: [ndctl PATCH 2/4] ndctl, monitor: set default log destination to 
> syslog
> if "--daemon" is specified
> 
> Hi Qi,
> 
> On 08/07/2018 09:17 AM, QI Fuli wrote:
> > When running monitor as a daemon, if the log destination is "standard"
> > or a relative path for log file, the messages will not be able to be logged.
> > Sometimes, users may not notice that the default log destination is 
> > "standard"
> > when they start monitor daemon by systemctl, so they will lose messages.
> > This patch is used to fix the unfriendly interface. When running
> > monitor as a daemon, the default log destination will be changed to
> > syslog. Also, the messages will be forwarded to syslog if the log 
> > destination is
> a relative path for log file.
> >
> > Signed-off-by: QI Fuli 
> > ---
> >  Documentation/ndctl/ndctl-monitor.txt | 16 +++-
> >  ndctl/monitor.c   |  5 -
> >  ndctl/monitor.conf|  2 ++
> >  3 files changed, 21 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/ndctl/ndctl-monitor.txt
> > b/Documentation/ndctl/ndctl-monitor.txt
> > index 1cba9ea..9a8d76b 100644
> > --- a/Documentation/ndctl/ndctl-monitor.txt
> > +++ b/Documentation/ndctl/ndctl-monitor.txt
> > @@ -67,7 +67,21 @@ OPTIONS
> >
> >  -l ::
> >  --log=::
> > -   Output notifications to , syslog or standard output.
> > +   Send log messages to the specified destination.
> > ++
> > +--
> > +::
> > +   Send log messages to specified . When fopen() is not able
> > +   to open , log messages will be forwarded to syslog.
> > +syslog::
> > +   Send messages to syslog.
> > +standard::
> > +   Send messages to standard output.
> > +--
> > ++
> > +The default log destination is 'syslog' if "--daemon" is specified,
> > +otherwise 'standard'. Note that standard and relative path for 
> > +will not work if "--daemon" is specified.
> >
> >  -c::
> >  --config-file=::
> > diff --git a/ndctl/monitor.c b/ndctl/monitor.c index bf1f1d3..2f3d751
> > 100644
> > --- a/ndctl/monitor.c
> > +++ b/ndctl/monitor.c
> > @@ -93,7 +93,8 @@ static void log_file(struct ndctl_ctx *ctx, int priority, 
> > const
> char *file,
> > f = fopen(monitor.log, "a+");
> > if (!f) {
> > ndctl_set_log_fn(ctx, log_syslog);
> > -   err(ctx, "open logfile %s failed\n", monitor.log);
> > +   err(ctx, "open logfile %s failed, forward messages to syslog\n",
> > +   monitor.log);
> > did_fail = 1;
> > notice(ctx, "%s\n", buf);
> > goto end;
> > @@ -644,6 +645,8 @@ int cmd_monitor(int argc, const char **argv, void *ctx)
> > }
> >
> > if (monitor.daemon) {
> 
> Why don't you add './standard' check? Like as:
> 
> if (strncmp(monitor.log, "./standard", 10) == 0)
> error("daemon doesn't work for 'standard' log 
> option");
> goto out;
> 
Hi Masa,

Thank you for your comment.

When running monitor as a daemon, the messages will not be able to be logged in 
following cases.
a) Users set the log destination to standard by using [--log] option or setting 
value of "log" in config file.
b) The log destination is standard by default.
c) Users set the log destination to a relative path of log file by using 
[--log] option or setting value of "log" in config file.

The './standard' check will only works for case a).

Also, it would be more friendly to set a default log destination to monitor 
daemon.

Thanks,
QI

> Thanks,
> Masa
> 
> > +   if (!monitor.log || strncmp(monitor.log, "./", 2) == 0)
> > +   ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_syslog);
> 
> > if (daemon(0, 0) != 0) {
> > err((struct ndctl_ctx *)ctx, "daemon start failed\n");
> > goto out;
> > diff --git a/ndctl/monitor.conf b/ndctl/monitor.conf index
> > 857aadf..934e2c0 100644
> > --- a/ndctl/monitor.conf
> > +++ b/ndctl/monitor.conf
> > @@ -38,4 +38,6 @@
> >  # to standard output (log=standard) or to write into a special file
> > (log=)  # by setting key "log". If this value is in conflict
> > with the value of  # [--log=] option, this value will be ignored.
> > +# Note: Setting value to "standard" or relative path for  will
> > +not work # when running moniotr as a daemon.
> >  # log = /var/log/ndctl/monitor.log
> >
> 

___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 2/4] ndctl, monitor: set default log destination to syslog if "--daemon" is specified

2018-08-07 Thread Masayoshi Mizuma 
Hi Qi,

On 08/07/2018 09:17 AM, QI Fuli wrote:
> When running monitor as a daemon, if the log destination is "standard" or
> a relative path for log file, the messages will not be able to be logged.
> Sometimes, users may not notice that the default log destination is "standard"
> when they start monitor daemon by systemctl, so they will lose messages.
> This patch is used to fix the unfriendly interface. When running monitor as a
> daemon, the default log destination will be changed to syslog. Also, the 
> messages
> will be forwarded to syslog if the log destination is a relative path for log 
> file.
> 
> Signed-off-by: QI Fuli 
> ---
>  Documentation/ndctl/ndctl-monitor.txt | 16 +++-
>  ndctl/monitor.c   |  5 -
>  ndctl/monitor.conf|  2 ++
>  3 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/ndctl/ndctl-monitor.txt 
> b/Documentation/ndctl/ndctl-monitor.txt
> index 1cba9ea..9a8d76b 100644
> --- a/Documentation/ndctl/ndctl-monitor.txt
> +++ b/Documentation/ndctl/ndctl-monitor.txt
> @@ -67,7 +67,21 @@ OPTIONS
>  
>  -l ::
>  --log=::
> - Output notifications to , syslog or standard output.
> + Send log messages to the specified destination.
> ++
> +--
> +::
> + Send log messages to specified . When fopen() is not able
> + to open , log messages will be forwarded to syslog.
> +syslog::
> + Send messages to syslog.
> +standard::
> + Send messages to standard output.
> +--
> ++
> +The default log destination is 'syslog' if "--daemon" is specified,
> +otherwise 'standard'. Note that standard and relative path for 
> +will not work if "--daemon" is specified.
>  
>  -c::
>  --config-file=::
> diff --git a/ndctl/monitor.c b/ndctl/monitor.c
> index bf1f1d3..2f3d751 100644
> --- a/ndctl/monitor.c
> +++ b/ndctl/monitor.c
> @@ -93,7 +93,8 @@ static void log_file(struct ndctl_ctx *ctx, int priority, 
> const char *file,
>   f = fopen(monitor.log, "a+");
>   if (!f) {
>   ndctl_set_log_fn(ctx, log_syslog);
> - err(ctx, "open logfile %s failed\n", monitor.log);
> + err(ctx, "open logfile %s failed, forward messages to syslog\n",
> + monitor.log);
>   did_fail = 1;
>   notice(ctx, "%s\n", buf);
>   goto end;
> @@ -644,6 +645,8 @@ int cmd_monitor(int argc, const char **argv, void *ctx)
>   }
>  
>   if (monitor.daemon) {

Why don't you add './standard' check? Like as:

if (strncmp(monitor.log, "./standard", 10) == 0)
error("daemon doesn't work for 'standard' log option");
goto out;

Thanks,
Masa

> + if (!monitor.log || strncmp(monitor.log, "./", 2) == 0)
> + ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_syslog);

>   if (daemon(0, 0) != 0) {
>   err((struct ndctl_ctx *)ctx, "daemon start failed\n");
>   goto out;
> diff --git a/ndctl/monitor.conf b/ndctl/monitor.conf
> index 857aadf..934e2c0 100644
> --- a/ndctl/monitor.conf
> +++ b/ndctl/monitor.conf
> @@ -38,4 +38,6 @@
>  # to standard output (log=standard) or to write into a special file 
> (log=)
>  # by setting key "log". If this value is in conflict with the value of
>  # [--log=] option, this value will be ignored.
> +# Note: Setting value to "standard" or relative path for  will not work
> +# when running moniotr as a daemon.
>  # log = /var/log/ndctl/monitor.log
> 
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[ndctl PATCH 2/4] ndctl, monitor: set default log destination to syslog if "--daemon" is specified

2018-08-07 Thread QI Fuli 
When running monitor as a daemon, if the log destination is "standard" or
a relative path for log file, the messages will not be able to be logged.
Sometimes, users may not notice that the default log destination is "standard"
when they start monitor daemon by systemctl, so they will lose messages.
This patch is used to fix the unfriendly interface. When running monitor as a
daemon, the default log destination will be changed to syslog. Also, the 
messages
will be forwarded to syslog if the log destination is a relative path for log 
file.

Signed-off-by: QI Fuli 
---
 Documentation/ndctl/ndctl-monitor.txt | 16 +++-
 ndctl/monitor.c   |  5 -
 ndctl/monitor.conf|  2 ++
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/Documentation/ndctl/ndctl-monitor.txt 
b/Documentation/ndctl/ndctl-monitor.txt
index 1cba9ea..9a8d76b 100644
--- a/Documentation/ndctl/ndctl-monitor.txt
+++ b/Documentation/ndctl/ndctl-monitor.txt
@@ -67,7 +67,21 @@ OPTIONS
 
 -l ::
 --log=::
-   Output notifications to , syslog or standard output.
+   Send log messages to the specified destination.
++
+--
+::
+   Send log messages to specified . When fopen() is not able
+   to open , log messages will be forwarded to syslog.
+syslog::
+   Send messages to syslog.
+standard::
+   Send messages to standard output.
+--
++
+The default log destination is 'syslog' if "--daemon" is specified,
+otherwise 'standard'. Note that standard and relative path for 
+will not work if "--daemon" is specified.
 
 -c::
 --config-file=::
diff --git a/ndctl/monitor.c b/ndctl/monitor.c
index bf1f1d3..2f3d751 100644
--- a/ndctl/monitor.c
+++ b/ndctl/monitor.c
@@ -93,7 +93,8 @@ static void log_file(struct ndctl_ctx *ctx, int priority, 
const char *file,
f = fopen(monitor.log, "a+");
if (!f) {
ndctl_set_log_fn(ctx, log_syslog);
-   err(ctx, "open logfile %s failed\n", monitor.log);
+   err(ctx, "open logfile %s failed, forward messages to syslog\n",
+   monitor.log);
did_fail = 1;
notice(ctx, "%s\n", buf);
goto end;
@@ -644,6 +645,8 @@ int cmd_monitor(int argc, const char **argv, void *ctx)
}
 
if (monitor.daemon) {
+   if (!monitor.log || strncmp(monitor.log, "./", 2) == 0)
+   ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_syslog);
if (daemon(0, 0) != 0) {
err((struct ndctl_ctx *)ctx, "daemon start failed\n");
goto out;
diff --git a/ndctl/monitor.conf b/ndctl/monitor.conf
index 857aadf..934e2c0 100644
--- a/ndctl/monitor.conf
+++ b/ndctl/monitor.conf
@@ -38,4 +38,6 @@
 # to standard output (log=standard) or to write into a special file 
(log=)
 # by setting key "log". If this value is in conflict with the value of
 # [--log=] option, this value will be ignored.
+# Note: Setting value to "standard" or relative path for  will not work
+# when running moniotr as a daemon.
 # log = /var/log/ndctl/monitor.log
-- 
2.18.0


___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm