> On Wed, 2021-06-02 at 09:47 -0700, Dan Williams wrote: > > On Tue, Jun 1, 2021 at 10:31 PM Verma, Vishal L > > <[email protected]> wrote: > > > > > > [switching to the new mailing list] > > > > > > On Mon, 2021-05-17 at 08:14 +0900, QI Fuli wrote: > > > > From: QI Fuli <[email protected]> > > > > > > > > This patch set is to rename monitor.conf to ndctl.conf, and make > > > > it a global ndctl configuration file that all ndctl commands can refer > > > > to. > > > > > > > > As this patch set has been pending until now, I would like to know > > > > if current idea works or not. If yes, I will finish the documents and > > > > test. > > > > > > > > Signed-off-by: QI Fuli <[email protected]> > > > > > > Hi Qi, > > > > > > Thanks for picking up on this! The approach generally looks good to > > > me, I think we can definitely move forward with this direction. > > > > > > One thing that stands out is - I don't think we can simply rename > > > the existing monitor.conf. We have to keep supporting the 'legacy' > > > monitor.conf so that we don't break any deployments. I'd suggest > > > keeping the old monitor.conf as is, and continuing to parse it as > > > is, but also adding a new ndctl.conf as you have done. > > > > > > We can indicate that 'monitor.conf' is legacy, and any new features > > > will only get added to the new global config to encourage migration > > > to the new config. Perhaps we can even provide a helper script to > > > migrate the old config to new - but I think it needs to be a user > > > triggered action. > > > > > > This is timely as I also need to go add some config related > > > functionality to daxctl, and basing it on this would be perfect, so > > > I'd love to get this series merged in soon. > > > > I wonder if ndctl should treat /etc/ndctl like a conf.d directory of > > which all files with the .conf suffix are concatenated into one > > combined configuration file. I.e. something that maintains legacy, but > > also allows for config fragments to be deployed individually. > > Agreed, this would be the most flexible. ciniparser doesn't seem to support > multiple files directly, but perhaps ndctl can, early on, load up multiple > files/dictionaries, and stash them in ndctl_ctx. Then there can be accessor > functions to retrieve specific conf strings from that as needed by different > commands.
Thank you very much for the comments. I also agree, and I am working on the v2 patch set now. I found that the style of ndctl.conf is different from monitor.conf, since there is no section name in monitor.conf. Do I need to keep the legacy style in monitor.conf, i.e. Can I add the section name [monitor] to monitor.conf? Best regards, QI
