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

[Pavel Emelyanov]

On 07/08/2018 10:05 AM, Adrian Reber wrote:
> 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 ;-). 

:D

> 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?

I would say, that we want a clear "what overrides what" semantics :) My
initial proposal was that the override sequence was -- first criu reads
a config file, then overrides its values with CLI/RPC options, then,
optionally reads another config files that polishes things up :)

> 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.

Yes, the way I understood your proposal we could pass custom config file
and "prefer config file" option int criu that would make the latter read
the config file from opt1 and override what was set by RPC.

Does Andrey want smth different?

-- Pavel

> 		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
>