Redesigning Configuration Manager (CfgMgr) class

[Stephen Morris]

On 27/08/14 09:10, Marcin Siodelski wrote:
> : :

So if I understand it right, the idea is that each configuration item
(e.g. list of Subnet4) has two objects:

1) A parser that interprets the data.
2) Objects that store the value of the configuration elements.

We still execute a two-phase approach: in the build() method, the
parser interprets the data, validates it and places it into a copy of
the data object (known as a "staging" object).  When commit() is
executed, the parser changes a pointer, so what was "staging" data
becomes the "current" data.  The original "current" data is retained
and becomes "old" data.  We also add a rollback() method that will
move "old" data back to "current" data.

That seems reasonable (although I doubt we will need more than one
level of rollback).


> Note that the use of "staging" configuration eliminates a need for 
> keeping an additional parser context which we currently use for
> keeping data integrity. This also eliminates a need for parsers to
> have build and commit phases. The commit is an atomic operation
> executed at the very end of the configuration parsing and it is
> always exception free. In build phase exceptions are possible and
> build() functions of configruation parsers operate on the "staging"
> configuration which they query from the CfgMgr. There will also be
> no need to provide storages for configuration parsers because
> configuration parsers will use the "staging" configuration as a
> storage.

I see that commit() and rollback() are likely to be methods in the
base class of parsers, but I would not remove the ability of parsers
to override them; some parsers may need to do additional work on a
commit() call (e.g. the hooks subsystem should load the new libraries
when commit() is called).


> Overall, I am pretty confident that those simple changes will
> reduce the complexity of the configuration parsers which is a pain
> in the back right now.
> 
> Feedback appreciated as well as additional requirements.

Additional requirements:

1) The parser code should be moved out of dhcpsrv and into a separate
library.

2) Parsers should be registered with the configuration manager rather
than be built into it.  So if we add a new feature to the code, the
parser for it can be in the same module/library as the feature. (This
has the implication that the parser needs to be able to specify the
part of the configuration file it is parsing, rather than the
configuration manager "knowing" it and calling the parser at the right
time.)


Stephen