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

[Pavel Emelyanov]

On 07/08/2018 07:28 PM, Radostin Stoyanov wrote:
> On 08/07/18 08:05, 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 ;-). 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.
> An alternative approach, to support both functionalities, could be to
> add a prefix
> (e.g. default-log-file) for options in the config file that will be
> overridden by CLI/RPC.
> Options which do not have this prefix (e.g. log-file) will override CLI/RPC.

Frankly speaking this complicates things a lot from my point of view :) but
as all -- your, Adrian's and mine -- solutions work, this is purely a question 
of taste. I only have two stylish notices regarding this.

> In other words, in the config file we can use a "log-file" option to
> specify a value that
> will overwrite CLI/RPC, and "default-log-file" for a value that will be
> overridden by CLI/RPC.
> 
> What do you think?

What if we have an option called --default-something configuring some default
for some action. For the sake of this RPC overriding we'd have to introduce the
--default-default-something option :D

Another thing is -- we have short-only options, how to attach a long --default 
to them?

-- Pavel