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

Submitted by Adrian Reber on June 28, 2018, 4:02 p.m.

Details

Message ID 1530201754-4493-6-git-send-email-adrian@lisas.de
State New
Series "Series without cover letter"
Headers show

Commit Message

Adrian Reber June 28, 2018, 4:02 p.m.
From: Adrian Reber <areber@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.

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@redhat.com>
---
 criu/config.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Patch hide | download patch | download mbox

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

Comments

Pavel Emelyanov July 6, 2018, 9:33 a.m.
On 06/28/2018 07:02 PM, Adrian Reber wrote:
> From: Adrian Reber <areber@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?

-- Pavel

> 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@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 Reber July 6, 2018, 10:37 a.m.
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@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.

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?

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@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
Pavel Emelyanov July 6, 2018, 12:08 p.m.
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@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.

> 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@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
>
Adrian Reber July 6, 2018, 1:16 p.m.
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@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@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
> >
Andrei Vagin July 6, 2018, 9:23 p.m.
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@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@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
> > >
Adrian Reber July 7, 2018, 7:23 a.m.
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@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@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
> > > >
Andrei Vagin July 8, 2018, 2:22 a.m.
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

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

# 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

Thanks,
Andrei

On Thu, Jun 28, 2018 at 04:02:32PM +0000, Adrian Reber wrote:
> From: Adrian Reber <areber@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.
> 
> 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@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);
> -- 
> 1.8.3.1
>
Adrian Reber July 8, 2018, 7:05 a.m.
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.

		Adrian

> On Thu, Jun 28, 2018 at 04:02:32PM +0000, Adrian Reber wrote:
> > From: Adrian Reber <areber@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.
> > 
> > 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@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);
> > -- 
> > 1.8.3.1
> > 

		Adrian
Radostin Stoyanov July 8, 2018, 4:28 p.m.
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?

Radostin
Andrei Vagin July 9, 2018, 6:24 a.m.
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.


> 
> Radostin
>
Adrian Reber July 9, 2018, 6:29 a.m.
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
Pavel Emelyanov July 9, 2018, 9:52 a.m.
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
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 :)

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

Does Andrey want smth different?

-- Pavel

> 		Adrian
> 
>> On Thu, Jun 28, 2018 at 04:02:32PM +0000, Adrian Reber wrote:
>>> From: Adrian Reber <areber@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.
>>>
>>> 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@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);
>>> -- 
>>> 1.8.3.1
>>>
> 
> 		Adrian
>
Adrian Reber July 9, 2018, 11:16 a.m.
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.

Right now the configuration files are all treated the same.

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

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?

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?

 Do we want configuration file options with different meaning? Something
 which changes options to override CLI/RPC options.


		Adrian
Pavel Emelyanov July 9, 2018, 11:42 a.m.
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".

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

>  Do we want configuration file options with different meaning? Something
>  which changes options to override CLI/RPC options.

I vote "no we don't", it's already too many options in criu, adding flavors to
them sounds as complication.

-- Pavel
Pavel Emelyanov July 9, 2018, 11:47 a.m.
On 07/08/2018 07:28 PM, 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.

Frankly speaking this complicates things a lot from my point of view :) but
as all -- your, Adrian's and mine -- solutions work, this is purely a question 
of taste. I only have two stylish notices regarding this.

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

What if we have an option called --default-something configuring some default
for some action. For the sake of this RPC overriding we'd have to introduce the
--default-default-something option :D

Another thing is -- we have short-only options, how to attach a long --default 
to them?

-- Pavel
Adrian Reber July 9, 2018, 4:29 p.m.
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
Pavel Emelyanov July 9, 2018, 5:16 p.m.
>>> 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.

Yes...

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

Almost. If 'prefer config file' is set then

global config -> home config -> CLI/RPC -> given config

(and if it's not, then  global -> home -> given -> CLI/RPC)

> And only if '--no-default-config' is set then we would get:
> 
>  CRIU_CONFIG_FILE -> CLI/RPC

Then

home config -> given config -> CLI/RPC?

IOW -- why skipping the home config?

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

I would state that 'prefer config file' is ignored when read from any config.
To keep things as simple as it's possible with this weird scheme %)

> 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
> .
>
Andrei Vagin July 9, 2018, 7:36 p.m.
On Mon, Jul 09, 2018 at 08:29:51AM +0200, Adrian Reber wrote:
> 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?

I'm going to review this series and merge it into the criu-dev branch.

Could you send this patches as a new series? The current version was not
tested by Jenkins and it is applied to the criu-dev with conflicts.

But I would prefer to have an ability to override values before merging
config files into the master branch. For me this functionality looks
much more usefull in a real life than changing default values...

> 
> 		Adrian
Adrian Reber July 10, 2018, 9:31 a.m.
On Mon, Jul 09, 2018 at 12:36:30PM -0700, Andrei Vagin wrote:
> On Mon, Jul 09, 2018 at 08:29:51AM +0200, Adrian Reber wrote:
> > 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?
> 
> I'm going to review this series and merge it into the criu-dev branch.
> 
> Could you send this patches as a new series? The current version was not
> tested by Jenkins and it is applied to the criu-dev with conflicts.

I rebased it and travis seems to be happy with it. Resending soon.

> But I would prefer to have an ability to override values before merging
> config files into the master branch. For me this functionality looks
> much more usefull in a real life than changing default values...

For the CLI use case it works most of the time because you will probably
not specify options you have in config file. But yes, I will provide
appropriate patches on top of it.

		Adrian
Adrian Reber July 10, 2018, 9:38 a.m.
On Mon, Jul 09, 2018 at 08:16:12PM +0300, Pavel Emelyanov wrote:
> >>> 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?
> 
> Almost. If 'prefer config file' is set then
> 
> global config -> home config -> CLI/RPC -> given config
> 
> (and if it's not, then  global -> home -> given -> CLI/RPC)
> 
> > And only if '--no-default-config' is set then we would get:
> > 
> >  CRIU_CONFIG_FILE -> CLI/RPC
> 
> Then
> 
> home config -> given config -> CLI/RPC?
> 
> IOW -- why skipping the home config?
> 
> > 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?
> 
> I would state that 'prefer config file' is ignored when read from any config.
> To keep things as simple as it's possible with this weird scheme %)

Okay, thanks. I will do these changes on top of the current patchset.

		Adrian