[PATCH] rteval: Allow configuration from stdin

[PATCH] rteval: Allow configuration from stdin

[Gabriele Monaco]

Modules can currently be set only via configuration file, if rteval is
called from a script run on different systems, the only way to make sure
the loaded modules are known is to provide (or generate) a configuration
file and pass it via -f/--infile .

Add the possibility to use - as input file to read the configuration via
standard input.

This allows something like:

 # rteval -f - << EOF
[measurement]
timerlat: module

[loads]
stressng:  module
EOF

Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
---
 rteval/rtevalConfig.py | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/rteval/rtevalConfig.py b/rteval/rtevalConfig.py
index 0c2fc6b..4c41ed7 100644
--- a/rteval/rtevalConfig.py
+++ b/rteval/rtevalConfig.py
@@ -238,10 +238,14 @@ class rtevalConfig:
             # Don't try to reread this file if it's already been parsed
             return
 
-        self.__info(f"reading config file {cfgfile}")
         ini = configparser.ConfigParser()
         ini.optionxform = str
-        ini.read(cfgfile)
+        if cfgfile == "-":
+            self.__info("reading config file stdin")
+            ini.read_file(sys.stdin)
+        else:
+            self.__info(f"reading config file {cfgfile}")
+            ini.read(cfgfile)
 
         # wipe any previously read config info
         if not append:

base-commit: 5edb31a58bea3c2433e817439e053b79221a06df
-- 
2.47.1

Re: [PATCH] rteval: Allow configuration from stdin

[Tomas Glozar]

po 13. 1. 2025 v 13:19 odesílatel Gabriele Monaco <gmonaco@redhat.com> napsal:
>
> Modules can currently be set only via configuration file, if rteval is
> called from a script run on different systems, the only way to make sure
> the loaded modules are known is to provide (or generate) a configuration
> file and pass it via -f/--infile .
>
> Add the possibility to use - as input file to read the configuration via
> standard input.
>
> This allows something like:
>
>  # rteval -f - << EOF
> [measurement]
> timerlat: module
>
> [loads]
> stressng:  module
> EOF
>
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
> ---
>  rteval/rtevalConfig.py | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/rteval/rtevalConfig.py b/rteval/rtevalConfig.py
> index 0c2fc6b..4c41ed7 100644
> --- a/rteval/rtevalConfig.py
> +++ b/rteval/rtevalConfig.py
> @@ -238,10 +238,14 @@ class rtevalConfig:
>              # Don't try to reread this file if it's already been parsed
>              return
>
> -        self.__info(f"reading config file {cfgfile}")
>          ini = configparser.ConfigParser()
>          ini.optionxform = str
> -        ini.read(cfgfile)
> +        if cfgfile == "-":
> +            self.__info("reading config file stdin")
> +            ini.read_file(sys.stdin)
> +        else:
> +            self.__info(f"reading config file {cfgfile}")
> +            ini.read(cfgfile)
>
>          # wipe any previously read config info
>          if not append:
>
> base-commit: 5edb31a58bea3c2433e817439e053b79221a06df
> --
> 2.47.1
>
>

Sounds useful and simple.

Reviewed-by: Tomas Glozar <tglozar@redhat.com>

Tomas

Re: [PATCH] rteval: Allow configuration from stdin

[John Kacur]

On Mon, 13 Jan 2025, Gabriele Monaco wrote:

> Modules can currently be set only via configuration file, if rteval is
> called from a script run on different systems, the only way to make sure
> the loaded modules are known is to provide (or generate) a configuration
> file and pass it via -f/--infile .

The config files are almost obsolete.
Everything has a reasonable default, or can be set from the CMI
The one exception is for the measurement module.

What would be useful would be a CMI interface to switch from rtla timerlat
to cyclictest.

Thanks

