[5019_rebase] Review changes:

 - RSOOListConfigParser migrated
 - Global parser tweaked (SrvConfig passed as parameter)
 - duplicate_option_warning removed

diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes
index 293fac72d329368a932fba9c936b0961ea31288e..d90bee729240d8e66fac9c0bef6c19bb567d8f73 100644 (file)
--- a/src/bin/dhcp4/dhcp4_messages.mes
+++ b/src/bin/dhcp4/dhcp4_messages.mes
@@ -533,11 +533,6 @@ server configuration, a set of parsers were successfully created, but
 one of them failed to commit its changes.  The reason for the failure
 is given in the message.
 
-% DHCP4_PARSER_CREATED created parser for configuration element %1
-A debug message output during a configuration update of the DHCPv4
-server, notifying that the parser for the specified configuration element
-has been successfully created.
-
 % DHCP4_PARSER_EXCEPTION failed to create or run parser for configuration element %1
 On receipt of message containing details to a change of its configuration,
 the DHCPv4 server failed to create a parser to decode the contents of

diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc
index 4eba35490076b0bd2b1dce83b9302f6aa780ca2f..2581a687ca229c2b766291e50f0887b26c47ed8d 100644 (file)
--- a/src/bin/dhcp4/json_config_parser.cc
+++ b/src/bin/dhcp4/json_config_parser.cc
@@ -196,19 +196,6 @@ protected:
         return (parser);
     }
 
-    /// @brief Issues a DHCP4 server specific warning regarding duplicate subnet
-    /// options.
-    ///
-    /// @param code is the numeric option code of the duplicate option
-    /// @param addr is the subnet address
-    /// @todo a means to know the correct logger and perhaps a common
-    /// message would allow this method to be emitted by the base class.
-    virtual void duplicate_option_warning(uint32_t code,
-                                         isc::asiolink::IOAddress& addr) {
-        LOG_WARN(dhcp4_logger, DHCP4_CONFIG_OPTION_DUPLICATE)
-            .arg(code).arg(addr.toText());
-    }
-
     /// @brief Instantiates the IPv4 Subnet based on a given IPv4 address
     /// and prefix length.
     ///
@@ -390,6 +377,7 @@ public:
     /// @brief Sets global parameters in staging configuration
     ///
     /// @param global global configuration scope
+    /// @param cfg Server configuration (parsed parameters will be stored here)
     ///
     /// Currently this method sets the following global parameters:
     ///
@@ -399,14 +387,13 @@ public:
     ///
     /// @throw DhcpConfigError if parameters are missing or
     /// or having incorrect values.
