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

[Adrian Reber]

On Mon, Jul 09, 2018 at 02:42:16PM +0300, Pavel Emelyanov wrote:
> 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".

What am I missing here is how to tell CRIU which of possible
configuration files to use.

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

That already exists:

First /etc/criu/default.conf then $HOME/.criu/default.conf then CLI/RPC:

 /etc/criu/default.conf -> $HOME/.criu/default.conf -> CLI/RPC

Custom configuration file replaces /etc/criu/default.conf and $HOME/.criu/default.conf so:

 CRIU_CONFIG_FILE -> CLI/RPC

So you are proposing to change it to

 /etc/criu/default.conf -> $HOME/.criu/default.conf -> CRIU_CONFIG_FILE -> CLI/RPC

and if 'prefer config file' is set via CLI/RPC it would change to:

 CLI/RPC -> /etc/criu/default.conf -> $HOME/.criu/default.conf -> CRIU_CONFIG_FILE

Right?

And only if '--no-default-config' is set then we would get:

 CRIU_CONFIG_FILE -> CLI/RPC

or if 'prefer config file' is set via CLI/RPC it would change to:

 CLI/RPC -> CRIU_CONFIG_FILE

Can we also set '--no-default-config' and 'prefer config file' via the
configuration file. (I would say no)

So if 'no-default-config' is part of CRIU_CONFIG_FILE the other two
configurations files are ignored after they have already been parsed.

Can 'prefer config file' be also set in the configuration files? In all
of them? Or only in CRIU_CONFIG_FILE?

The behavior would be the same for RPC and CLI CRIU usage.

The most important changes from my current patches to this would be that
CRIU_CONFIG_FILE does not disable /etc/criu/default.conf and
$HOME/.criu/default.conf  parsing and that CLI needs
'--prefer-config-file'.

		Adrian

		Adrian