RE: [RFC PATCH v4] ndctl: monitor: add ndctl monitor daemon

["Qi, Fuli" <qi.fuli@jp.fujitsu.com>]

> -----Original Message-----
> From: Dan Williams [mailto:dan.j.williams@intel.com]
> Sent: Saturday, April 7, 2018 4:03 AM
> To: Qi, Fuli/斉 福利 <qi.fuli@jp.fujitsu.com>
> Cc: linux-nvdimm <linux-nvdimm@lists.01.org>
> Subject: Re: [RFC PATCH v4] ndctl: monitor: add ndctl monitor daemon
> 
> bus="<bus filter option>"
> 
> 
> On Thu, Apr 5, 2018 at 4:17 PM, Qi, Fuli <qi.fuli@jp.fujitsu.com> wrote:
> [..]
> >> This seems to needlessly tie ndctl to systemd, it should be able to
> >> operate without requiring systemd. I expect it would be
> >> straightforward to copy the configuration file implementation from git.
> >
> > I have read the configuration file implementation of git, my
> > understanding is that git daemon does not have any options used to override
> default configuration.
> > I want to confirm if the configuration file is only used for ndctl monitor.
> > If yes, I do not think copy the configuration file implementation from
> > git is a good choice, because only getting keys and values from
> > configuration file is needed for us and the structure of configuration file
> implementation in git is too complexity.
> > I prefer to borrow from udev[1], because the implementation in udev is simpler
> and it seems ndctl also borrows a lot from udev.
> >
> > [1]
> > https://git.kernel.org/pub/scm/linux/hotplug/udev.git/tree/src/libudev
> > .c
> 
> Thank you for doing the due diligence on this investigation it is appreciated.
> 
> I think the simple udev approach is acceptable. Going back to the proposed
> command line options of: --dimm-events, --namespace-events, --region-events,
> --bus-events and device filter selectors (like the ndctl list options) we can just have
> variables in the config file for those, so:
> 
> dimm-events="<list of dimm events>"
> namespace-events=="<list of namespace events>"
> region-events=="<list of region events>"
> bus-events="<list of bus events>"
> dimm="<bus filter option>"
> dimm="<dimm filter option>"
> region="<region filter option>"
> namespace="<namespace filter option>"
> 
> Thoughts?

Thank you very much.

I think the "logfile=<logfile path>" is also needed in the configuration file.

One more question is that do you think it is necessary for ndctl list to support "one option multiple arguments"?
Currently, only "one option one argument" is allowed in ndctl list like "ndctl list --dimm nmem1 --bus ndbus1".
Due to monitor should support "one option multiple arguments" like "ndctl monitor --dimm nmem1,nmem2",
I am thinking modify the "strut util_filter_params" in util/filter.h, change the "const char *bus" to "char **buses"
or "link_list bus" and refactor the util_filter_walk in util/filter.h, then ndctl list also can support "ndctl list --dimm nmem1,nmem2".
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm