[CRIU] [PATCH v2 2/4] Track which configuration options have been changed

[Adrian Reber]

On Tue, May 15, 2018 at 02:21:49PM +0300, Pavel Emelyanov wrote:
> On 05/10/2018 01:25 PM, Adrian Reber wrote:
> > On Wed, May 09, 2018 at 11:53:51AM -0700, Andrei Vagin wrote:
> >> On Wed, May 09, 2018 at 05:16:48PM +0000, Adrian Reber wrote:
> >>> From: Adrian Reber <areber at redhat.com>
> >>>
> >>> For the upcoming RPC configuration file support, the RPC code path needs
> >>> to know which values have been changed from the default.
> >>>
> >>> The reason for this is, that we do not want that values specified via
> >>> RPC are overwriting user specified settings in the configuration file.
> >>>
> >>> For bool options is is not possible to tell if the value is the default
> >>> value as a result of the configuration file or as a result of just being
> >>> the default.
> >>>
> >>> For default values, which are not modified by configuration files we
> >>> want to use the RPC specified value. If a user changes a value to the
> >>> default in the configuration file we do not want to have it changed by
> >>> the RPC code path.
> >>>
> >>> This means that values from the configuration file have a higher
> >>> priority than values configured via RPC.
> >>
> >> Should it be the same for command line criu options?
> >>
> >> I have some doubts about such behaviour... Maybe we need to add a marker
> >> for a config options, which will say how it should be applied. Pavel,
> >> what do you think?
> 
> Please, find my answers inline.
> 
> > This is the real complicated thing about the configuration file. I
> > discussed this a lot with Veronika when she implemented it and it was
> > never really clear what is the right thing to do. The current
> > implementation is basically what I would expect how a tool handles it
> > (this must not be correct).
> > 
> > For CLI the configuration files are parsed first, but the user can
> > overwrite it via command-line. For certain cases this makes sense as the
> > user might want to change the default behavior of the configuration
> > files.
> > 
> > Example: The package manager or the system administrator sets
> > verbosity=4. I as user do not like it and can disable it via CLI by
> > setting --verbosity=0 (or a configuration file in my home directory).
> > First configuration files are parsed and then the CLI options are
> > evaluated and it the end I get --verbosity=0 -> good.
> > 
> > 
> > On the other hand for cases like LXC, where LXC builds a
> > command-line and uses CRIU via CLI it does not make sense as it makes it
> > impossible to change CRIU's behavior.
> > 
> > Example: LXC sets '-vvvvv' and as user I want to change it to
> > 'verbosity=0'. As CRIU's command-line is part of LXC C code I cannot
> > change it quickly and therefore I try to set it via configuration file
> > (verbosity=0). This however does not work as the configuration file is
> > parsed first and the CLI overrides it. As a user I do not get what I
> > want -> bad.
> > 
> > 
> > So for CLI the current implementation in criu-dev can be correct (real
> > CLI) or wrong (CLI embedded in C code like LXC). One could argue that
> > LXC usage of CRIU is not how it is supposed to be and that RPC would be
> > better, but that is the current situation.
> > 
> > 
> > So CLI: first configuration file then CLI
> 
> Agreed, this is the sane policy for most of the tools out there -- the priority is to
> read config file, fix it with environment vairables (if any), then apply CLI options.
> 
> > For RPC, I would expect it to be the other way. As user I cannot change
> > how RPC is used without changing the code. Therefore I would expect that
> > the configuration file overrides the setting via RPC for me to be able
> > to change the behavior.
> > 
> > Example: runc also sets verbosity to 4. As a user I do not want big log
> > files and I change it to 'verbosity=0' via configuration file. Result:
> > CRIU uses verbosity=0 as the configuration file overrides the RPC
> > parameters. -> good, as a user I get what I expect.
> > 
> > It is, however, just the other way as CLI. Which is probably bad as it
> > might confuse the user.
> 
> Yup, this is the first thought that I got when saw Andrew's answer above -- RPC is just
> another CLI, so it should go last in the priority chain above. However ... (more comments 
> below).
> 
> > So it really is difficult to do it correctly.
> > 
> >>> The reason for this is, that we want to enable the user to change CRIU's
> >>> behavior when embedded in a container runtime and used via RPC (e.g.
> >>> runc).
> >>>
> >>> If an option is specified in the configuration file the values for that
> >>> option are set to 1/true in the corresponding opt_changed_by_config.
> >>>
> >>> Now the RPC code path can check if an option has been changed via
> >>> configuration file and can then ignore the value specified via RPC.
> >>
> >> Will it be simpler just to run parsing of config options after handling rpc
> >> options?
> 
> It will, sure. There are tree (but we can do 4 some day) places where CRIU takes options
> from -- config, CLI and RPC. And the sequence of these calls defines the priorities.
> 
> > Maybe, not sure. As we would need to run getopt() a second time. I
> > thought about it, but I was not sure. The problem is that we would need
> > to change the RPC parameters to a format which getopt() understands and
> > that also seemed unnecessary complicated.
> > 
> > The current configuration file code is nice that it just transforms the
> > configuration file to getopt() compatible input, on the other hand that
> > makes is complicated for the RPC case as we would need to create
> > getopt() compatible from the RPC options.
> 
> Well, yes. AFAIU the problem, we cannot easily change the way Docker (and others)
> "configure" CRIU via RPC and want some way to by-pass the configuration parameter 
> through it. Config files seem to suit this, but were designed to set some defaults
> that are to be used if no other ... recommendations are given, not to nail down any
> option.
> 
> Trying to make config file become docker-transparend-cli doesn't just "looks wrong",
> it's also problematic since fixing a global config would affect every single CRIU 
> invocation that might happen in between. To get a "CLI/RPC overrider" we need
> something, that it local to a single CRIU invocation... maybe another "local" or
> "one-shot" config file, that CRIU will read after reading the global one and parsing
> CLI/RPC. But can we tell CRIU where this one-shot file is through docker/lxd call?

Pavel, thanks for your comments, although I am not sure what you
propose. So for RPC you basically say we do not want to have
configuration files, right? So should we merge the CLI patches?
Everything concerning the CLI patches seems to be as everyone expects.

I guess I understand the 'one-shot' idea but I am also not sure how it
should work. Maybe you are also not sure ;) We are looking for mechanism
to influence LXD (which uses CLI) and docker/runc (which uses RPC). But
it should be different than configuration files, right?

		Adrian