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

[Adrian Reber]

On Fri, Jul 06, 2018 at 03:08:58PM +0300, Pavel Emelyanov wrote:
> On 07/06/2018 01:37 PM, Adrian Reber wrote:
> > On Fri, Jul 06, 2018 at 12:33:27PM +0300, Pavel Emelyanov wrote:
> >> On 06/28/2018 07:02 PM, 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.
> >>
> >> But how is RPC client supposed to affect CRIU's environment if the latter is
> >> fork-ed and exec-ed by some (Docker/LXD) daemon?
> > 
> > This seems to dependent on the definition of what a RPC client is.
> 
> :D
> 
> >>From my point of view runc is basically the only RPC client CRIU
> > currently have besides p.haul.
> > 
> > My idea was, if the RPC client (runc) wants to allow the user to
> > override RPC settings via configuration file, the RPC client sets
> > opts.prefer_config_file to True. If the RPC client wants to offer the
> > possibility to specify a non-default configuration file it can offer
> > that and tell CRIU by setting the CRIU_CONFIG_FILE environment variable.
> > 
> > I did not expect CRIU_CONFIG_FILE to be set by the user which then calls
> > docker which then calls runc which then calls CRIU:
> > 
> > user->docker->runc->criu (are there even more layers between the user and criu???)
> > 
> > But if runc sets CRIU_CONFIG_FILE it should work, right?
> 
> Right, but my question was exactly about the case you mentioned --  user->docker->runc->criu.
> IIRC, this was the unavoidable obstacle lots of people suffer from.

CRIU still parses /etc/criu/default.conf and $HOME/.criu/default.conf.
So this should help most of the users. Then again in the RPC case it
will be difficult as RPC overrides configuration file settings, for
explicitly set RPC values.

I see the environment variable more for direct RPC users like runc who
want to disable default configuration file and provide one special file.
This was less the user interface (user->docker->runc->criu) and more the
runc interface (runc->criu).

If the stack user->docker->runc->criu wants to pass a special
configuration file from the user to criu it is probably only possible if
the whole stack is aware of it.

> > So from my point of view CRIU_CONFIG_FILE is more for runc to define a
> > non standard configuration than for the actual user to set one.
> > 
> > 		Adrian
> > 
> >>> 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);
> >>>
> > 
> > 		Adrian
> >