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

[Adrian Reber]

On Sun, Jul 08, 2018 at 11:24:40PM -0700, Andrei Vagin wrote:
> On Sun, Jul 08, 2018 at 05:28:04PM +0100, 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.
> > 
> > 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?
> 
> I think it is a good idea. My example is a real use-case where I'm going
> to use this functionality. I don't know where Docker or LXC save criu log
> files, for me it will be easier to specify a log file in a config file.

What does this mean for the current patchset? Can it be merged and I add
the override values later? Or do you want it all together?

		Adrian