Re: [RFC PATCH v3 2/5] ndctl: monitor: add ndctl create-monitor command

2018-02-11 Thread Dan Williams 
On Fri, Feb 9, 2018 at 12:02 AM, QI Fuli  wrote:
> This patch is used to add $ndctl create-monitor command, by which users can
> create a new monitor. Users can select the DIMMS to be monitored by using
> [--dimm] [--bus] [--region] [--namespace] options. The notifications can
> be outputed to a special file or syslog by using [--output] option, the
> special file will be placed under /var/log/ndctl. A name is also required for
> a monitor,so users can destroy the monitor by the name. When a monitor is
> created successfully, a file with same name will be created under
> /var/ndctl/monitor.
> Example:
> #ndctl create-monitor --monitor m_nmem1 --dimm nmem1 --output m_nmem.log

Hi Qi,

This is getting closer to where I want to see this go, but still some
architecture details to incorporate. I mentioned on the cover letter
that systemd can handle starting, stopping, and show the status of the
monitor. The other detail to incorporate is that monitor events can
come DIMMs, but also namespaces, regions, and the bus.

The event list I have collected to date is:

dimm-spares-remaining
dimm-media-temperature
dimm-controller-temperature
dimm-health-state
dimm-unclean-shutdown
dimm-detected
namespace-media-error
namespace-detected
region-media-error
region-detected
bus-media-error
bus-address-range-scrub-complete
bus-detected

...and I think all of those should be separate options, probably
something like the following, but I'd Vishal to comment if this scheme
can be handled with the bash tab-completion implementation:

   ndctl monitor --dimm-events=spares-remaining,media-temperature
--namespace-events=all --regions-events --bus=ACPI.NFIT

...where an empty ---events option is equivalent to
---events=all. Also, similar to "ndctl list" specifying
specific buses, namespaces, etc causes the monitor to filter the
objects based on those properties.

Since "ndctl list" already has this filtering implemented I'd like to
see it refactored and shared between the 2 implementations rather than
duplicated as is done in this patch. In other words rework cmd_list()
into a generic nvdimm object walking routine with callback functions
to 'list' or 'monitor' a given object that matches the filter.

Let me know if the above makes sense. I'm thinking the 'ndctl list'
refactoring might be something I need to handle.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RFC PATCH v3 1/5] ndctl: nvdimmd: add LOG_NOTICE level to

2018-02-11 Thread Dan Williams 
On Sun, Feb 11, 2018 at 12:18 PM, Dan Williams  wrote:
> On Fri, Feb 9, 2018 at 12:02 AM, QI Fuli  wrote:
>> This patch adds LOG_NOTICE level to log_priority for the notifications
>> of nvdimm dimm over threshold event. Currently, the LOG_INFO level
>> is too low, and the notifications are not up to LOG_ERROR level.
>>
>
> Looks good, applied.

Although, this patch has little to do with nvdimmd, so I'm changing
the subject to:

"ndctl, log: add support for LOG_NOTICE messages"
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [RFC PATCH v3 1/5] ndctl: nvdimmd: add LOG_NOTICE level to

2018-02-11 Thread Dan Williams 
On Fri, Feb 9, 2018 at 12:02 AM, QI Fuli  wrote:
> This patch adds LOG_NOTICE level to log_priority for the notifications
> of nvdimm dimm over threshold event. Currently, the LOG_INFO level
> is too low, and the notifications are not up to LOG_ERROR level.
>

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