From: Marcin Siodelski <marcin@isc.org>
Date: Tue, 2 Sep 2014 18:47:10 +0000 (+0200)
Subject: [3534] Commit and rollback the staging configuration.
X-Git-Tag: trac3595_base~7^2~4
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=615df6d2fdc5b66410d17f3edbd9e9fbf6273c8b;p=thirdparty%2Fkea.git

[3534] Commit and rollback the staging configuration.
---

diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc
index 10cf95b7f6..a0c4aeecec 100644
--- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc
+++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc
@@ -145,19 +145,16 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) {
     }
 
     // Configuration may change active interfaces. Therefore, we have to reopen
-    // sockets according to new configuration. This operation is not exception
-    // safe and we really don't want to emit exceptions to whoever called this
-    // method. Instead, catch an exception and create appropriate answer.
-    try {
-        CfgMgr::instance().getCurrentCfg()->getCfgIface()
-            .openSockets(CfgIface::V4, srv->getPort(),
-                         getInstance()->useBroadcast());
+    // sockets according to new configuration. It is possible that this
+    // operation will fail for some interfaces but the openSockets function
+    // guards against exceptions and invokes a callback function to
+    // log warnings. Since we allow that this fails for some interfaces there
+    // is no need to rollback configuration if socket fails to open on any
+    // of the interfaces.
+    CfgMgr::instance().getStagingCfg()->
+        getCfgIface().openSockets(CfgIface::V4, srv->getPort(),
+                                  getInstance()->useBroadcast());
 
-    } catch (std::exception& ex) {
-        err << "failed to open sockets after server reconfiguration: "
-            << ex.what();
-        answer = isc::config::createAnswer(1, err.str());
-    }
     return (answer);
 }
 
@@ -177,7 +174,7 @@ void ControlledDhcpv4Srv::shutdown() {
 
 ControlledDhcpv4Srv::~ControlledDhcpv4Srv() {
     cleanup();
-    
+
     server_ = NULL; // forget this instance. Noone should call any handlers at
                     // this stage.
 }
diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc
index 7dc972490c..abb54772d2 100644
--- a/src/bin/dhcp4/json_config_parser.cc
+++ b/src/bin/dhcp4/json_config_parser.cc
@@ -648,7 +648,6 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
     // Rollback changes as the configuration parsing failed.
     if (rollback) {
         globalContext().reset(new ParserContext(original_context));
-        CfgMgr::instance().rollback();
         return (answer);
     }
 
diff --git a/src/bin/dhcp4/kea_controller.cc b/src/bin/dhcp4/kea_controller.cc
index 2098eb1f36..66317fb1b7 100644
--- a/src/bin/dhcp4/kea_controller.cc
+++ b/src/bin/dhcp4/kea_controller.cc
@@ -102,7 +102,14 @@ void configure(const std::string& file_name) {
             isc_throw(isc::BadValue, reason);
         }
 
+        // Configuration successful.
+        CfgMgr::instance().commit();
+
     }  catch (const std::exception& ex) {
+        // If configuration failed at any stage, we drop the staging
+        // configuration and continue to use the previous one.
+        CfgMgr::instance().rollback();
+
         LOG_ERROR(dhcp4_logger, DHCP4_CONFIG_LOAD_FAIL)
             .arg(file_name).arg(ex.what());
         isc_throw(isc::BadValue, "configuration error using file '"
diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc
index 1ba9ba8934..e351e502b9 100644
--- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc
+++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc
@@ -140,19 +140,14 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) {
     }
 
     // Configuration may change active interfaces. Therefore, we have to reopen
-    // sockets according to new configuration. This operation is not exception
-    // safe and we really don't want to emit exceptions to the callback caller.
-    // Instead, catch an exception and create appropriate answer.
-    try {
-        CfgMgr::instance().getCurrentCfg()->getCfgIface()
-            .openSockets(CfgIface::V6, srv->getPort());
-
-    } catch (const std::exception& ex) {
-        std::ostringstream err;
-        err << "failed to open sockets after server reconfiguration: "
-            << ex.what();
-        answer = isc::config::createAnswer(1, err.str());
-    }
+    // sockets according to new configuration. It is possible that this
+    // operation will fail for some interfaces but the openSockets function
+    // guards against exceptions and invokes a callback function to
+    // log warnings. Since we allow that this fails for some interfaces there
+    // is no need to rollback configuration if socket fails to open on any
+    // of the interfaces.
+    CfgMgr::instance().getStagingCfg()->
+        getCfgIface().openSockets(CfgIface::V6, srv->getPort());
 
     return (answer);
 }
diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc
index c45724f5ba..ea98266bf3 100644
--- a/src/bin/dhcp6/json_config_parser.cc
+++ b/src/bin/dhcp6/json_config_parser.cc
@@ -849,7 +849,6 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) {
     // Rollback changes as the configuration parsing failed.
     if (rollback) {
         globalContext().reset(new ParserContext(original_context));
-        CfgMgr::instance().rollback();
         return (answer);
     }
 
diff --git a/src/bin/dhcp6/kea_controller.cc b/src/bin/dhcp6/kea_controller.cc
index 58ca5ad022..603b3c7823 100644
--- a/src/bin/dhcp6/kea_controller.cc
+++ b/src/bin/dhcp6/kea_controller.cc
@@ -106,7 +106,14 @@ void configure(const std::string& file_name) {
             isc_throw(isc::BadValue, reason);
         }
 
+        // Configuration successful.
+        CfgMgr::instance().commit();
+
     }  catch (const std::exception& ex) {
+        // If configuration failed at any stage, we drop the staging
+        // configuration and continue to use the previous one.
+        CfgMgr::instance().rollback();
+
         LOG_ERROR(dhcp6_logger, DHCP6_CONFIG_LOAD_FAIL)
             .arg(file_name).arg(ex.what());
         isc_throw(isc::BadValue, "configuration error using file '"