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

[Andrei Vagin]

On Fri, Jul 06, 2018 at 03:16:48PM +0200, Adrian Reber wrote:
> 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.

It should be possible to specify config for each operation:

docker checkpoint ${CT} --criu-config criu-config-${CT}

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