[CRIU] [PATCH v3 5/7] config: check for CRIU_CONFIG_FILE environment variable

[Adrian Reber]

On Sat, Jul 07, 2018 at 07:22:33PM -0700, Andrei Vagin wrote:
> Hi Adrian,
> 
> Last time, we discussed that we need a way how to override criu options
> which are set by Docker, runc, LXC or other tools.
> 
> https://marc.info/?l=openvz-criu&m=152844187925264&w=2

Yes and no. Now it gets complicated because Pavel and you are expecting
different things ;-). This initial idea was the configuration file
options are defaults and can be overridden by CLI/RPC settings.

> > My idea was to introduce two config files for criu. The first one is exsting one, that
> > criu reads from known global place (/etc/...). Then criu parses CLI/RPC options and 
> > overrides values read from config. Then should go the 2nd config, which is formatted
> > the same way as the 1st one, but it sits not in some other place path to which is
> > somehow (I don't know how to pass this knowledge "through" docker/lxd) told to criu
> > by the caller. This 2nd config file will override options set via CLI/RPC. Also, since
> > the path to this 2nd config file is specified by the caller, if some other criu invocation
> > happens in parallel it will not erroneously pick this file's options.
> > 
> 
> But CRIU_CONFIG_FILE doesn't work this way, does it?

It does as described above. CLI/RPC overrides configuration file.

> # cat /tmp/criu.cfg 
> log-file /tmp/criu.log
> # export CRIU_CONFIG_FILE="/tmp/criu.cfg"
> # ./criu/criu dump -t 8888888 -v4
> # cat /tmp/criu.log | grep Error
> (00.001984) Error (criu/util.c:407): Can't open 8888888: No such file or directory
> (00.002021) Error (compel/src/lib/infect.c:341): Unable to detach from 8888888: No such process
> (00.002056) Error (criu/cr-dump.c:1834): Dumping FAILED.
> 
> # unlink /tmp/criu.log 
> # ./criu/criu dump -t 8888888 -v4 --log-file dump.log
> # cat /tmp/criu.log | grep Error
> cat: /tmp/criu.log: No such file or directory

Exactly what is happening here. We have a 'default' setting in
'/tmp/criu.log', but once we say '--log-file' via CLI the configuration
file value is no longer valid.

The initial idea of the configuration file was to be able to set default
values which can be changed by CLI/RPC. So instead of saying -v4
every time on the CLI I set it once in the configuration file and it
always works. For the one case where I want -v0 I can still set it via
CLI.

So we really need to figure out what we want. Do we want to set
defaults and be able to override them via CLI/RPC? Or do we want to set
CRIU options which can be never overridden by CLI/RPC?

My current patch set supports both setups for RPC, if the RPC client
(runc) sets opts.prefer_config_file=True, the configuration files have
override any settings done via RPC. If opts.prefer_config_file is set to
False, CRIU behaves just like in CLI mode and any configuration file
value which is explicitly set via RPC is ignored. Both modes of
opts.prefer_config_file have their advantages and drawbacks. If it is
set to True, the user can break the RPC client by setting incompatible
values in the configuration file. If opts.prefer_config_file is set to
false some things cannot be changed because the RPC client already sets
them.

		Adrian

> On Thu, Jun 28, 2018 at 04:02:32PM +0000, Adrian Reber wrote:
> > From: Adrian Reber <areber at redhat.com>
> > 
> > To be able to influence, especially via RPC, that another configuration
> > file should be used, this introduces the environment variable
> > CRIU_CONFIG_FILE.
> > 
> > If set, the file it points to will be used instead of the default
> > configuration files and also instead of the configuration file specified
> > via the command-line.
> > 
> > CRIU now first checks for CRIU_CONFIG_FILE and uses that, if that is not
> > set it looks for '--config FILEPATH' and uses that, if that is not set
> > it uses the default configuration files.
> > 
> > The idea behind this option is that now it is possible for an RPC client
> > to set CRIU_CONFIG_FILE and also set the RPC option
> > opts.prefer_config_file to tell CRIU to prefer the options from a
> > non-default configuration file over the options set via RPC.
> > 
> > Signed-off-by: Adrian Reber <areber at redhat.com>
> > ---
> >  criu/config.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/criu/config.c b/criu/config.c
> > index f3bf35b..d764832 100644
> > --- a/criu/config.c
> > +++ b/criu/config.c
> > @@ -157,6 +157,7 @@ int init_config(int argc, char **argv, int *global_cfg_argc, int *user_cfg_argc,
> >  {
> >  	bool no_default_config = false;
> >  	char *cfg_file = NULL;
> > +	char *cfg_from_env = getenv("CRIU_CONFIG_FILE");
> >  	int i;
> >  
> >  	/*
> > @@ -188,6 +189,14 @@ int init_config(int argc, char **argv, int *global_cfg_argc, int *user_cfg_argc,
> >  		}
> >  	}
> >  
> > +	/*
> > +	 * If the environment variable CRIU_CONFIG_FILE is set it
> > +	 * will overwrite the configuration file set via '--config'.
> > +	 */
> > +
> > +	if (cfg_from_env)
> > +		cfg_file = cfg_from_env;
> > +
> >  	init_configuration(argc, argv, no_default_config, cfg_file);
> >  	if (global_conf != NULL)
> >  		*global_cfg_argc = count_elements(global_conf);
> > -- 
> > 1.8.3.1
> > 

		Adrian

-- 
Adrian Reber <adrian at lisas.de>            http://lisas.de/~adrian/
ATM cell has no roaming feature turned on, notebooks can't connect