BIND 10 trac2637, updated. c80274000aed9ab258b3e0b46ddb2c3212a7b877 [2637] Fix the DHCP configuration handling.

BIND 10 source code commits bind10-changes at lists.isc.org
Fri Jan 18 14:43:29 UTC 2013


The branch, trac2637 has been updated
       via  c80274000aed9ab258b3e0b46ddb2c3212a7b877 (commit)
      from  d31f5bd476744ced094fc2aecb557b945586b056 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit c80274000aed9ab258b3e0b46ddb2c3212a7b877
Author: Marcin Siodelski <marcin at isc.org>
Date:   Fri Jan 18 15:43:17 2013 +0100

    [2637] Fix the DHCP configuration handling.
    
    In particular, the new configuration is merged with the existing
    configuration to trigger the full reconfiguration of the server. This is
    to satisfy dependencies between various configuration values.

-----------------------------------------------------------------------

Summary of changes:
 src/bin/dhcp4/ctrl_dhcp4_srv.cc |   52 ++++++++++++++++++++++++++------
 src/bin/dhcp4/ctrl_dhcp4_srv.h  |   21 +++++++++++++
 src/bin/dhcp6/ctrl_dhcp6_srv.cc |   62 ++++++++++++++++++++++++++++++---------
 src/bin/dhcp6/ctrl_dhcp6_srv.h  |   21 +++++++++++++
 4 files changed, 133 insertions(+), 23 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc
