All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Verma, Vishal L" <vishal.l.verma@intel.com>
To: "fukuri.sai@gmail.com" <fukuri.sai@gmail.com>,
	"nvdimm@lists.linux.dev" <nvdimm@lists.linux.dev>
Cc: "qi.fuli@fujitsu.com" <qi.fuli@fujitsu.com>
Subject: Re: [RFC ndctl PATCH 0/3] Rename monitor.conf to ndctl.conf as a ndctl global config file
Date: Wed, 2 Jun 2021 05:31:07 +0000	[thread overview]
Message-ID: <0e2b6f25a3ba8d20604f8c3aa4d8854ade0835c4.camel@intel.com> (raw)
In-Reply-To: <20210516231427.64162-1-qi.fuli@fujitsu.com>

[switching to the new mailing list]

On Mon, 2021-05-17 at 08:14 +0900, QI Fuli wrote:
> From: QI Fuli <qi.fuli@fujitsu.com>
> 
> 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 <qi.fuli@fujitsu.com>

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.

Thanks again!
-Vishal

> 
> QI Fuli (3):
>   ndctl, ccan: import ciniparser
>   ndctl, util: add parse-configs helper
>   ndctl, rename monitor.conf to ndctl.conf
> 
>  Makefile.am                        |   8 +-
>  ccan/ciniparser/ciniparser.c       | 480 +++++++++++++++++++++++++++++
>  ccan/ciniparser/ciniparser.h       | 262 ++++++++++++++++
>  ccan/ciniparser/dictionary.c       | 266 ++++++++++++++++
>  ccan/ciniparser/dictionary.h       | 166 ++++++++++
>  configure.ac                       |   8 +-
>  ndctl/Makefile.am                  |   9 +-
>  ndctl/monitor.c                    | 127 ++------
>  ndctl/{monitor.conf => ndctl.conf} |  16 +-
>  util/parse-configs.c               |  47 +++
>  util/parse-configs.h               |  26 ++
>  11 files changed, 1294 insertions(+), 121 deletions(-)
>  create mode 100644 ccan/ciniparser/ciniparser.c
>  create mode 100644 ccan/ciniparser/ciniparser.h
>  create mode 100644 ccan/ciniparser/dictionary.c
>  create mode 100644 ccan/ciniparser/dictionary.h
>  rename ndctl/{monitor.conf => ndctl.conf} (82%)
>  create mode 100644 util/parse-configs.c
>  create mode 100644 util/parse-configs.h
> 


  parent reply	other threads:[~2021-06-02  5:31 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-16 23:14 [RFC ndctl PATCH 0/3] Rename monitor.conf to ndctl.conf as a ndctl global config file QI Fuli
2021-05-16 23:14 ` [RFC ndctl PATCH 1/3] ndctl, ccan: import ciniparser QI Fuli
2021-05-16 23:14 ` [RFC ndctl PATCH 2/3] ndctl, util: add parse-configs helper QI Fuli
2021-05-16 23:14 ` [RFC ndctl PATCH 3/3] ndctl, rename monitor.conf to ndctl.conf QI Fuli
2021-06-02  6:33   ` Verma, Vishal L
2021-06-02  5:31 ` Verma, Vishal L [this message]
2021-06-02 16:47   ` [RFC ndctl PATCH 0/3] Rename monitor.conf to ndctl.conf as a ndctl global config file Dan Williams
2021-06-02 17:15     ` Verma, Vishal L
2021-06-17  0:25       ` qi.fuli
2021-07-08  6:34         ` Verma, Vishal L

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0e2b6f25a3ba8d20604f8c3aa4d8854ade0835c4.camel@intel.com \
    --to=vishal.l.verma@intel.com \
    --cc=fukuri.sai@gmail.com \
    --cc=nvdimm@lists.linux.dev \
    --cc=qi.fuli@fujitsu.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.