Re: [ndctl PATCH] monitor: removing the requirement of default config
On Mon, 2019-04-01 at 06:20 +, qi.f...@fujitsu.com wrote: > > Hi Vishal, > > Thanks for your comment. > I think it might be misunderstood. > > Current scenario is: > 1. Check if "-c " option is provided > a. if it is, make the as config_file > b. if it is not, make default configuration file(/etc/ndctl/monitor.conf) > as config_file > 2. Try to read options from config_file > a. if config_file cannot be opened, then check if "-c" option is provided >(1) if it is, stop starting monitor (user may make a wrong in "-c" > option) >(2) if it is not, finish read_config_file (default configuration > file(/etc/ndctl/monitor.conf) is missing) > b. if config_file can be opened successfully, merge the options from > config_file with cli options > > It is consistent with the ideal scenario as you mentioned. Hi Qi, Looking at it in more detail, I see you're right. I've applied the patch - thanks for the explanation. > > Thanks, > QI Fuli ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
RE: [ndctl PATCH] monitor: removing the requirement of default config
> -Original Message-
> From: Verma, Vishal L [mailto:vishal.l.ve...@intel.com]
> Sent: Saturday, March 30, 2019 2:37 AM
> To: linux-nvdimm@lists.01.org; Qi, Fuli/斉 福利
> Subject: Re: [ndctl PATCH] monitor: removing the requirement of default
> config
>
>
> On Fri, 2019-03-29 at 20:01 +0900, QI Fuli wrote:
> > Removing the requirement of default configuration file.
> > If "-c, --config-file" option is not specified, default configuration
> > file should be dispensable.
> >
> > Link: https://github.com/pmem/ndctl/issues/83
> > Signed-off-by: QI Fuli
> >
> > ---
> > ndctl/monitor.c | 7 +--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/ndctl/monitor.c b/ndctl/monitor.c
> > index 346a6df..6829a6b 100644
> > --- a/ndctl/monitor.c
> > +++ b/ndctl/monitor.c
> > @@ -484,8 +484,11 @@ static int read_config_file(struct ndctl_ctx *ctx,
> struct monitor *_monitor,
> >
> > f = fopen(config_file, "r");
> > if (!f) {
> > - err(, "config-file: %s cannot be opened\n",
> config_file);
> > - rc = -errno;
> > + if (_monitor->config_file) {
> > + err(, "config-file: %s cannot be
> opened\n",
> > + config_file);
> > + rc = -errno;
> > + }
> > goto out;
> > }
> >
> Hi Qi,
>
> Thanks for the quick patch. However, this makes it so that the only way
> in which a config file gets used is by explicitly specifying it with a
> -c option, and that kind of makes a 'default' config file pointless..
>
> The ideal scenario would be:
> 1. Check if default config exists
>a. if it does, use options from there
>b. if any cli options provided, use those to override the ones in
> config file (if any)
>
> 2. If default config file is missing, only use cli options
>
> 3. If -c provided, use that, but perform the same treatment as
> 1b
> above, i.e. any cli options override anything in the config file from
> -c
> as well.
Hi Vishal,
Thanks for your comment.
I think it might be misunderstood.
Current scenario is:
1. Check if "-c " option is provided
a. if it is, make the as config_file
b. if it is not, make default configuration file(/etc/ndctl/monitor.conf) as
config_file
2. Try to read options from config_file
a. if config_file cannot be opened, then check if "-c" option is provided
(1) if it is, stop starting monitor (user may make a wrong in "-c"
option)
(2) if it is not, finish read_config_file (default configuration
file(/etc/ndctl/monitor.conf) is missing)
b. if config_file can be opened successfully, merge the options from
config_file with cli options
It is consistent with the ideal scenario as you mentioned.
Thanks,
QI Fuli
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [ndctl PATCH] monitor: removing the requirement of default config
On Fri, 2019-03-29 at 20:01 +0900, QI Fuli wrote:
> Removing the requirement of default configuration file.
> If "-c, --config-file" option is not specified, default configuration
> file should be dispensable.
>
> Link: https://github.com/pmem/ndctl/issues/83
> Signed-off-by: QI Fuli
>
> ---
> ndctl/monitor.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/ndctl/monitor.c b/ndctl/monitor.c
> index 346a6df..6829a6b 100644
> --- a/ndctl/monitor.c
> +++ b/ndctl/monitor.c
> @@ -484,8 +484,11 @@ static int read_config_file(struct ndctl_ctx *ctx,
> struct monitor *_monitor,
>
> f = fopen(config_file, "r");
> if (!f) {
> - err(, "config-file: %s cannot be opened\n",
> config_file);
> - rc = -errno;
> + if (_monitor->config_file) {
> + err(, "config-file: %s cannot be opened\n",
> + config_file);
> + rc = -errno;
> + }
> goto out;
> }
>
Hi Qi,
Thanks for the quick patch. However, this makes it so that the only way
in which a config file gets used is by explicitly specifying it with a
-c option, and that kind of makes a 'default' config file pointless..
The ideal scenario would be:
1. Check if default config exists
a. if it does, use options from there
b. if any cli options provided, use those to override the ones in
config file (if any)
2. If default config file is missing, only use cli options
3. If -c provided, use that, but perform the same treatment as 1b
above, i.e. any cli options override anything in the config file from -c
as well.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
