Re: [RFC PATCH v3 0/5] ndctl: monitor: monitor the smart events of

2018-02-12 Thread Qi, Fuli

On 2018/02/10 13:06, Dan Williams wrote:


On Fri, Feb 9, 2018 at 12:02 AM, QI Fuli  wrote:

This is a patch set of ndctl monitor, a tiny daemon to monitor the smart
events of nvdimm dimms. When a smart event fires, monitor will output
the notification which including dimm health status to syslog or a
special file according to users' configuration. The output notification
follows json format and can be consumed by log collectors like Fluentd.

Currently, I implemeted the following four commands to control monitor daemon.
$ndctl create-monitor
$ndctl list-monitor
$ndctl show-monitor
$ndclt destroy-monitor

I will appreciate if you could give some comments.

Nice to see this new update.

I have a high level comment on the proposed command structure, as far
as I can see the only new command we need is:

ndctl monitor

...that launches a monitor. We can have options to control whether
that monitor is a one-shot check of events sources, or a daemon that
forks into the background, but the 'create', 'list', 'show', and
'destroy' are the responsibility of systemd or System V services. For
example:

 create: systemctl start ndctl-monitor
 show: systemctl status ndctl-monitor
 destroy: systemctl stop ndctl-monitor



Thank you for your comments, I will use systemd in next version.


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


Re: [RFC PATCH v3 0/5] ndctl: monitor: monitor the smart events of

2018-02-12 Thread Qi, Fuli



On 2018/02/10 3:06, Verma, Vishal L wrote:

On Fri, 2018-02-09 at 17:02 +0900, QI Fuli wrote:

This is a patch set of ndctl monitor, a tiny daemon to monitor the
smart
events of nvdimm dimms. When a smart event fires, monitor will output
the notification which including dimm health status to syslog or a
special file according to users' configuration. The output
notification
follows json format and can be consumed by log collectors like
Fluentd.

Currently, I implemeted the following four commands to control
monitor daemon.
$ndctl create-monitor
$ndctl list-monitor
$ndctl show-monitor
$ndclt destroy-monitor

I will appreciate if you could give some comments.

Change log since v2:
  - Changing the interface of daemon to the ndctl command line
  - Changing the name of daemon form "nvdimmd" to "monitor"
  - Removing the config file, unit_file, nvdimmd dir
  - Removing nvdimmd_test program
  - Adding ndctl/monitor.c

Change log since v1:
  - Adding a config file(/etc/nvdimmd/nvdimmd.conf)
  - Using struct log_ctx instead of syslog()
 - Using log_syslog() to save the notify messages to syslog
 - Using log_file() to save the notify messages to special file
  - Adding LOG_NOTICE level to log_priority
  - Using automake instead of Makefile
  - Adding a new util file(nvdimmd/util.c) including helper functions
needed for nvdimm daemon
  - Adding nvdimmd_test program

QI Fuli (5):
   ndctl: monitor: add LOG_NOTICE level to log_priority
   ndctl: monitor: add ndclt create-monitor command
   ndctl: monitor: add ndclt list-monitor command
   ndctl: monitor: add ndclt show-monitor command
   ndctl: monitor: add ndclt destroy-monitor command

 ^
I haven't had a chance to look at the rest of the series, but spotted a
quick typo - this is in the subject line of each commit as well:
s/ndclt/ndctl/

Thank you for your comment, I will be more careful next time.



  builtin.h |   4 +
  configure.ac  |   3 +
  ndctl/Makefile.am |   3 +-
  ndctl/monitor.c   | 463
++
  ndctl/ndctl.c |   4 +
  util/log.c|   2 +
  util/log.h|   3 +
  7 files changed, 481 insertions(+), 1 deletion(-)
  create mode 100644 ndctl/monitor.c




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


Re: [RFC PATCH v3 0/5] ndctl: monitor: monitor the smart events of

2018-02-09 Thread Dan Williams
On Fri, Feb 9, 2018 at 12:02 AM, QI Fuli  wrote:
> This is a patch set of ndctl monitor, a tiny daemon to monitor the smart
> events of nvdimm dimms. When a smart event fires, monitor will output
> the notification which including dimm health status to syslog or a
> special file according to users' configuration. The output notification
> follows json format and can be consumed by log collectors like Fluentd.
>
> Currently, I implemeted the following four commands to control monitor daemon.
> $ndctl create-monitor
> $ndctl list-monitor
> $ndctl show-monitor
> $ndclt destroy-monitor
>
> I will appreciate if you could give some comments.

Nice to see this new update.

I have a high level comment on the proposed command structure, as far
as I can see the only new command we need is:

   ndctl monitor

...that launches a monitor. We can have options to control whether
that monitor is a one-shot check of events sources, or a daemon that
forks into the background, but the 'create', 'list', 'show', and
'destroy' are the responsibility of systemd or System V services. For
example:

create: systemctl start ndctl-monitor
show: systemctl status ndctl-monitor
destroy: systemctl stop ndctl-monitor
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [RFC PATCH v3 0/5] ndctl: monitor: monitor the smart events of

2018-02-09 Thread Verma, Vishal L

On Fri, 2018-02-09 at 17:02 +0900, QI Fuli wrote:
> This is a patch set of ndctl monitor, a tiny daemon to monitor the
> smart
> events of nvdimm dimms. When a smart event fires, monitor will output
> the notification which including dimm health status to syslog or a
> special file according to users' configuration. The output
> notification
> follows json format and can be consumed by log collectors like
> Fluentd.
> 
> Currently, I implemeted the following four commands to control
> monitor daemon.
> $ndctl create-monitor
> $ndctl list-monitor
> $ndctl show-monitor
> $ndclt destroy-monitor
> 
> I will appreciate if you could give some comments.
> 
> Change log since v2:
>  - Changing the interface of daemon to the ndctl command line
>  - Changing the name of daemon form "nvdimmd" to "monitor"
>  - Removing the config file, unit_file, nvdimmd dir
>  - Removing nvdimmd_test program
>  - Adding ndctl/monitor.c
> 
> Change log since v1:
>  - Adding a config file(/etc/nvdimmd/nvdimmd.conf)
>  - Using struct log_ctx instead of syslog()
> - Using log_syslog() to save the notify messages to syslog
> - Using log_file() to save the notify messages to special file
>  - Adding LOG_NOTICE level to log_priority
>  - Using automake instead of Makefile
>  - Adding a new util file(nvdimmd/util.c) including helper functions
>needed for nvdimm daemon
>  - Adding nvdimmd_test program
> 
> QI Fuli (5):
>   ndctl: monitor: add LOG_NOTICE level to log_priority
>   ndctl: monitor: add ndclt create-monitor command
>   ndctl: monitor: add ndclt list-monitor command
>   ndctl: monitor: add ndclt show-monitor command
>   ndctl: monitor: add ndclt destroy-monitor command
^
I haven't had a chance to look at the rest of the series, but spotted a
quick typo - this is in the subject line of each commit as well:
s/ndclt/ndctl/

> 
>  builtin.h |   4 +
>  configure.ac  |   3 +
>  ndctl/Makefile.am |   3 +-
>  ndctl/monitor.c   | 463
> ++
>  ndctl/ndctl.c |   4 +
>  util/log.c|   2 +
>  util/log.h|   3 +
>  7 files changed, 481 insertions(+), 1 deletion(-)
>  create mode 100644 ndctl/monitor.c
> 
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm