On Thu, May 22, 2025 at 11:38:34AM -0700, Dan Williams wrote:
> Alison Schofield wrote:
> > On Wed, May 21, 2025 at 09:00:19AM +0000, Zhijian Li (Fujitsu) wrote:
> > >
> > >
> > > On 20/05/2025 03:28, alison.schofi...@intel.com wrote:
> > > > diff --git a/ndctl/monitor.c b/ndctl/monitor.c
> > > > index bd8a74863476..925b37f4184b 100644
> > > > --- a/ndctl/monitor.c
> > > > +++ b/ndctl/monitor.c
> > > > @@ -658,7 +658,7 @@ int cmd_monitor(int argc, const char **argv, struct
> > > > ndctl_ctx *ctx)
> > > > rc = -ENXIO;
> > > > goto out;
> > > > }
> > > > -
> > > > + info(&monitor, "monitor ready\n");
> > >
> > >
> > > This brings to mind my initial contribution to ndctl, where it commented
> > > that monitor expects to
> > > output content in json format[1]? So this update could break it, does it
> > > matter now?
> > >
> > > [1]
> > > https://lore.kernel.org/linux-cxl/4c2341c8a4e579e9643b7daa3eb412b0ac0da98a.ca...@intel.com/
> >
> > That is odd because right above where you wanted to add that info[] to
> > cxl/monitor.c another info[] was added to the log for the daemon starting ?
> >
> > ndctl/monitor.c has a few info[] going to the log.
> >
> > In the ndctl/monitor.c the presence of a log does not mean the monitor
> > started. I'll poke around more about the need for json. I get that in
> > theory, but I'm doubtful in practice that a json parser would die on
> > those info entries.
>
> It's not just monitor, the expectation for all binaries in the project
> is that all stdout is json and anything that is not json is only allowed
> on stderr. That way you can always use these utilities in tooling
> pipelines that do not need not to build a pile of grep and awk to
> extract useful data.
>
> It turns out, unfortunately, that LOG_INFO is the only log level in
> util/log.c that violates the expectation that all non-json goes to
> stderr. So I would support a change like this to remove that trap:
>
Ah, understood (now). Thanks & will do.
> diff --git a/util/log.c b/util/log.c
> index d4eef0a1fc5c..4ee85c7609c3 100644
> --- a/util/log.c
> +++ b/util/log.c
> @@ -18,10 +18,7 @@ void log_syslog(struct log_ctx *ctx, int priority, const
> char *file, int line,
> void log_standard(struct log_ctx *ctx, int priority, const char *file, int
> line,
> const char *fn, const char *format, va_list args)
> {
> - if (priority == 6)
> - vfprintf(stdout, format, args);
> - else
> - vfprintf(stderr, format, args);
> + vfprintf(stderr, format, args);
> }
>
> void log_file(struct log_ctx *ctx, int priority, const char *file, int line,