> 
> Add the possibility to use - as input file to read the configuration via
> standard input.
> 
> This allows something like:
> 
>  # rteval -f - << EOF
> [measurement]
> timerlat: module
> 
> [loads]
> stressng:  module
> EOF
> 
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
> ---
>  rteval/rtevalConfig.py | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/rteval/rtevalConfig.py b/rteval/rtevalConfig.py
> index 0c2fc6b..4c41ed7 100644
> --- a/rteval/rtevalConfig.py
> +++ b/rteval/rtevalConfig.py
> @@ -238,10 +238,14 @@ class rtevalConfig:
>              # Don't try to reread this file if it's already been parsed
>              return
>  
> -        self.__info(f"reading config file {cfgfile}")
>          ini = configparser.ConfigParser()
>          ini.optionxform = str
> -        ini.read(cfgfile)
> +        if cfgfile == "-":
> +            self.__info("reading config file stdin")
> +            ini.read_file(sys.stdin)
> +        else:
> +            self.__info(f"reading config file {cfgfile}")
> +            ini.read(cfgfile)
>  
>          # wipe any previously read config info
>          if not append:
> 
> base-commit: 5edb31a58bea3c2433e817439e053b79221a06df
> -- 
> 2.47.1
> 
> 
> 

Re: [PATCH] rteval: Allow configuration from stdin

[Gabriele Monaco]

On Mon, 2025-01-13 at 16:34 -0500, John Kacur wrote:
> 
> 
> On Mon, 13 Jan 2025, Gabriele Monaco wrote:
> 
> > Modules can currently be set only via configuration file, if rteval
> > is
> > called from a script run on different systems, the only way to make
> > sure
> > the loaded modules are known is to provide (or generate) a
> > configuration
> > file and pass it via -f/--infile .
> 
> The config files are almost obsolete.
> Everything has a reasonable default, or can be set from the CMI
> The one exception is for the measurement module.
> 
> What would be useful would be a CMI interface to switch from rtla
> timerlat
> to cyclictest.
> 

Mmh, that was actually my first idea, something like --measurement-
modules=timerlat,cyclictest --loads-modules=hackbench
Basically behaving just like the measurement and loads sections in the
config.

I opted for this as it was simpler but if the config file is getting
obsolete, I think you can just ignore the patch.

I could start drafting a patch for the CLI options to specify modules
if they look alright to you.

Another not too different option could be kinda like stress-ng,
specifying the modules just like --cyclictest , --timerlat , --
hackbench , etc.

Both would require a bit of refactoring how we load modules.
I don't have any strong opinion on this, thoughts?

Thanks,
Gabriele

Re: [PATCH] rteval: Allow configuration from stdin

[Crystal Wood]

On Tue, 2025-01-14 at 07:40 +0100, Gabriele Monaco wrote:
> On Mon, 2025-01-13 at 16:34 -0500, John Kacur wrote:
> > 
> > 
> > On Mon, 13 Jan 2025, Gabriele Monaco wrote:
> > 
> > > Modules can currently be set only via configuration file, if rteval
> > > is
> > > called from a script run on different systems, the only way to make
> > > sure
> > > the loaded modules are known is to provide (or generate) a
> > > configuration
> > > file and pass it via -f/--infile .
> > 
> > The config files are almost obsolete.
> > Everything has a reasonable default, or can be set from the CMI
> > The one exception is for the measurement module.
> > 
> > What would be useful would be a CMI interface to switch from rtla
> > timerlat
> > to cyclictest.
> > 
> 
> Mmh, that was actually my first idea, something like --measurement-
> modules=timerlat,cyclictest --loads-modules=hackbench
> Basically behaving just like the measurement and loads sections in the
> config.
> 
> I opted for this as it was simpler but if the config file is getting
> obsolete, I think you can just ignore the patch.

It's a simple enough patch that it might be worth taking anyway...
unless we actually do want to deprecate the config file entirely, rather
than just making sure it's not the only place where certain things can
be configured.

> 
> I could start drafting a patch for the CLI options to specify modules
> if they look alright to you.
> 
> Another not too different option could be kinda like stress-ng,
> specifying the modules just like --cyclictest , --timerlat , --
> hackbench , etc.

It fits the pattern of module-specific parameters, so I kind of like it
from an interface perspective.  Implementation wise, though, something
like
--load-modules='cyclictest,sysstat' or --load cyclictest --load sysstat
might be easier (interface wise, I prefer the latter).

> 
> Both would require a bit of refactoring how we load modules.

That could be a good thing, if the new way is simpler.

-Crystal

Re: [PATCH] rteval: Allow configuration from stdin

[John Kacur]

