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

[Pavel Emelyanov]

On 07/09/2018 02:16 PM, Adrian Reber wrote:
> On Mon, Jul 09, 2018 at 12:52:55PM +0300, Pavel Emelyanov wrote:
>> 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
> 
> Clear semantics. Definitely.
> 
>> 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 :)
> 
> Configuration files with different meaning... This feels... complicated.

:)

> Have you seen Radostin's proposal of different option names in the
> configuration file. Options that override CLI/RPC have a different
> name. That also does not (yet) feel totally correct, but might be a
> better approach than configuration files with different meanings.

I have, but haven't yet written a response (spoiler: it looks too complicated)

> Right now the configuration files are all treated the same.

Yup, and my proposal is to take their order of reading into account.

>>> 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.
> 
> "Prefer config file" currently overrides RPC values for any
> configuration file.

+1

> The custom configuration file is treated as the other> configuration files. Only that the default configuration files are
> ignored if a custom file is specified.

I propose not to, but override options from default config file by custom one :)

> Do you see it as an important difference if the configuration file is in
> a custom location? Like if the user already specified a custom
> configuration file location then it is something special and must
> override the RPC client?

Location of the config file can be any. I see it useful to tell criu where to read
(one of) config from because reading a config file from "known place" will break
concurrent runs of criu-s with different "options".

> We can also easily provide a setting 'prefer config file' via CLI, to
> match the RPC behavior.
> 
>> Does Andrey want smth different?
> 
> Not sure ;) I will let him answer that.
> 
> 
> It feels like we are getting close to a solution and we only need to
> come to a consensus about the right behavior. If we continue the
> discussion we should soon have something acceptable to everyone.
> 
>  Do we want configuration files with different meaning?

I vote for "all config files being the same" but the order of reading matters.

>  Do we want configuration file options with different meaning? Something
>  which changes options to override CLI/RPC options.

I vote "no we don't", it's already too many options in criu, adding flavors to
them sounds as complication.

-- Pavel