index 7f533c0..04502c1 100644
--- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc
+++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc
@@ -47,7 +47,22 @@ namespace dhcp {
 ControlledDhcpv4Srv* ControlledDhcpv4Srv::server_ = NULL;
 
 ConstElementPtr
-ControlledDhcpv4Srv::dhcp4ConfigHandler(ConstElementPtr) {
+ControlledDhcpv4Srv::dhcp4StubConfigHandler(ConstElementPtr) {
+    // This configuration handler is intended to be used only
+    // when the initial configuration comes in. To receive this
+    // configuration a pointer to this handler must be passed
+    // using ModuleCCSession constructor. This constructor will
+    // invoke the handler and will store the configuration for
+    // the configuration session when the handler returns success.
+    // Since this configuration is partial we just pretend to
+    // parse it and always return success. The function that
+    // initiates the session must get the configuration on its
+    // own using getFullConfig.
+    return (isc::config::createAnswer(0, "Configuration accepted."));
+}
+
+ConstElementPtr
+ControlledDhcpv4Srv::dhcp4ConfigHandler(ConstElementPtr new_config) {
     if (!server_ || !server_->config_session_) {
         // That should never happen as we install config_handler
         // after we instantiate the server.
@@ -56,6 +71,7 @@ ControlledDhcpv4Srv::dhcp4ConfigHandler(ConstElementPtr) {
                                       " server is during startup/shutdown phase.");
         return (answer);
     }
+
     // The configuration passed to this handler function is partial.
     // In other words, it just includes the values being modified.
     // In the same time, there are dependencies between various
@@ -65,15 +81,26 @@ ControlledDhcpv4Srv::dhcp4ConfigHandler(ConstElementPtr) {
     // removes that definition is triggered while a relevant option value
     // may remain configured. This eventually results in the DHCP server
     // configuration being in the inconsistent state.
-    // In order to work around this problem we always get the full
-    // configuration for the server. This triggers all parsers to reset
-    // the configuration data and check the dependencies between values
-    // being set. Thus, the configuration passed to this function
-    // is ignored.
+    // In order to work around this problem we need to merge the new
+    // configuration with the existing (full) configuration.
+
+    // Let's create a new object that will hold the merged configuration.
+    boost::shared_ptr<MapElement> merged_config(new MapElement());
+    // Let's get the existing configuration.
     ConstElementPtr full_config = server_->config_session_->getFullConfig();
-    LOG_DEBUG(dhcp4_logger, DBG_DHCP4_COMMAND, DHCP4_CONFIG_UPDATE)
-              .arg(full_config->str());
+    // The full_config and merged_config should be always non-NULL
+    // but to provide some level of exception safety we check that they
+    // really are (in case we go out of memory).
+    if (full_config && merged_config) {
+        merged_config->setValue(full_config->mapValue());
+
+        // Merge an existing and new configuration.
+        isc::data::merge(merged_config, new_config);
+        LOG_DEBUG(dhcp4_logger, DBG_DHCP4_COMMAND, DHCP4_CONFIG_UPDATE)
+            .arg(full_config->str());
+    }
 
+    // Configure the server.
     return (configureDhcp4Server(*server_, full_config));
 }
 
@@ -125,8 +152,15 @@ void ControlledDhcpv4Srv::establishSession() {
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_CCSESSION_STARTING)
               .arg(specfile);
     cc_session_ = new Session(io_service_.get_io_service());
+    // Create a session with the dummy configuration handler.
+    // Dumy configuration handler is internally invoked by the
+    // constructor and on success the constructor updates
+    // the current session with the configuration that had been
+    // commited in the previous session. If we did not install
+    // the dummy handler, the previous configuration would have
+    // been lost.
     config_session_ = new ModuleCCSession(specfile, *cc_session_,
-                                          NULL,
+                                          dhcp4StubConfigHandler,
                                           dhcp4CommandHandler, false);
     config_session_->start();
 
diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.h b/src/bin/dhcp4/ctrl_dhcp4_srv.h
index 9bd261c..ab4d45a 100644
--- a/src/bin/dhcp4/ctrl_dhcp4_srv.h
+++ b/src/bin/dhcp4/ctrl_dhcp4_srv.h
@@ -94,6 +94,27 @@ protected:
     static isc::data::ConstElementPtr
     dhcp4ConfigHandler(isc::data::ConstElementPtr new_config);
 
+    /// @brief A dummy configuration handler that always returns success.
+    ///
+    /// This configuration handler does not perform configuration
+    /// parsing and always returns success. A dummy hanlder should
+    /// be installed using \ref isc::config::ModuleCCSession ctor
+    /// to get the initial configuration. This initial configuration
+    /// comprises values for only those elements that were modified
+    /// the previous session. The \ref dhcp4ConfigHandler can't be
+    /// used to parse the initial configuration because it needs the
+    /// full configuration to satisfy dependencies between the
+    /// various configuration values. Installing the dummy handler
+    /// that guarantees to return success causes initial configuration
+    /// to be stored for the session being created and that it can
+    /// be later accessed with \ref isc::ConfigData::getFullConfig.
+    ///
+    /// @param new_config new configuration.
+    ///
+    /// @return success configuration status.
+    static isc::data::ConstElementPtr
+    dhcp4StubConfigHandler(isc::data::ConstElementPtr new_config);
+
     /// @brief A callback for handling incoming commands.
     ///
     /// @param command textual representation of the command
diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc
index 3545e2d..f41e240 100644
--- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc
+++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc
@@ -46,7 +46,22 @@ namespace dhcp {
 ControlledDhcpv6Srv* ControlledDhcpv6Srv::server_ = NULL;
 
 ConstElementPtr
-ControlledDhcpv6Srv::dhcp6ConfigHandler(ConstElementPtr) {
+ControlledDhcpv6Srv::dhcp6StubConfigHandler(ConstElementPtr) {
+    // This configuration handler is intended to be used only
+    // when the initial configuration comes in. To receive this
+    // configuration a pointer to this handler must be passed
+    // using ModuleCCSession constructor. This constructor will
+    // invoke the handler and will store the configuration for
+    // the configuration session when the handler returns success.
+    // Since this configuration is partial we just pretend to
+    // parse it and always return success. The function that
+    // initiates the session must get the configuration on its
+    // own using getFullConfig.
+    return (isc::config::createAnswer(0, "Configuration accepted."));
+}
+
+ConstElementPtr
+ControlledDhcpv6Srv::dhcp6ConfigHandler(ConstElementPtr new_config) {
 
     if (!server_ || !server_->config_session_) {
         // That should never happen as we install config_handler
@@ -66,16 +81,27 @@ ControlledDhcpv6Srv::dhcp6ConfigHandler(ConstElementPtr) {
     // removes that definition is triggered while a relevant option value
     // may remain configured. This eventually results in the DHCP server
     // configuration being in the inconsistent state.
-    // In order to work around this problem we always get the full
-    // configuration for the server. This triggers all parsers to reset
-    // the configuration data and check the dependencies between values
-    // being set. Thus, the configuration passed to this function
-    // is ignored.
+    // In order to work around this problem we need to merge the new
+    // configuration with the existing (full) configuration.
+
+    // Let's create a new object that will hold the merged configuration.
+    boost::shared_ptr<MapElement> merged_config(new MapElement());
+    // Let's get the existing configuration.
     ConstElementPtr full_config = server_->config_session_->getFullConfig();
-    LOG_DEBUG(dhcp6_logger, DBG_DHCP6_COMMAND, DHCP6_CONFIG_UPDATE)
-              .arg(full_config->str());
+    // The full_config and merged_config should be always non-NULL
+    // but to provide some level of exception safety we check that they
+    // really are (in case we go out of memory).
+    if (full_config && merged_config) {
+        merged_config->setValue(full_config->mapValue());
+
+        // Merge an existing and new configuration.
+        isc::data::merge(merged_config, new_config);
+        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_COMMAND, DHCP6_CONFIG_UPDATE)
+            .arg(merged_config->str());
+    }
 
-    return (configureDhcp6Server(*server_, full_config));
+    // Configure the server.
+    return (configureDhcp6Server(*server_, merged_config));
 }
 
 ConstElementPtr
@@ -126,18 +152,26 @@ void ControlledDhcpv6Srv::establishSession() {
     LOG_DEBUG(dhcp6_logger, DBG_DHCP6_START, DHCP6_CCSESSION_STARTING)
               .arg(specfile);
     cc_session_ = new Session(io_service_.get_io_service());
+    // Create a session with the dummy configuration handler.
+    // Dumy configuration handler is internally invoked by the
+    // constructor and on success the constructor updates
+    // the current session with the configuration that had been
+    // commited in the previous session. If we did not install
+    // the dummy handler, the previous configuration would have
+    // been lost.
     config_session_ = new ModuleCCSession(specfile, *cc_session_,
-                                          NULL,
+                                          dhcp6StubConfigHandler,
                                           dhcp6CommandHandler, false);
     config_session_->start();
 
-    // We initially create ModuleCCSession() without configHandler, as
-    // the session module is too eager to send partial configuration.
-    // We want to get the full configuration, so we explicitly call
-    // getFullConfig() and then pass it to our configHandler.
+    // The constructor already pulled the configuration that had
+    // been created in the previous session thanks to the dummy
+    // handler. We can switch to the handler that will be
+    // parsing future changes to the configuration.
     config_session_->setConfigHandler(dhcp6ConfigHandler);
 
     try {
+        // Pull the full configuration out from the session.
         configureDhcp6Server(*this, config_session_->getFullConfig());
     } catch (const DhcpConfigError& ex) {
         LOG_ERROR(dhcp6_logger, DHCP6_CONFIG_LOAD_FAIL).arg(ex.what());
diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.h b/src/bin/dhcp6/ctrl_dhcp6_srv.h
index ef1acab..42c588d 100644
--- a/src/bin/dhcp6/ctrl_dhcp6_srv.h
+++ b/src/bin/dhcp6/ctrl_dhcp6_srv.h
@@ -94,6 +94,27 @@ protected:
     static isc::data::ConstElementPtr
     dhcp6ConfigHandler(isc::data::ConstElementPtr new_config);
 
+    /// @brief A dummy configuration handler that always returns success.
+    ///
+    /// This configuration handler does not perform configuration
+    /// parsing and always returns success. A dummy hanlder should
+    /// be installed using \ref isc::config::ModuleCCSession ctor
+    /// to get the initial configuration. This initial configuration
+    /// comprises values for only those elements that were modified
+    /// the previous session. The \ref dhcp6ConfigHandler can't be
+    /// used to parse the initial configuration because it needs the
+    /// full configuration to satisfy dependencies between the
+    /// various configuration values. Installing the dummy handler
+    /// that guarantees to return success causes initial configuration
+    /// to be stored for the session being created and that it can
+    /// be later accessed with \ref isc::ConfigData::getFullConfig.
+    ///
+    /// @param new_config new configuration.
+    ///
+    /// @return success configuration status.
+    static isc::data::ConstElementPtr
+    dhcp6StubConfigHandler(isc::data::ConstElementPtr new_config);
+
     /// @brief A callback for handling incoming commands.
     ///
     /// @param command textual representation of the command



More information about the bind10-changes mailing list