[-- Attachment #1: Type: text/plain, Size: 3759 bytes --]



On Tue, 14 Jan 2025, Crystal Wood wrote:

> On Tue, 2025-01-14 at 07:40 +0100, Gabriele Monaco wrote:
> > On Mon, 2025-01-13 at 16:34 -0500, John Kacur wrote:
> > > 
> > > 
> > > On Mon, 13 Jan 2025, Gabriele Monaco wrote:
> > > 
> > > > Modules can currently be set only via configuration file, if rteval
> > > > is
> > > > called from a script run on different systems, the only way to make
> > > > sure
> > > > the loaded modules are known is to provide (or generate) a
> > > > configuration
> > > > file and pass it via -f/--infile .
> > > 
> > > The config files are almost obsolete.
> > > Everything has a reasonable default, or can be set from the CMI
> > > The one exception is for the measurement module.
> > > 
> > > What would be useful would be a CMI interface to switch from rtla
> > > timerlat
> > > to cyclictest.
> > > 
> > 
> > Mmh, that was actually my first idea, something like --measurement-
> > modules=timerlat,cyclictest --loads-modules=hackbench
> > Basically behaving just like the measurement and loads sections in the
> > config.
> > 
> > I opted for this as it was simpler but if the config file is getting
> > obsolete, I think you can just ignore the patch.
> 
> It's a simple enough patch that it might be worth taking anyway...
> unless we actually do want to deprecate the config file entirely, rather
> than just making sure it's not the only place where certain things can
> be configured.

I'm tending in the direction of deprecating the config file, I don't find 
it too useful. In any case, I don't want to add to it, we will then end up 
having to maintain that.

> 
> > 
> > I could start drafting a patch for the CLI options to specify modules
> > if they look alright to you.
> > 
> > Another not too different option could be kinda like stress-ng,
> > specifying the modules just like --cyclictest , --timerlat , --
> > hackbench , etc.
> 
> It fits the pattern of module-specific parameters, so I kind of like it
> from an interface perspective.  Implementation wise, though, something
> like
> --load-modules='cyclictest,sysstat' or --load cyclictest --load sysstat
> might be easier (interface wise, I prefer the latter).
> 

Note that cyclictest and timerlat in rteval jargon are measurement modules
The current way that load modules work is that most are implicitly 
implied. stress-ng is not among the default implicitly implied load 
modules, but if you choose stressng-option then it sets itself as
an exclusive load module and turns off the others. If a person tries to 
run with a stressng-option and an option for an other load module, it is 
an error condition.

It is possible to run both the cyclictest and timerlat measurement modules 
without using any specific cyclictest or timerlat options. (so that 
doesn't work as a way to differentiate)

There are some measurement module options that will apply to either one.
For example 
--measurement-cpulist CPULIST

will run measurement modules on the CPULIST whether that be cyclictest or 
timerlat.

I think the simplest thing to do would be to implement these two options 
without parameters.
--cyclictest
--timerlat

Now, you wouldn't HAVE to use them, currently timerlat is the default, so 
if you don't specify, then it would use the default.

Like with stress-ng and the load modules, if a person tried to use a 
cyclictest and a timerlat option at the sametime, it should create an 
error and exit.

I like the above idea, because other than the optional option of 
--cyclictest and --timerlat, nothing else changes, so it wouldn't break 
anybody's scripts in any kind of major way.

John

> > 
> > Both would require a bit of refactoring how we load modules.
> 
> That could be a good thing, if the new way is simpler.
> 
> -Crystal
> 
> 
> 

Re: [PATCH] rteval: Allow configuration from stdin

[Crystal Wood]

On Tue, 2025-01-14 at 18:07 -0500, John Kacur wrote:
> 
> On Tue, 14 Jan 2025, Crystal Wood wrote:
> 
> > On Tue, 2025-01-14 at 07:40 +0100, Gabriele Monaco wrote:
> > > I could start drafting a patch for the CLI options to specify modules
> > > if they look alright to you.
> > > 
> > > Another not too different option could be kinda like stress-ng,
> > > specifying the modules just like --cyclictest , --timerlat , --
> > > hackbench , etc.
> > 
> > It fits the pattern of module-specific parameters, so I kind of like it
> > from an interface perspective.  Implementation wise, though, something
> > like
> > --load-modules='cyclictest,sysstat' or --load cyclictest --load sysstat
> > might be easier (interface wise, I prefer the latter).
> > 
> 
> Note that cyclictest and timerlat in rteval jargon are measurement modules

Sorry, brain fart... s/load/measurement/ (I'd prefer just "measure" but
I guess we want to be consistent with stuff like --measurement-cpulist)

> The current way that load modules work is that most are implicitly 
> implied. 
> 

They're explicitly enabled in the default config file...

> stress-ng is not among the default implicitly implied load 
> modules, but if you choose stressng-option then it sets itself as
> an exclusive load module and turns off the others. If a person tries to 
> run with a stressng-option and an option for an other load module, it is 
> an error condition.
> 
> It is possible to run both the cyclictest and timerlat measurement modules 
> without using any specific cyclictest or timerlat options. (so that 
> doesn't work as a way to differentiate)

I don't think implicit enablement based on module-specific options is a
good model to emulate, regardless.

> 
> There are some measurement module options that will apply to either one.
> For example 
> --measurement-cpulist CPULIST
> 
> will run measurement modules on the CPULIST whether that be cyclictest or 
> timerlat.
> 
> I think the simplest thing to do would be to implement these two options 
> without parameters.
> --cyclictest
> --timerlat

If you mean special casing these two modules rather than having a
general mechanism for enabling modules, I'm not a huge fan of that...
though it might be preferable to overengineering something.

What about sysstat?

And how would we specify a module to disable?  If timerlat is the
default (currently this is only the case if the measurement section of
the config file is missing), and the user enables cyclictest, do we
hardcode that that disables timerlat?  Or have a way to prefer command-
line-specified modules over conflicting default modules?

> 
> Now, you wouldn't HAVE to use them, currently timerlat is the default, so 
> if you don't specify, then it would use the default.
> 
> Like with stress-ng and the load modules, if a person tried to use a 
> cyclictest and a timerlat option at the sametime, it should create an 
> error and exit.
> 
> I like the above idea, because other than the optional option of 
> --cyclictest and --timerlat, nothing else changes, so it wouldn't break 
> anybody's scripts in any kind of major way.

None of these proposals should break existing scripts, assuming we don't
change the defaults.  Removing config file support, OTOH, would force
people still using cyclictest to start specifying it on the command line
one way or another.


-Crystal

Re: [PATCH] rteval: Allow configuration from stdin

[Gabriele Monaco]

On Wed, 2025-01-15 at 20:14 -0500, Crystal Wood wrote:
> On Tue, 2025-01-14 at 18:07 -0500, John Kacur wrote:
> > 
> > On Tue, 14 Jan 2025, Crystal Wood wrote:
> > 
> > > On Tue, 2025-01-14 at 07:40 +0100, Gabriele Monaco wrote:
> > > > I could start drafting a patch for the CLI options to specify
> > > > modules
> > > > if they look alright to you.
> > > > 
> > > > Another not too different option could be kinda like stress-ng,
> > > > specifying the modules just like --cyclictest , --timerlat , --
> > > > hackbench , etc.
> > > 
> > > It fits the pattern of module-specific parameters, so I kind of
> > > like it
> > > from an interface perspective.  Implementation wise, though,
> > > something
> > > like
> > > --load-modules='cyclictest,sysstat' or --load cyclictest --load
> > > sysstat
> > > might be easier (interface wise, I prefer the latter).
> > > 
> > 
> > Note that cyclictest and timerlat in rteval jargon are measurement
> > modules
> 
> Sorry, brain fart... s/load/measurement/ (I'd prefer just "measure"
> but
> I guess we want to be consistent with stuff like --measurement-
> cpulist)
> 
> > The current way that load modules work is that most are implicitly 
> > implied. 
> > 
> 
> They're explicitly enabled in the default config file...
> 
> > stress-ng is not among the default implicitly implied load 
> > modules, but if you choose stressng-option then it sets itself as
> > an exclusive load module and turns off the others. If a person
> > tries to 
> > run with a stressng-option and an option for an other load module,
> > it is 
> > an error condition.
> > 
> > It is possible to run both the cyclictest and timerlat measurement
> > modules 
> > without using any specific cyclictest or timerlat options. (so that
> > doesn't work as a way to differentiate)
> 
> I don't think implicit enablement based on module-specific options is
> a
> good model to emulate, regardless.
> 
> > 
> > There are some measurement module options that will apply to either
> > one.
> > For example 
> > --measurement-cpulist CPULIST
> > 
> > will run measurement modules on the CPULIST whether that be
> > cyclictest or 
> > timerlat.
> > 
> > I think the simplest thing to do would be to implement these two
> > options 
> > without parameters.
> > --cyclictest
> > --timerlat
> 
> If you mean special casing these two modules rather than having a
> general mechanism for enabling modules, I'm not a huge fan of that...
> though it might be preferable to overengineering something.
> 
> What about sysstat?
> 
> And how would we specify a module to disable?  If timerlat is the
> default (currently this is only the case if the measurement section
> of
> the config file is missing), and the user enables cyclictest, do we
> hardcode that that disables timerlat?  Or have a way to prefer
> command-
> line-specified modules over conflicting default modules?
> 
> > 
> > Now, you wouldn't HAVE to use them, currently timerlat is the
> > default, so 
> > if you don't specify, then it would use the default.
> > 
> > Like with stress-ng and the load modules, if a person tried to use
> > a 
> > cyclictest and a timerlat option at the sametime, it should create
> > an 
> > error and exit.
> > 
> > I like the above idea, because other than the optional option of 
> > --cyclictest and --timerlat, nothing else changes, so it wouldn't
> > break 
> > anybody's scripts in any kind of major way.
> 
> None of these proposals should break existing scripts, assuming we
> don't
> change the defaults.  Removing config file support, OTOH, would force
> people still using cyclictest to start specifying it on the command
> line
> one way or another.

Strictly speaking, enabling timerlat as the default measurement module
did break existing scripts, that's what brought me to start this patch
in the first place.

I agree that we should try to keep the same behaviour with respect to
default values, no measurement module specified via command line should
be the same as no measurement section in the config files, as
specifying only one should disable the other. Specifying both should be
allowed.

I see the command line as another setup that should override the config
file, while behaving exactly the same (where the notation --
measurement-modules would indeed be a bit more familiar, but I still
like --cyclictest/--timerlat).

The order could be: command-line > explicit config file (-f) > default
config file (/etc) > default values, this wouldn't affect the behaviour
much.

I only see the behaviour with stress-ng a bit strange, is it really not
a thing to run it in conjunction with hackbench and kcompile?

I see the simplest thing for now is to start with command-line options
for measurement only. After we consolidate that, we can do the same for
loads (with or without special cases).

Thoughts?

Gabriele

Re: [PATCH] rteval: Allow configuration from stdin

[Crystal Wood]

On Thu, 2025-01-16 at 07:45 +0100, Gabriele Monaco wrote:
> 
> On Wed, 2025-01-15 at 20:14 -0500, Crystal Wood wrote:
> > On Tue, 2025-01-14 at 18:07 -0500, John Kacur wrote:
> > > Now, you wouldn't HAVE to use them, currently timerlat is the
> > > default, so if you don't specify, then it would use the default.
> > > 
> > > Like with stress-ng and the load modules, if a person tried to
> > > use a cyclictest and a timerlat option at the sametime, it should
> > > create an error and exit.
> > > 
> > > I like the above idea, because other than the optional option of
> > > --cyclictest and --timerlat, nothing else changes, so it wouldn't
> > > break anybody's scripts in any kind of major way.
> > 
> > None of these proposals should break existing scripts, assuming we
> > don't
> > change the defaults.  Removing config file support, OTOH, would force
> > people still using cyclictest to start specifying it on the command
> > line
> > one way or another.
> 
> Strictly speaking, enabling timerlat as the default measurement module
> did break existing scripts, that's what brought me to start this patch
> in the first place.

By "these proposals" I meant the command line stuff in this thread.

> 
> I agree that we should try to keep the same behaviour with respect to
> default values, no measurement module specified via command line should
> be the same as no measurement section in the config files, as
> specifying only one should disable the other. Specifying both should be
> allowed.

Why should specifying both be allowed?  We go out of our way to prevent
that, because they'll interfere with one another's latencies.

> 
> I see the command line as another setup that should override the config
> file, while behaving exactly the same (where the notation --
> measurement-modules would indeed be a bit more familiar, but I still
> like --cyclictest/--timerlat).
> 
> The order could be: command-line > explicit config file (-f) > default
> config file (/etc) > default values, this wouldn't affect the behaviour
> much.

It's not the same, since the command line would only override individual
options, while a config file replaces all defaults (at least when it
comes to enabling modules, not sure about other defaults).

> I see the simplest thing for now is to start with command-line options
> for measurement only. After we consolidate that, we can do the same for
> loads (with or without special cases).

What makes loads harder?


-Crystal