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

[Adrian Reber]

On Fri, Jul 06, 2018 at 02:23:24PM -0700, Andrei Vagin wrote:
> 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}

That should be no problem to implement. I can implement the runc part of
it once this has been merged and after my external network namespaces have
been merged into runc.

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