[CRIU] [RESEND PATCH v3 3/7] RPC: Use configuration file also from RPC

Adrian Reber adrian at lisas.de
Thu Jul 19 21:01:23 MSK 2018


On Wed, Jul 11, 2018 at 11:45:33AM -0700, Andrei Vagin wrote:
> On Tue, Jul 10, 2018 at 09:39:26AM +0000, Adrian Reber wrote:
> > From: Adrian Reber <areber at redhat.com>
> > 
> > This enables the usage of configuration file values in the RPC code
> > path. The main change is to verify if the user has actually changed a
> > configuration option in the configuration file and if it has been
> > changed the value configured via RPC is ignored.
> > 
> > As it is not possible to detect if a configuration option has the
> > default value because it has not been changed or set by the user or if
> > the user actually set it to the default value a copy of the options
> > structure (opt_changed_by_config) is used to track if a configuration
> > option has been changed by the user. This structure does not contain
> > that actual value of the configuration option it only tracks if an
> > option has been set. The value is still in the original options
> > structure (opts).
> > 
> > With this it is now possible to override CRIU's behavior if started via
> > RPC. If the user sets a value in one of the configuration files the
> > value coming from RPC is ignored. To avoid undesired and undefined
> > behavior the RPC client has to explicitly enable that configuration
> > files are overwriting RPC options by setting 'prefer_config_file' to
> > true.
> 
> I think all this logic with determining whether a value is a default one
> or not are overcomplicated.
> 
> prefer_config_file should not be global. We have configs in /etc and a
> user home directory which change default values.
> 
> Then we should have an option or options to specify configs which has to
> be applied before rpc or command line options and after them.
> 
> struct options opts = {};
> 
> apply_config(global_conf)
> apply_config(user_conf)
> apply_config(pre_conf)

What is pre_conf?

> parse_command_line()
> apply_rpc_options()
> apply_config(post_conf)

What is post_conf?

> Here on each step, we override options of a previous step.

Thinking about it, I think the reason for the current patch status that
I did not want to parse configuration files from two places. My patch
only had the getopt call just after starting the CRIU binary. No matter
if it is CLI or RPC. I guess that is the reason why my patches feel a
bit over complicated.

But maybe this should be done correctly, which means to remove getopt
config parsing from main() and place it in a function which can be
called after RPC options have been evaluated.

Maybe it is even a good idea as this means we can reduce the size of the
main() function a bit by moving code paths, especially config parsing
out of main().

Thanks for the offer on a conference call. But I think the biggest
problem was that I was not ready to move a lot of code around. But maybe
this just needs to be done, to have a cleaner solution.

		Adrian


More information about the CRIU mailing list