-    void parse(ConstElementPtr global) {
+    void parse(SrvConfigPtr cfg, ConstElementPtr global) {
 
         // Set whether v4 server is supposed to echo back client-id
         // (yes = RFC6842 compatible, no = backward compatibility)
         bool echo_client_id = getBoolean(global, "echo-client-id");
         CfgMgr::instance().echoClientId(echo_client_id);
 
-        SrvConfigPtr srv_cfg = CfgMgr::instance().getStagingCfg();
         std::string name;
         ConstElementPtr value;
         try {
@@ -414,14 +401,14 @@ public:
             name = "decline-probation-period";
             value = global->get(name);
             uint32_t probation_period = getUint32(name, value);
-            srv_cfg->setDeclinePeriod(probation_period);
+            cfg->setDeclinePeriod(probation_period);
 
             // Set the DHCPv4-over-DHCPv6 interserver port.
             name = "dhcp4o6-port";
             value = global->get(name);
             // @todo Change for uint16_t
             uint32_t dhcp4o6_port = getUint32(name, value);
-            srv_cfg->setDhcp4o6Port(dhcp4o6_port);
+            cfg->setDhcp4o6Port(dhcp4o6_port);
         } catch (const isc::data::TypeError& ex) {
             isc_throw(DhcpConfigError,
                       "invalid value type specified for parameter '" << name
@@ -553,16 +540,6 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
     // for option definitions. This is equivalent to commiting empty container.
     LibDHCP::setRuntimeOptionDefs(OptionDefSpaceContainer());
 
-    // Some of the values specified in the configuration depend on
-    // other values. Typically, the values in the subnet4 structure
-    // depend on the global values. Also, option values configuration
-    // must be performed after the option definitions configurations.
-    // Thus we group parsers and will fire them in the right order:
-    // all parsers other than: lease-database, subnet4 and option-data parser,
-    // then: option-data parser, subnet4 parser, lease-database parser.
-    // Please do not change this order!
-    ParserCollection independent_parsers;
-
     // Some of the parsers alter the state of the system in a way that can't
     // easily be undone. (Or alter it in a way such that undoing the change has
     // the same risk of failure as doing the change.)
@@ -720,21 +697,10 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
                 continue;
             }
 
-            /// @todo: 5116 ticket: remove this chunk below once all parser
-            /// tickets are done.
-            ParserPtr parser(createGlobalDhcp4ConfigParser(config_pair.first,
-                                                           config_pair.second));
-            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PARSER_CREATED)
-                      .arg(config_pair.first);
-
-            // Those parsers should be started before other
-            // parsers so we can call build straight away.
-            independent_parsers.push_back(parser);
-            parser->build(config_pair.second);
-            // The commit operation here may modify the global storage
-            // but we need it so as the subnet6 parser can access the
-            // parsed data.
-            parser->commit();
+            // If we got here, no code handled this parameter, so we bail out.
+            isc_throw(DhcpConfigError,
+                      "unsupported global configuration parameter: " << config_pair.first
+                      << " (" << config_pair.second->getPosition() << ")");
         }
 
         // Setup the command channel.
@@ -742,7 +708,7 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
 
         // Apply global options in the staging config.
         Dhcp4ConfigParser global_parser;
-        global_parser.parse(mutable_cfg);
+        global_parser.parse(srv_cfg, mutable_cfg);
 
     } catch (const isc::Exception& ex) {
         LOG_ERROR(dhcp4_logger, DHCP4_PARSER_FAIL)

diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc
index d32eaa7c8ad418c7848bf45b147a91ec10e03f20..f15611c6cea90e1f342ec3d5339c9c11c331891a 100644 (file)
--- a/src/bin/dhcp6/json_config_parser.cc
+++ b/src/bin/dhcp6/json_config_parser.cc
@@ -543,30 +543,17 @@ public:
 /// The options on this list can be specified using an option code or option
 /// name. Therefore, the values on the list should always be enclosed in
 /// "quotes".
-class RSOOListConfigParser : public DhcpConfigParser {
+class RSOOListConfigParser : public isc::data::SimpleParser {
 public:
 
-    /// @brief constructor
-    ///
-    /// As this is a dedicated parser, it must be used to parse
-    /// "relay-supplied-options" parameter only. All other types will throw exception.
-    ///
-    /// @param param_name name of the configuration parameter being parsed
-    /// @throw BadValue if supplied parameter name is not "relay-supplied-options"
-    RSOOListConfigParser(const std::string& param_name) {
-        if (param_name != "relay-supplied-options") {
-            isc_throw(BadValue, "Internal error. RSOO configuration "
-                      "parser called for the wrong parameter: " << param_name);
-        }
-    }
-
     /// @brief parses parameters value
     ///
     /// Parses configuration entry (list of sources) and adds each element
     /// to the RSOO list.
     ///
     /// @param value pointer to the content of parsed values
-    virtual void build(isc::data::ConstElementPtr value) {
+    /// @param cfg server configuration (RSOO will be stored here)
+    void parse(SrvConfigPtr cfg, isc::data::ConstElementPtr value) {
         try {
             BOOST_FOREACH(ConstElementPtr source_elem, value->listValue()) {
                 std::string option_str = source_elem->stringValue();
@@ -603,7 +590,7 @@ public:
                                   " relay-supplied-options");
                     }
                 }
-                CfgMgr::instance().getStagingCfg()->getCfgRSOO()->enable(code);
+                cfg->getCfgRSOO()->enable(code);
             }
         } catch (const std::exception& ex) {
             // Rethrow exception with the appended position of the parsed
@@ -611,9 +598,6 @@ public:
             isc_throw(DhcpConfigError, ex.what() << " (" << value->getPosition() << ")");
         }
     }
-
-    /// @brief Does nothing.
-    virtual void commit() {}
 };
 
 class Dhcp6ConfigParser : public isc::data::SimpleParser {
@@ -622,6 +606,7 @@ public:
     /// @brief Sets global parameters in staging configuration
     ///
     /// @param global global configuration scope
+    /// @param cfg Server configuration (parsed parameters will be stored here)
     ///
     /// Currently this method sets the following global parameters:
     ///
@@ -630,9 +615,8 @@ public:
     ///
     /// @throw DhcpConfigError if parameters are missing or
     /// or having incorrect values.
-    void parse(ConstElementPtr global) {
+    void parse(SrvConfigPtr srv_config, ConstElementPtr global) {
 
-        SrvConfigPtr srv_config = CfgMgr::instance().getStagingCfg();
         std::string name;
         ConstElementPtr value;
         try {
@@ -703,14 +687,13 @@ DhcpConfigParser* createGlobal6DhcpConfigParser(const std::string& config_id,
     // lease-database and hosts-database have been converted to SimpleParser already.
     // mac-source has been converted to SimpleParser.
     // dhcp-ddns has been converted to SimpleParser
-    if (config_id.compare("relay-supplied-options") == 0) {
-        parser = new RSOOListConfigParser(config_id);
+    // rsoo has been converted to SimpleParser.
     // control-socket has been converted to SimpleParser.
     // expired-leases-processing has been converted to SimpleParser.
     // client-classes has been converted to SimpleParser.
     // host-reservation-identifiers have been converted to SimpleParser already.
     // server-id has been migrated to SimpleParser
-    } else {
+    {
         isc_throw(DhcpConfigError,
                 "unsupported global configuration parameter: "
                   << config_id << " (" << element->getPosition() << ")");
@@ -967,6 +950,12 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) {
                 continue;
             }
 
+            if (config_pair.first == "relay-supplied-options") {
+                RSOOListConfigParser parser;
+                parser.parse(srv_config, config_pair.second);
+                continue;
+            }
+
             /// @todo: 5116 ticket: remove this chunk below once all parser
             /// tickets are done.
             ParserPtr parser(createGlobal6DhcpConfigParser(config_pair.first,
@@ -989,7 +978,7 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) {
 
         // Apply global options in the staging config.
         Dhcp6ConfigParser global_parser;
-        global_parser.parse(mutable_cfg);
+        global_parser.parse(srv_config, mutable_cfg);
 
     } catch (const isc::Exception& ex) {
         LOG_ERROR(dhcp6_logger, DHCP6_PARSER_FAIL)