[master] Merge branch 'trac3588'

src/bin/dhcp4/dhcp4_srv.cc

@@ -1806,8 +1806,8 @@ Dhcpv4Srv::unpackOptions(const OptionBuffer& buf,
         // Get the list of stdandard option definitions.
         option_defs = LibDHCP::getOptionDefs(Option::V4);
     } else if (!option_space.empty()) {
-        OptionDefContainerPtr option_defs_ptr =
-            CfgMgr::instance().getOptionDefs(option_space);
+        OptionDefContainerPtr option_defs_ptr = CfgMgr::instance()
+            .getCurrentCfg()->getCfgOptionDef()->getAll(option_space);
         if (option_defs_ptr != NULL) {
             option_defs = *option_defs_ptr;
         }

src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -80,6 +80,7 @@ public:
         srv_.reset(new Dhcpv4Srv(0));
         // Create fresh context.
         globalContext()->copyContext(ParserContext(Option::V4));
+        resetConfiguration();
     }
 
     // Check that no hooks libraries are loaded.  This is a pre-condition for
@@ -444,8 +445,6 @@ public:
     void resetConfiguration() {
         string config = "{ \"interfaces\": [ \"*\" ],"
             "\"hooks-libraries\": [ ], "
-            "\"rebind-timer\": 2000, "
-            "\"renew-timer\": 1000, "
             "\"valid-lifetime\": 4000, "
             "\"subnet4\": [ ], "
             "\"dhcp-ddns\": { \"enable-updates\" : false }, "
@@ -453,6 +452,7 @@ public:
             "\"option-data\": [ ] }";
         static_cast<void>(executeConfiguration(config,
                                                "reset configuration database"));
+        CfgMgr::instance().clear();
     }
 
 
@@ -1231,7 +1231,8 @@ TEST_F(Dhcp4ParserTest, optionDefIpv4Address) {
     ElementPtr json = Element::fromJSON(config);
 
     // Make sure that the particular option definition does not exist.
-    OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100);
+    OptionDefinitionPtr def = CfgMgr::instance().getStagingCfg()->
+        getCfgOptionDef()->get("isc", 100);
     ASSERT_FALSE(def);
 
     // Use the configuration string to create new option definition.
@@ -1241,7 +1242,7 @@ TEST_F(Dhcp4ParserTest, optionDefIpv4Address) {
     checkResult(status, 0);
 
     // The option definition should now be available in the CfgMgr.
-    def = CfgMgr::instance().getOptionDef("isc", 100);
+    def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->get("isc", 100);
     ASSERT_TRUE(def);
 
     // Verify that the option definition data is valid.
@@ -1272,7 +1273,8 @@ TEST_F(Dhcp4ParserTest, optionDefRecord) {
     ElementPtr json = Element::fromJSON(config);
 
     // Make sure that the particular option definition does not exist.
-    OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100);
+    OptionDefinitionPtr def = CfgMgr::instance().getStagingCfg()->
+        getCfgOptionDef()->get("isc", 100);
     ASSERT_FALSE(def);
 
     // Use the configuration string to create new option definition.
@@ -1282,7 +1284,7 @@ TEST_F(Dhcp4ParserTest, optionDefRecord) {
     checkResult(status, 0);
 
     // The option definition should now be available in the CfgMgr.
-    def = CfgMgr::instance().getOptionDef("isc", 100);
+    def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->get("isc", 100);
     ASSERT_TRUE(def);
 
     // Check the option data.
@@ -1330,8 +1332,10 @@ TEST_F(Dhcp4ParserTest, optionDefMultiple) {
     ElementPtr json = Element::fromJSON(config);
 
     // Make sure that the option definitions do not exist yet.
-    ASSERT_FALSE(CfgMgr::instance().getOptionDef("isc", 100));
-    ASSERT_FALSE(CfgMgr::instance().getOptionDef("isc", 101));
+    ASSERT_FALSE(CfgMgr::instance().getStagingCfg()->
+                 getCfgOptionDef()->get("isc", 100));
+    ASSERT_FALSE(CfgMgr::instance().getStagingCfg()->
+                 getCfgOptionDef()->get("isc", 101));
 
     // Use the configuration string to create new option definitions.
     ConstElementPtr status;
@@ -1340,7 +1344,8 @@ TEST_F(Dhcp4ParserTest, optionDefMultiple) {
     checkResult(status, 0);
 
     // Check the first definition we have created.
-    OptionDefinitionPtr def1 = CfgMgr::instance().getOptionDef("isc", 100);
+    OptionDefinitionPtr def1 = CfgMgr::instance().getStagingCfg()->
+        getCfgOptionDef()->get("isc", 100);
     ASSERT_TRUE(def1);
 
     // Check the option data.
@@ -1351,7 +1356,8 @@ TEST_F(Dhcp4ParserTest, optionDefMultiple) {
     EXPECT_TRUE(def1->getEncapsulatedSpace().empty());
 
     // Check the second option definition we have created.
-    OptionDefinitionPtr def2 = CfgMgr::instance().getOptionDef("isc", 101);
+    OptionDefinitionPtr def2 = CfgMgr::instance().getStagingCfg()->
+        getCfgOptionDef()->get("isc", 101);
     ASSERT_TRUE(def2);
 
     // Check the option data.
@@ -1392,7 +1398,8 @@ TEST_F(Dhcp4ParserTest, optionDefDuplicate) {
     ElementPtr json = Element::fromJSON(config);
 
     // Make sure that the option definition does not exist yet.
-    ASSERT_FALSE(CfgMgr::instance().getOptionDef("isc", 100));
+    ASSERT_FALSE(CfgMgr::instance().getStagingCfg()->
+                 getCfgOptionDef()->get("isc", 100));
 
     // Use the configuration string to create new option definitions.
     ConstElementPtr status;
@@ -1422,7 +1429,8 @@ TEST_F(Dhcp4ParserTest, optionDefArray) {
     ElementPtr json = Element::fromJSON(config);
 
     // Make sure that the particular option definition does not exist.
-    OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100);
+    OptionDefinitionPtr def = CfgMgr::instance().getStagingCfg()->
+        getCfgOptionDef()->get("isc", 100);
     ASSERT_FALSE(def);
 
     // Use the configuration string to create new option definition.
@@ -1432,7 +1440,8 @@ TEST_F(Dhcp4ParserTest, optionDefArray) {
     checkResult(status, 0);
 
     // The option definition should now be available in the CfgMgr.
-    def = CfgMgr::instance().getOptionDef("isc", 100);
+    def = CfgMgr::instance().getStagingCfg()->
+        getCfgOptionDef()->get("isc", 100);
     ASSERT_TRUE(def);
 
     // Check the option data.
@@ -1463,7 +1472,8 @@ TEST_F(Dhcp4ParserTest, optionDefEncapsulate) {
     ElementPtr json = Element::fromJSON(config);
 
     // Make sure that the particular option definition does not exist.
-    OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100);
+    OptionDefinitionPtr def = CfgMgr::instance().getStagingCfg()->
+        getCfgOptionDef()->get("isc", 100);
     ASSERT_FALSE(def);
 
     // Use the configuration string to create new option definition.
@@ -1473,7 +1483,8 @@ TEST_F(Dhcp4ParserTest, optionDefEncapsulate) {
     checkResult(status, 0);
 
     // The option definition should now be available in the CfgMgr.
-    def = CfgMgr::instance().getOptionDef("isc", 100);
+    def = CfgMgr::instance().getStagingCfg()->
+        getCfgOptionDef()->get("isc", 100);
     ASSERT_TRUE(def);
 
     // Check the option data.
@@ -1670,7 +1681,8 @@ TEST_F(Dhcp4ParserTest, optionStandardDefOverride) {
         "}";
     ElementPtr json = Element::fromJSON(config);
 
-    OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("dhcp4", 109);
+    OptionDefinitionPtr def = CfgMgr::instance().getStagingCfg()->
+        getCfgOptionDef()->get("dhcp4", 109);
     ASSERT_FALSE(def);
 
     // Use the configuration string to create new option definition.
@@ -1680,7 +1692,8 @@ TEST_F(Dhcp4ParserTest, optionStandardDefOverride) {
     checkResult(status, 0);
 
     // The option definition should now be available in the CfgMgr.
-    def = CfgMgr::instance().getOptionDef("dhcp4", 109);
+    def = CfgMgr::instance().getStagingCfg()->
+        getCfgOptionDef()->get("dhcp4", 109);
     ASSERT_TRUE(def);
 
     // Check the option data.
@@ -1736,7 +1749,8 @@ TEST_F(Dhcp4ParserTest, optionStandardDefOverride) {
     // Expecting success.
     checkResult(status, 0);
 
-    def = CfgMgr::instance().getOptionDef("dhcp4", 65);
+    def = CfgMgr::instance().getStagingCfg()->
+        getCfgOptionDef()->get("dhcp4", 65);
     ASSERT_TRUE(def);
 
     // Check the option data.
@@ -1950,6 +1964,8 @@ TEST_F(Dhcp4ParserTest, optionDataEncapsulate) {
     ASSERT_TRUE(status);
     checkResult(status, 0);
 
+    CfgMgr::instance().clear();
+
     // Stage 2. Configure base option and a subnet. Please note that
     // the configuration from the stage 2 is repeated because BIND
     // configuration manager sends whole configuration for the lists
@@ -2524,6 +2540,8 @@ TEST_F(Dhcp4ParserTest, stdOptionDataEncapsulate) {
     ASSERT_TRUE(status);
     checkResult(status, 0);
 
+    CfgMgr::instance().clear();
+
     // Once the definitions have been added we can configure the
     // standard option #17. This option comprises an enterprise
     // number and sub options. By convention (introduced in
@@ -2879,6 +2897,10 @@ TEST_F(Dhcp4ParserTest, LibrariesSpecified) {
     EXPECT_TRUE(checkMarkerFile(LOAD_MARKER_FILE, "12"));
     EXPECT_FALSE(checkMarkerFileExists(UNLOAD_MARKER_FILE));
 
+    // Commit the changes so as we get the fresh configuration for the
+    // second part of this test.
+    CfgMgr::instance().commit();
+
     // Unload the libraries.  The load file should not have changed, but
     // the unload one should indicate the unload() functions have been run.
     config = buildHooksLibrariesConfig();

src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -1315,10 +1315,12 @@ TEST_F(Dhcpv4SrvTest, unpackOptions) {
 
     // Add option definitions to the Configuration Manager. Each goes under
     // different option space.
-    CfgMgr& cfgmgr = CfgMgr::instance();
-    ASSERT_NO_THROW(cfgmgr.addOptionDef(opt_def, "space-foobar"));
-    ASSERT_NO_THROW(cfgmgr.addOptionDef(opt_def2, "space-foo"));
-    ASSERT_NO_THROW(cfgmgr.addOptionDef(opt_def3, "space-bar"));
+    CfgOptionDefPtr cfg_option_def =
+        CfgMgr::instance().getStagingCfg()->getCfgOptionDef();
+    ASSERT_NO_THROW(cfg_option_def->add(opt_def, "space-foobar"));
+    ASSERT_NO_THROW(cfg_option_def->add(opt_def2, "space-foo"));
+    ASSERT_NO_THROW(cfg_option_def->add(opt_def3, "space-bar"));
+    CfgMgr::instance().commit();
 
     // Create the buffer holding the structure of options.
     const char raw_data[] = {

src/bin/dhcp4/tests/kea_controller_unittest.cc

@@ -18,6 +18,7 @@
 #include <dhcp/dhcp4.h>
 #include <dhcp4/ctrl_dhcp4_srv.h>
 #include <dhcpsrv/cfgmgr.h>
+#include <log/logger_support.h>
 
 #include <boost/scoped_ptr.hpp>
 #include <gtest/gtest.h>
@@ -57,6 +58,7 @@ public:
     }
 
     ~JSONFileBackendTest() {
+        isc::log::setDefaultLoggingOutput();
         static_cast<void>(unlink(TEST_FILE));
     };
 

src/bin/dhcp6/dhcp6_srv.cc

@@ -2444,7 +2444,8 @@ Dhcpv6Srv::unpackOptions(const OptionBuffer& buf,
         option_defs = LibDHCP::getOptionDefs(Option::V6);
     } else if (!option_space.empty()) {
         OptionDefContainerPtr option_defs_ptr =
-            CfgMgr::instance().getOptionDefs(option_space);
+            CfgMgr::instance().getCurrentCfg()->getCfgOptionDef()->
+            getAll(option_space);
         if (option_defs_ptr != NULL) {
             option_defs = *option_defs_ptr;
         }

src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -302,6 +302,7 @@ public:
         try {
             ElementPtr json = Element::fromJSON(config);
             status = configureDhcp6Server(srv_, json);
+
         } catch (const std::exception& ex) {
             ADD_FAILURE() << "Unable to " << operation << ". "
                    << "The following configuration was used: " << std::endl
@@ -382,6 +383,7 @@ public:
         EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json));
         checkResult(x, 1);
         EXPECT_TRUE(errorContainsPosition(x, "<string>"));
+        CfgMgr::instance().clear();
     }
 
     /// @brief Test invalid option paramater value.
@@ -399,6 +401,7 @@ public:
         EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json));
         checkResult(x, 1);
         EXPECT_TRUE(errorContainsPosition(x, "<string>"));
+        CfgMgr::instance().clear();
     }
 
     /// @brief Test option against given code and data.
@@ -468,11 +471,13 @@ public:
                            const size_t expected_data_len) {
         std::string config = createConfigWithOption(params);
         ASSERT_TRUE(executeConfiguration(config, "parse option configuration"));
+
         // The subnet should now hold one option with the specified code.
         Subnet::OptionDescriptor desc =
             getOptionFromSubnet(IOAddress("2001:db8:1::5"), option_code);
         ASSERT_TRUE(desc.option);
         testOption(desc, option_code, expected_data, expected_data_len);
+        CfgMgr::instance().clear();
     }
 
     int rcode_;          ///< Return code (see @ref isc::config::parseAnswer)
@@ -1466,7 +1471,8 @@ TEST_F(Dhcp6ParserTest, optionDefIpv6Address) {
     ElementPtr json = Element::fromJSON(config);
 
     // Make sure that the particular option definition does not exist.
-    OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100);
+    OptionDefinitionPtr def = CfgMgr::instance().getStagingCfg()->
+        getCfgOptionDef()->get("isc", 100);
     ASSERT_FALSE(def);
 
     // Use the configuration string to create new option definition.
@@ -1475,7 +1481,7 @@ TEST_F(Dhcp6ParserTest, optionDefIpv6Address) {
     ASSERT_TRUE(status);
 
     // The option definition should now be available in the CfgMgr.
-    def = CfgMgr::instance().getOptionDef("isc", 100);
+    def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->get("isc", 100);
     ASSERT_TRUE(def);
 
     // Verify that the option definition data is valid.
@@ -1505,7 +1511,8 @@ TEST_F(Dhcp6ParserTest, optionDefRecord) {
     ElementPtr json = Element::fromJSON(config);
 
     // Make sure that the particular option definition does not exist.
-    OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100);
+    OptionDefinitionPtr def = CfgMgr::instance().getStagingCfg()->
+        getCfgOptionDef()->get("isc", 100);
     ASSERT_FALSE(def);
 
     // Use the configuration string to create new option definition.
@@ -1515,7 +1522,7 @@ TEST_F(Dhcp6ParserTest, optionDefRecord) {
     checkResult(status, 0);
 
     // The option definition should now be available in the CfgMgr.
-    def = CfgMgr::instance().getOptionDef("isc", 100);
+    def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->get("isc", 100);
     ASSERT_TRUE(def);
 
     // Check the option data.
@@ -1562,8 +1569,10 @@ TEST_F(Dhcp6ParserTest, optionDefMultiple) {
     ElementPtr json = Element::fromJSON(config);
 
     // Make sure that the option definitions do not exist yet.
-    ASSERT_FALSE(CfgMgr::instance().getOptionDef("isc", 100));
-    ASSERT_FALSE(CfgMgr::instance().getOptionDef("isc", 101));
+    ASSERT_FALSE(CfgMgr::instance().getStagingCfg()->
+                 getCfgOptionDef()->get("isc", 100));
+    ASSERT_FALSE(CfgMgr::instance().getStagingCfg()->
+                 getCfgOptionDef()->get("isc", 101));
 
     // Use the configuration string to create new option definitions.
     ConstElementPtr status;
@@ -1572,7 +1581,8 @@ TEST_F(Dhcp6ParserTest, optionDefMultiple) {
     checkResult(status, 0);
 
     // Check the first definition we have created.
-    OptionDefinitionPtr def1 = CfgMgr::instance().getOptionDef("isc", 100);
+    OptionDefinitionPtr def1 = CfgMgr::instance().getStagingCfg()->
+        getCfgOptionDef()->get("isc", 100);
     ASSERT_TRUE(def1);
 
     // Check the option data.
@@ -1582,7 +1592,8 @@ TEST_F(Dhcp6ParserTest, optionDefMultiple) {
     EXPECT_FALSE(def1->getArrayType());
 
     // Check the second option definition we have created.
-    OptionDefinitionPtr def2 = CfgMgr::instance().getOptionDef("isc", 101);
+    OptionDefinitionPtr def2 = CfgMgr::instance().getStagingCfg()->
+        getCfgOptionDef()->get("isc", 101);
     ASSERT_TRUE(def2);
 
     // Check the option data.
@@ -1622,7 +1633,8 @@ TEST_F(Dhcp6ParserTest, optionDefDuplicate) {
     ElementPtr json = Element::fromJSON(config);
 
     // Make sure that the option definition does not exist yet.
-    ASSERT_FALSE(CfgMgr::instance().getOptionDef("isc", 100));
+    ASSERT_FALSE(CfgMgr::instance().getStagingCfg()->
+                 getCfgOptionDef()->get("isc", 100));
 
     // Use the configuration string to create new option definitions.
     ConstElementPtr status;
@@ -1652,7 +1664,8 @@ TEST_F(Dhcp6ParserTest, optionDefArray) {
     ElementPtr json = Element::fromJSON(config);
 
     // Make sure that the particular option definition does not exist.
-    OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100);
+    OptionDefinitionPtr def = CfgMgr::instance().getStagingCfg()->
+        getCfgOptionDef()->get("isc", 100);
     ASSERT_FALSE(def);
 
     // Use the configuration string to create new option definition.
@@ -1662,7 +1675,7 @@ TEST_F(Dhcp6ParserTest, optionDefArray) {
     checkResult(status, 0);
 
     // The option definition should now be available in the CfgMgr.
-    def = CfgMgr::instance().getOptionDef("isc", 100);
+    def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->get("isc", 100);
     ASSERT_TRUE(def);
 
     // Check the option data.
@@ -1692,7 +1705,8 @@ TEST_F(Dhcp6ParserTest, optionDefEncapsulate) {
     ElementPtr json = Element::fromJSON(config);
 
     // Make sure that the particular option definition does not exist.
-    OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100);
+    OptionDefinitionPtr def = CfgMgr::instance().getStagingCfg()->
+        getCfgOptionDef()->get("isc", 100);
     ASSERT_FALSE(def);
 
     // Use the configuration string to create new option definition.
@@ -1702,7 +1716,7 @@ TEST_F(Dhcp6ParserTest, optionDefEncapsulate) {
     checkResult(status, 0);
 
     // The option definition should now be available in the CfgMgr.
-    def = CfgMgr::instance().getOptionDef("isc", 100);
+    def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->get("isc", 100);
     ASSERT_TRUE(def);
 
     // Check the option data.
@@ -1900,7 +1914,8 @@ TEST_F(Dhcp6ParserTest, optionStandardDefOverride) {
         "}";
     ElementPtr json = Element::fromJSON(config);
 
-    OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("dhcp6", 100);
+    OptionDefinitionPtr def = CfgMgr::instance().getStagingCfg()->
+        getCfgOptionDef()->get("dhcp6", 100);
     ASSERT_FALSE(def);
 
     // Use the configuration string to create new option definition.
@@ -1910,7 +1925,8 @@ TEST_F(Dhcp6ParserTest, optionStandardDefOverride) {
     checkResult(status, 0);
 
     // The option definition should now be available in the CfgMgr.
-    def = CfgMgr::instance().getOptionDef("dhcp6", 100);
+    def = CfgMgr::instance().getStagingCfg()->
+        getCfgOptionDef()->get("dhcp6", 100);
     ASSERT_TRUE(def);
 
     // Check the option data.
@@ -1966,7 +1982,8 @@ TEST_F(Dhcp6ParserTest, optionStandardDefOverride) {
     // Expecting success.
     checkResult(status, 0);
 
-    def = CfgMgr::instance().getOptionDef("dhcp6", 59);
+    def = CfgMgr::instance().getStagingCfg()->
+        getCfgOptionDef()->get("dhcp6", 59);
     ASSERT_TRUE(def);
 
     // Check the option data.
@@ -2190,6 +2207,8 @@ TEST_F(Dhcp6ParserTest, optionDataEncapsulate) {
     ASSERT_TRUE(status);
     checkResult(status, 0);
 
+    CfgMgr::instance().clear();
+
     // Stage 2. Configure base option and a subnet. Please note that
     // the configuration from the stage 2 is repeated because BIND
     // configuration manager sends whole configuration for the lists
@@ -2383,6 +2402,8 @@ TEST_F(Dhcp6ParserTest, optionDataBoolean) {
     ASSERT_TRUE(executeConfiguration(config, "parse configuration with a"
                                      " boolean value"));
 
+    CfgMgr::instance().commit();
+
     // The subnet should now hold one option with the code 1000.
     Subnet::OptionDescriptor desc =
         getOptionFromSubnet(IOAddress("2001:db8:1::5"), 1000);
@@ -2773,6 +2794,8 @@ TEST_F(Dhcp6ParserTest, stdOptionDataEncapsulate) {
     ASSERT_TRUE(status);
     checkResult(status, 0);
 
+    CfgMgr::instance().clear();
+
     // Once the definitions have been added we can configure the
     // standard option #17. This option comprises an enterprise
     // number and sub options. By convention (introduced in
@@ -3015,6 +3038,8 @@ TEST_F(Dhcp6ParserTest, LibrariesSpecified) {
     EXPECT_TRUE(checkMarkerFile(LOAD_MARKER_FILE, "12"));
     EXPECT_FALSE(checkMarkerFileExists(UNLOAD_MARKER_FILE));
 
+    CfgMgr::instance().commit();
+
     // Unload the libraries.  The load file should not have changed, but
     // the unload one should indicate the unload() functions have been run.
     config = buildHooksLibrariesConfig();

src/bin/dhcp6/tests/dhcp6_srv_unittest.cc

@@ -1713,10 +1713,12 @@ TEST_F(Dhcpv6SrvTest, unpackOptions) {
 
     // Add option definitions to the Configuration Manager. Each goes under
     // different option space.
-    CfgMgr& cfgmgr = CfgMgr::instance();
-    ASSERT_NO_THROW(cfgmgr.addOptionDef(opt_def, "space-foobar"));
-    ASSERT_NO_THROW(cfgmgr.addOptionDef(opt_def2, "space-foo"));
-    ASSERT_NO_THROW(cfgmgr.addOptionDef(opt_def3, "space-bar"));
+    CfgOptionDefPtr cfg_option_def =
+        CfgMgr::instance().getStagingCfg()->getCfgOptionDef();
+    ASSERT_NO_THROW(cfg_option_def->add(opt_def, "space-foobar"));
+    ASSERT_NO_THROW(cfg_option_def->add(opt_def2, "space-foo"));
+    ASSERT_NO_THROW(cfg_option_def->add(opt_def3, "space-bar"));
+    CfgMgr::instance().commit();
 
     // Create the buffer holding the structure of options.
     const char raw_data[] = {

src/bin/dhcp6/tests/kea_controller_unittest.cc

@@ -18,6 +18,7 @@
 #include <dhcp/dhcp6.h>
 #include <dhcp6/ctrl_dhcp6_srv.h>
 #include <dhcpsrv/cfgmgr.h>
+#include <log/logger_support.h>
 
 #include <boost/scoped_ptr.hpp>
 #include <gtest/gtest.h>
@@ -52,6 +53,7 @@ public:
     }
 
     ~JSONFileBackendTest() {
+        isc::log::setDefaultLoggingOutput();
         static_cast<void>(unlink(TEST_FILE));
     };
 

src/lib/dhcp/option_definition.cc

@@ -92,6 +92,16 @@ OptionDefinition::OptionDefinition(const std::string& name,
       encapsulated_space_(encapsulated_space) {
 }
 
+bool
+OptionDefinition::equals(const OptionDefinition& other) const {
+    return (name_ == other.name_ &&
+            code_ == other.code_ &&
+            type_ == other.type_ &&
+            array_type_ == other.array_type_ &&
+            encapsulated_space_ == other.encapsulated_space_ &&
+            record_fields_ == other.record_fields_);
+}
+
 void
 OptionDefinition::addRecordField(const std::string& data_type_name) {
     OptionDataType data_type = OptionDataTypeUtil::getDataType(data_type_name);

src/lib/dhcp/option_definition.h

@@ -203,6 +203,35 @@ public:
                               const char* encapsulated_space);
 
 
+    /// @name Comparison functions and operators.
+    ///
+    //@{
+    /// @brief Check if option definition is equal to other.
+    ///
+    /// @param other Option definition to compare to.
+    ///
+    /// @return true if two option definitions are equal, false otherwise.
+    bool equals(const OptionDefinition& other) const;
+
+    /// @brief Equality operator.
+    ///
+    /// @param other Option definition to compare to.
+    ///
+    /// @return true if two option definitions are equal, false otherwise.
+    bool operator==(const OptionDefinition& other) const {
+        return (equals(other));
+    }
+
+    /// @brief Inequality operator.
+    ///
+    /// @param other Option definition to compare to.
+    ///
+    /// @return true if option definitions are not equal, false otherwise.
+    bool operator!=(const OptionDefinition& other) const {
+        return (!equals(other));
+    }
+    //@}
+
     /// @brief Adds data field to the record.
     ///
     /// @param data_type_name name of the data type for the field.
@@ -247,7 +276,9 @@ public:
     /// @brief Return list of record fields.
     ///
     /// @return list of record fields.
-    const RecordFieldsCollection& getRecordFields() const { return (record_fields_); }
+    const RecordFieldsCollection& getRecordFields() const {
+        return (record_fields_);
+    }
 
     /// @brief Return option data type.
     ///

src/lib/dhcp/option_space.h

@@ -21,6 +21,9 @@
 #include <stdint.h>
 #include <string>
 
+#define DHCP4_OPTION_SPACE "dhcp4"
+#define DHCP6_OPTION_SPACE "dhcp6"
+
 namespace isc {
 namespace dhcp {
 

src/lib/dhcp/tests/option_definition_unittest.cc

@@ -47,6 +47,7 @@ class OptionDefinitionTest : public ::testing::Test {
 public:
     // @brief Constructor.
     OptionDefinitionTest() { }
+
 };
 
 // The purpose of this test is to verify that OptionDefinition
@@ -115,6 +116,99 @@ TEST_F(OptionDefinitionTest, constructor) {
     );
 }
 
+// This test checks that the copy constructor works properly.
+TEST_F(OptionDefinitionTest, copyConstructor) {
+    OptionDefinition opt_def("option-foo", 27, "record", true);
+    ASSERT_NO_THROW(opt_def.addRecordField("uint16"));
+    ASSERT_NO_THROW(opt_def.addRecordField("string"));
+
+    OptionDefinition opt_def_copy(opt_def);
+    EXPECT_EQ("option-foo", opt_def_copy.getName());
+    EXPECT_EQ(27, opt_def_copy.getCode());
+    EXPECT_TRUE(opt_def_copy.getArrayType());
+    EXPECT_TRUE(opt_def_copy.getEncapsulatedSpace().empty());
+    ASSERT_EQ(OPT_RECORD_TYPE, opt_def_copy.getType());
+    const OptionDefinition::RecordFieldsCollection fields =
+        opt_def_copy.getRecordFields();
+    ASSERT_EQ(2, fields.size());
+    EXPECT_EQ(OPT_UINT16_TYPE, fields[0]);
+    EXPECT_EQ(OPT_STRING_TYPE, fields[1]);
+
+    // Let's make another test to check if encapsulated option space is
+    // copied properly.
+    OptionDefinition opt_def2("option-bar", 30, "uint32", "isc");
+    OptionDefinition opt_def_copy2(opt_def2);
+    EXPECT_EQ("option-bar", opt_def_copy2.getName());
+    EXPECT_EQ(30, opt_def_copy2.getCode());
+    EXPECT_FALSE(opt_def_copy2.getArrayType());
+    EXPECT_EQ(OPT_UINT32_TYPE, opt_def_copy2.getType());
+    EXPECT_EQ("isc", opt_def_copy2.getEncapsulatedSpace());
+}
+
+// This test checks that two option definitions may be compared fot equality.
+TEST_F(OptionDefinitionTest, equality) {
+    // Equal definitions.
+    EXPECT_TRUE(OptionDefinition("option-foo", 5, "uint16", false)
+                == OptionDefinition("option-foo", 5, "uint16", false));
+    EXPECT_FALSE(OptionDefinition("option-foo", 5, "uint16", false)
+                 != OptionDefinition("option-foo", 5, "uint16", false));
+
+    // Differ by name.
+    EXPECT_FALSE(OptionDefinition("option-foo", 5, "uint16", false)
+                 == OptionDefinition("option-foobar", 5, "uint16", false));
+    EXPECT_FALSE(OptionDefinition("option-bar", 5, "uint16", false)
+                == OptionDefinition("option-foo", 5, "uint16", false));
+    EXPECT_TRUE(OptionDefinition("option-bar", 5, "uint16", false)
+                 != OptionDefinition("option-foo", 5, "uint16", false));
+
+    // Differ by option code.
+    EXPECT_FALSE(OptionDefinition("option-foo", 5, "uint16", false)
+                == OptionDefinition("option-foo", 6, "uint16", false));
+    EXPECT_TRUE(OptionDefinition("option-foo", 5, "uint16", false)
+                 != OptionDefinition("option-foo", 6, "uint16", false));
+
+    // Differ by type of the data.
+    EXPECT_FALSE(OptionDefinition("option-foo", 5, "uint16", false)
+                == OptionDefinition("option-foo", 5, "uint32", false));
+    EXPECT_TRUE(OptionDefinition("option-foo", 5, "uint16", false)
+                 != OptionDefinition("option-foo", 5, "uint32", false));
+
+    // Differ by array-type property.
+    EXPECT_FALSE(OptionDefinition("option-foo", 5, "uint16", false)
+                == OptionDefinition("option-foo", 5, "uint16", true));
+    EXPECT_TRUE(OptionDefinition("option-foo", 5, "uint16", false)
+                 != OptionDefinition("option-foo", 5, "uint16", true));
+
+    // Differ by record fields.
+    OptionDefinition def1("option-foo", 5, "record");
+    OptionDefinition def2("option-foo", 5, "record");
+
+    // There are no record fields specified yet, so initially they have
+    // to be equal.
+    ASSERT_TRUE(def1 == def2);
+    ASSERT_FALSE(def1 != def2);
+
+    // Add some record fields.
+    ASSERT_NO_THROW(def1.addRecordField("uint16"));
+    ASSERT_NO_THROW(def2.addRecordField("uint16"));
+
+    // Definitions should still remain equal.
+    ASSERT_TRUE(def1 == def2);
+    ASSERT_FALSE(def1 != def2);
+
+    // Add additional record field to one of the definitions but not the
+    // other. They should now be unequal.
+    ASSERT_NO_THROW(def1.addRecordField("string"));
+    ASSERT_FALSE(def1 == def2);
+    ASSERT_TRUE(def1 != def2);
+
+    // Add the same record field to the other definition. They should now
+    // be equal again.
+    ASSERT_NO_THROW(def2.addRecordField("string"));
+    EXPECT_TRUE(def1 == def2);
+    EXPECT_FALSE(def1 != def2);
+}
+
 // The purpose of this test is to verify that various data fields
 // can be specified for an option definition when this definition
 // is marked as 'record' and that fields can't be added if option

src/lib/dhcpsrv/Makefile.am

@@ -46,6 +46,7 @@ libkea_dhcpsrv_la_SOURCES += addr_utilities.cc addr_utilities.h
 libkea_dhcpsrv_la_SOURCES += alloc_engine.cc alloc_engine.h
 libkea_dhcpsrv_la_SOURCES += callout_handle_store.h
 libkea_dhcpsrv_la_SOURCES += cfg_iface.cc cfg_iface.h
+libkea_dhcpsrv_la_SOURCES += cfg_option_def.cc cfg_option_def.h
 libkea_dhcpsrv_la_SOURCES += cfgmgr.cc cfgmgr.h
 libkea_dhcpsrv_la_SOURCES += csv_lease_file4.cc csv_lease_file4.h
 libkea_dhcpsrv_la_SOURCES += csv_lease_file6.cc csv_lease_file6.h

src/lib/dhcpsrv/cfg_option_def.cc

@@ -0,0 +1,142 @@
+// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <dhcp/libdhcp++.h>
+#include <dhcp/option_definition.h>
+#include <dhcp/option_space.h>
+#include <dhcpsrv/cfg_option_def.h>
+
+namespace isc {
+namespace dhcp {
+
+void
+CfgOptionDef::copyTo(CfgOptionDef& new_config) const {
+    const std::list<std::string>& names =
+        option_definitions_.getOptionSpaceNames();
+    for (std::list<std::string>::const_iterator name = names.begin();
+         name != names.end(); ++name) {
+        OptionDefContainerPtr defs = getAll(*name);
+        for (OptionDefContainer::const_iterator def = defs->begin();
+             def != defs->end(); ++def) {
+            OptionDefinitionPtr new_def =
+                OptionDefinitionPtr(new OptionDefinition(**def));
+            new_config.add(new_def, *name);
+        }
+    }
+}
+
+bool
+CfgOptionDef::equals(const CfgOptionDef& other) const {
+    // Get our option space names.
+    const std::list<std::string>& names = option_definitions_.getOptionSpaceNames();
+    // Get option space names held by the other object.
+    const std::list<std::string>&
+        other_names = other.option_definitions_.getOptionSpaceNames();
+    // Compareg that sizes are the same. If they hold different number of
+    // option space names the objects are not equal.
+    if (names.size() != other_names.size()) {
+        return (false);
+    }
+    // Iterate over all option space names and get the definitions for each
+    // of them.
+    for (std::list<std::string>::const_iterator name = names.begin();
+         name != names.end(); ++name) {
+        // Get all definitions.
+        OptionDefContainerPtr defs = getAll(*name);
+        OptionDefContainerPtr other_defs = other.getAll(*name);
+        // Compare sizes. If they hold different number of definitions,
+        // they are unequal.
+        if (defs->size() != defs->size()) {
+            return (false);
+        }
+        // For each option definition, try to find one in the other object.
+        for (OptionDefContainer::const_iterator def = defs->begin();
+             def != defs->end(); ++def) {
+            OptionDefinitionPtr
+                other_def = other.get(*name, (*def)->getCode());
+            // Actually compare them.
+            if (!other_def || (*other_def != **def)) {
+                return (false);
+            }
+        }
+    }
+
+    // All checks passed.
+    return (true);
+}
+
+void
+CfgOptionDef::add(const OptionDefinitionPtr& def,
+                  const std::string& option_space) {
+    if (!OptionSpace::validateName(option_space)) {
+        isc_throw(BadValue, "invalid option space name '"
+                  << option_space << "'");
+
+    // Option definition being added must be a valid pointer.
+    } else if (!def) {
+        isc_throw(MalformedOptionDefinition,
+                  "option definition must not be NULL");
+    // Must not duplicate an option definition.
+    } else if (get(option_space, def->getCode())) {
+        isc_throw(DuplicateOptionDefinition, "option definition with code '"
+                  << def->getCode() << "' already exists in option"
+                  " space '" << option_space << "'");
+
+    // Must not override standard option definition.
+    } else if (((option_space == DHCP4_OPTION_SPACE) &&
+                LibDHCP::isStandardOption(Option::V4, def->getCode()) &&
+                LibDHCP::getOptionDef(Option::V4, def->getCode())) ||
+               ((option_space == DHCP6_OPTION_SPACE) &&
+                LibDHCP::isStandardOption(Option::V6, def->getCode()) &&
+                LibDHCP::getOptionDef(Option::V6, def->getCode()))) {
+        isc_throw(BadValue, "unable to override definition of option '"
+                  << def->getCode() << "' in standard option space '"
+                  << option_space << "'");
+    }
+    // Add the definition.
+    option_definitions_.addItem(def, option_space);
+}
+
+OptionDefContainerPtr
+CfgOptionDef::getAll(const std::string& option_space) const {
+    /// @todo Does option space require any validation here?
+    return (option_definitions_.getItems(option_space));
+}
+
+OptionDefinitionPtr
+CfgOptionDef::get(const std::string& option_space,
+                  const uint16_t option_code) const {
+    // Get the pointer to collection of the option definitions that belong
+    // to the particular option space.
+    OptionDefContainerPtr defs = getAll(option_space);
+    // If there are any option definitions for this option space, get the
+    // one that has the specified option code.
+    if (defs && !defs->empty()) {
+        const OptionDefContainerTypeIndex& idx = defs->get<1>();
+        const OptionDefContainerTypeRange& range = idx.equal_range(option_code);
+        // If there is more than one definition matching the option code,
+        // return the first one. In fact, it shouldn't happen that we have
+        // more than one because we check for duplicates when we add them.
+        if (std::distance(range.first, range.second) > 0) {
+            return (*range.first);
+        }
+    }
+    // Nothing found. Return NULL pointer.
+    return (OptionDefinitionPtr());
+}
+
+
+
+} // end of namespace isc::dhcp
+} // end of namespace isc

src/lib/dhcpsrv/cfg_option_def.h

@@ -0,0 +1,142 @@
+// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef CFG_OPTION_DEF_H
+#define CFG_OPTION_DEF_H
+
+#include <dhcp/option_definition.h>
+#include <dhcpsrv/option_space_container.h>
+#include <string>
+
+namespace isc {
+namespace dhcp {
+
+/// @brief Represents option definitions used by the DHCP server.
+///
+/// This class provides methods to add and retrieve option definitions
+/// specified by the administrator for the DHCP server. Option definitions
+/// specify formats of the options. This class doesn't hold information
+/// about the data being carried by the options.
+///
+/// Option definitions are grouped by option spaces. The option space is
+/// identified by the unique name which is specified as a string. The
+/// following names: "dhcp4" and "dhcp6" are reserved, though. They are
+/// names of option spaces used for standard top-level DHCPv4 and DHCPv6
+/// options respectively.
+class CfgOptionDef {
+public:
+
+    /// @brief Copies this configuration to a new configuration.
+    ///
+    /// This method copies the option definitions stores in the configuration
+    /// to an object passed as parameter. There are no shared objects or
+    /// pointers between the original object and a copy.
+    ///
+    /// @warning This function doesn't perform a cleanup of the @c new_config
+    /// before copying the new configuration into it. It is caller's
+    /// responsibility to make sure that this object doesn't contain any
+    /// garbage data.
+    ///
+    /// @param [out] new_config An object to which the configuration will be
+    /// copied.
+    void copyTo(CfgOptionDef& new_config) const;
+
+    /// @name Methods and operators used for comparing objects.
+    ///
+    //@{
+    /// @brief Check if configuration is equal to other configuration.
+    ///
+    /// @param other An object holding configuration to compare to.
+    ///
+    /// @return true if configurations are equal, false otherwise.
+    bool equals(const CfgOptionDef& other) const;
+
+    /// @brief Equality operator.
+    ///
+    /// @param other An object holding configuration to compare to.
+    ///
+    /// @return true if configurations are equal, false otherwise.
+    bool operator==(const CfgOptionDef& other) const {
+        return (equals(other));
+    }
+
+    /// @brief Inequality operator.
+    ///
+    /// @param other An object holding configuration to compare to.
+    ///
+    /// @return true if configurations are unequal, false otherwise.
+    bool operator!=(const CfgOptionDef& other) const {
+        return (!equals(other));
+    }
+
+    //@}
+
+    /// @brief Add new option definition.
+    ///
+    /// @param def option definition to be added.
+    /// @param option_space name of the option space to add definition to.
+    ///
+    /// @throw isc::dhcp::DuplicateOptionDefinition when the particular
+    /// option definition already exists.
+    /// @throw isc::dhcp::MalformedOptionDefinition when the pointer to
+    /// an option definition is NULL.
+    /// @throw isc::BadValue when the option space name is empty or
+    /// when trying to override the standard option (in dhcp4 or dhcp6
+    /// option space).
+    void add(const OptionDefinitionPtr& def, const std::string& option_space);
+
+    /// @brief Return option definitions for particular option space.
+    ///
+    /// @param option_space option space.
+    ///
+    /// @return Pointer to the collection of option definitions for
+    /// the particular option space. The option collection is empty
+    /// if no option exists for the option space specified.
+    OptionDefContainerPtr getAll(const std::string& option_space) const;
+
+    /// @brief Return option definition for a particular option space and code.
+    ///
+    /// @param option_space option space.
+    /// @param option_code option code.
+    ///
+    /// @return An option definition or NULL pointer if option definition
+    /// has not been found.
+    OptionDefinitionPtr get(const std::string& option_space,
+                            const uint16_t option_code) const;
+
+private:
+
+    /// @brief A collection of option definitions.
+    ///
+    /// The option definitions stored in this container can be accessed
+    /// using the option space name they belong to.
+    OptionSpaceContainer<OptionDefContainer, OptionDefinitionPtr,
+                         std::string> option_definitions_;
+
+};
+
+/// @name Pointers to the @c CfgOptionDef objects.
+//@{
+/// @brief Non-const pointer.
+typedef boost::shared_ptr<CfgOptionDef> CfgOptionDefPtr;
+
+/// @brief Const pointer.
+typedef boost::shared_ptr<const CfgOptionDef> ConstCfgOptionDefPtr;
+
+//@}
+
+}
+}
+
+#endif // CFG_OPTION_DEF_H

src/lib/dhcpsrv/cfgmgr.cc

@@ -61,72 +61,6 @@ CfgMgr::addOptionSpace6(const OptionSpacePtr& space) {
     spaces6_.insert(make_pair(space->getName(), space));
 }
 
-void
-CfgMgr::addOptionDef(const OptionDefinitionPtr& def,
-                     const std::string& option_space) {
-    // @todo we need better validation of the provided option space name here.
-    // This will be implemented when #2313 is merged.
-    if (option_space.empty()) {
-        isc_throw(BadValue, "option space name must not be empty");
-    } else if (!def) {
-        // Option definition must point to a valid object.
-        isc_throw(MalformedOptionDefinition, "option definition must not be NULL");
-
-    } else if (getOptionDef(option_space, def->getCode())) {
-        // Option definition must not be overriden.
-        isc_throw(DuplicateOptionDefinition, "option definition already added"
-                  << " to option space " << option_space);
-
-    // We must not override standard (assigned) option for which there is a
-    // definition in libdhcp++. The standard options belong to dhcp4 or dhcp6
-    // option space.
-    } else if ((option_space == "dhcp4" &&
-                LibDHCP::isStandardOption(Option::V4, def->getCode()) &&
-                LibDHCP::getOptionDef(Option::V4, def->getCode())) ||
-               (option_space == "dhcp6" &&
-                LibDHCP::isStandardOption(Option::V6, def->getCode()) &&
-                LibDHCP::getOptionDef(Option::V6, def->getCode()))) {
-        isc_throw(BadValue, "unable to override definition of option '"
-                  << def->getCode() << "' in standard option space '"
-                  << option_space << "'.");
-
-    }
-    // Actually add a new item.
-    option_def_spaces_.addItem(def, option_space);
-}
-
-OptionDefContainerPtr
-CfgMgr::getOptionDefs(const std::string& option_space) const {
-    // @todo Validate the option space once the #2313 is implemented.
-
-    return (option_def_spaces_.getItems(option_space));
-}
-
-OptionDefinitionPtr
-CfgMgr::getOptionDef(const std::string& option_space,
-                     const uint16_t option_code) const {
-    // @todo Validate the option space once the #2313 is implemented.
-
-    // Get a reference to option definitions for a particular option space.
-    OptionDefContainerPtr defs = getOptionDefs(option_space);
-    // If there are no matching option definitions then return the empty pointer.
-    if (!defs || defs->empty()) {
-        return (OptionDefinitionPtr());
-    }
-    // If there are some option definitions for a particular option space
-    // use an option code to get the one we want.
-    const OptionDefContainerTypeIndex& idx = defs->get<1>();
-    const OptionDefContainerTypeRange& range = idx.equal_range(option_code);
-    // If there is no definition that matches option code, return empty pointer.
-    if (std::distance(range.first, range.second) == 0) {
-        return (OptionDefinitionPtr());
-    }
-    // If there is more than one definition matching an option code, return
-    // the first one. This should not happen because we check for duplicates
-    // when addOptionDef is called.
-    return (*range.first);
-}
-
 Subnet6Ptr
 CfgMgr::getSubnet6(const std::string& iface,
                    const isc::dhcp::ClientClasses& classes) {
@@ -297,10 +231,6 @@ void CfgMgr::addSubnet4(const Subnet4Ptr& subnet) {
     subnets4_.push_back(subnet);
 }
 
-void CfgMgr::deleteOptionDefs() {
-    option_def_spaces_.clearItems();
-}
-
 void CfgMgr::deleteSubnets4() {
     LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE, DHCPSRV_CFGMGR_DELETE_SUBNET4);
     subnets4_.clear();

src/lib/dhcpsrv/cfgmgr.h

@@ -17,11 +17,9 @@
 
 #include <asiolink/io_address.h>
 #include <dhcp/option.h>
-#include <dhcp/option_definition.h>
 #include <dhcp/option_space.h>
 #include <dhcp/classify.h>
 #include <dhcpsrv/d2_client_mgr.h>
-#include <dhcpsrv/option_space_container.h>
 #include <dhcpsrv/pool.h>
 #include <dhcpsrv/subnet.h>
 #include <dhcpsrv/srv_config.h>
@@ -106,41 +104,6 @@ public:
     /// accessing it.
     static CfgMgr& instance();
 
-    /// @brief Add new option definition.
-    ///
-    /// @param def option definition to be added.
-    /// @param option_space name of the option space to add definition to.
-    ///
-    /// @throw isc::dhcp::DuplicateOptionDefinition when the particular
-    /// option definition already exists.
-    /// @throw isc::dhcp::MalformedOptionDefinition when the pointer to
-    /// an option definition is NULL.
-    /// @throw isc::BadValue when the option space name is empty or
-    /// when trying to override the standard option (in dhcp4 or dhcp6
-    /// option space).
-    void addOptionDef(const OptionDefinitionPtr& def,
-                      const std::string& option_space);
-
-    /// @brief Return option definitions for particular option space.
-    ///
-    /// @param option_space option space.
-    ///
-    /// @return pointer to the collection of option definitions for
-    /// the particular option space. The option collection is empty
-    /// if no option exists for the option space specified.
-    OptionDefContainerPtr
-    getOptionDefs(const std::string& option_space) const;
-
-    /// @brief Return option definition for a particular option space and code.
-    ///
-    /// @param option_space option space.
-    /// @param option_code option code.
-    ///
-    /// @return an option definition or NULL pointer if option definition
-    /// has not been found.
-    OptionDefinitionPtr getOptionDef(const std::string& option_space,
-                                     const uint16_t option_code) const;
-
     /// @brief Adds new DHCPv4 option space to the collection.
     ///
     /// @param space option space to be added.
@@ -235,9 +198,6 @@ public:
     /// @param subnet new subnet to be added.
     void addSubnet6(const Subnet6Ptr& subnet);
 
-    /// @brief Delete all option definitions.
-    void deleteOptionDefs();
-
     /// @todo: Add subnet6 removal routines. Currently it is not possible
     /// to remove subnets. The only case where subnet6 removal would be
     /// needed is a dynamic server reconfiguration - a use case that is not
@@ -555,13 +515,6 @@ private:
     /// @return true if the duplicate subnet exists.
     bool isDuplicate(const Subnet6& subnet) const;
 
-    /// @brief A collection of option definitions.
-    ///
-    /// A collection of option definitions that can be accessed
-    /// using option space name they belong to.
-    OptionSpaceContainer<OptionDefContainer,
-        OptionDefinitionPtr, std::string> option_def_spaces_;
-
     /// @brief Container for defined DHCPv6 option spaces.
     OptionSpaceCollection spaces6_;
 
@@ -581,8 +534,6 @@ private:
     ///
     /// This is a structure that will hold all configuration.
     /// @todo: migrate all other parameters to that structure.
-    /// @todo: maybe this should be a vector<Configuration>, so we could keep
-    ///        previous configurations and do a rollback if needed?
     SrvConfigPtr configuration_;
 
     /// @name Configuration List.

src/lib/dhcpsrv/dhcp_parsers.cc

@@ -43,7 +43,6 @@ ParserContext::ParserContext(Option::Universe universe):
     uint32_values_(new Uint32Storage()),
     string_values_(new StringStorage()),
     options_(new OptionStorage()),
-    option_defs_(new OptionDefStorage()),
     hooks_libraries_(),
     universe_(universe)
 {
@@ -54,7 +53,6 @@ ParserContext::ParserContext(const ParserContext& rhs):
     uint32_values_(),
     string_values_(),
     options_(),
-    option_defs_(),
     hooks_libraries_(),
     universe_(rhs.universe_)
 {
@@ -79,7 +77,6 @@ ParserContext::copyContext(const ParserContext& ctx) {
     copyContextPointer(ctx.uint32_values_, uint32_values_);
     copyContextPointer(ctx.string_values_, string_values_);
     copyContextPointer(ctx.options_, options_);
-    copyContextPointer(ctx.option_defs_, option_defs_);
     copyContextPointer(ctx.hooks_libraries_, hooks_libraries_);
     // Copy universe.
     universe_ = ctx.universe_;
@@ -446,7 +443,7 @@ OptionDataParser::createOption(ConstElementPtr option_data) {
         // options. They are expected to be in the global storage
         // already.
         OptionDefContainerPtr defs =
-            global_context_->option_defs_->getItems(space);
+            CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->getAll(space);
 
         // The getItems() should never return the NULL pointer. If there are
         // no option definitions for the particular option space a pointer
@@ -612,17 +609,11 @@ OptionDataListParser::commit() {
 
 // ******************************** OptionDefParser ****************************
 OptionDefParser::OptionDefParser(const std::string&,
-                                 OptionDefStoragePtr storage,
                                  ParserContextPtr global_context)
-    : storage_(storage),
-      boolean_values_(new BooleanStorage()),
+    : boolean_values_(new BooleanStorage()),
       string_values_(new StringStorage()),
       uint32_values_(new Uint32Storage()),
       global_context_(global_context) {
-    if (!storage_) {
-        isc_throw(isc::dhcp::DhcpConfigError, "parser logic error: "
-             << "options storage may not be NULL");
-    }
 }
 
 void
@@ -655,31 +646,20 @@ OptionDefParser::build(ConstElementPtr option_def) {
     // Create an instance of option definition.
     createOptionDef(option_def);
 
-    // Get all items we collected so far for the particular option space.
-    OptionDefContainerPtr defs = storage_->getItems(option_space_name_);
-
-    // Check if there are any items with option code the same as the
-    // one specified for the definition we are now creating.
-    const OptionDefContainerTypeIndex& idx = defs->get<1>();
-    const OptionDefContainerTypeRange& range =
-            idx.equal_range(option_definition_->getCode());
+    try {
+        CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->
+            add(option_definition_, option_space_name_);
 
-    // If there are any items with this option code already we need
-    // to issue an error because we don't allow duplicates for
-    // option definitions within an option space.
-    if (std::distance(range.first, range.second) > 0) {
-        isc_throw(DhcpConfigError, "duplicated option definition for"
-                  << " code '" << option_definition_->getCode() << "' ("
+    } catch (const std::exception& ex) {
+        // Append position if there is a failure.
+        isc_throw(DhcpConfigError, ex.what() << " ("
                   << option_def->getPosition() << ")");
     }
 }
 
 void
 OptionDefParser::commit() {
-    if (storage_ && option_definition_ &&
-        OptionSpace::validateName(option_space_name_)) {
-            storage_->addItem(option_definition_, option_space_name_);
-    }
+    // Do nothing.
 }
 
 void
@@ -775,20 +755,11 @@ OptionDefParser::createOptionDef(ConstElementPtr option_def_element) {
 // ******************************** OptionDefListParser ************************
 OptionDefListParser::OptionDefListParser(const std::string&,
                                          ParserContextPtr global_context)
-    : storage_(global_context->option_defs_),
-      global_context_(global_context) {
-    if (!storage_) {
-        isc_throw(isc::dhcp::DhcpConfigError, "parser logic error: "
-             << "storage may not be NULL");
-    }
+    : global_context_(global_context) {
 }
 
 void
 OptionDefListParser::build(ConstElementPtr option_def_list) {
-    // Clear existing items in the storage.
-    // We are going to replace all of them.
-    storage_->clearItems();
-
     if (!option_def_list) {
         isc_throw(DhcpConfigError, "parser error: a pointer to a list of"
                   << " option definitions is NULL ("
@@ -797,36 +768,8 @@ OptionDefListParser::build(ConstElementPtr option_def_list) {
 
     BOOST_FOREACH(ConstElementPtr option_def, option_def_list->listValue()) {
         boost::shared_ptr<OptionDefParser>
-            parser(new OptionDefParser("single-option-def", storage_,
-                                       global_context_));
+            parser(new OptionDefParser("single-option-def", global_context_));
         parser->build(option_def);
-        parser->commit();
-    }
-
-    CfgMgr& cfg_mgr = CfgMgr::instance();
-    cfg_mgr.deleteOptionDefs();
-
-    // We need to move option definitions from the temporary
-    // storage to the storage.
-    std::list<std::string> space_names = storage_->getOptionSpaceNames();
-
-    BOOST_FOREACH(std::string space_name, space_names) {
-        BOOST_FOREACH(OptionDefinitionPtr def,
-                    *(storage_->getItems(space_name))) {
-            // All option definitions should be initialized to non-NULL
-            // values. The validation is expected to be made by the
-            // OptionDefParser when creating an option definition.
-            assert(def);
-            // The Config Manager may thrown an exception if the duplicated
-            // definition is being added. Catch the exceptions here to and
-            // append the position in the config.
-            try {
-                cfg_mgr.addOptionDef(def, space_name);
-            } catch (const std::exception& ex) {
-                isc_throw(DhcpConfigError, ex.what() << " ("
-                          << option_def_list->getPosition() << ")");
-            }
-        }
     }
 }
 
@@ -1099,8 +1042,8 @@ SubnetConfigParser::appendSubOptions(const std::string& option_space,
             return;
         }
     } else {
-        const OptionDefContainerPtr defs =
-                global_context_->option_defs_->getItems(option_space);
+        OptionDefContainerPtr defs = CfgMgr::instance().getStagingCfg()
+            ->getCfgOptionDef()->getAll(option_space);
 
         const OptionDefContainerTypeIndex& idx = defs->get<1>();
         const OptionDefContainerTypeRange& range =

src/lib/dhcpsrv/dhcp_parsers.h

@@ -216,9 +216,6 @@ public:
     /// @brief Storage for options.
     OptionStoragePtr options_;
 
-    /// @brief Storage for option definitions.
-    OptionDefStoragePtr option_defs_;
-
     /// @brief Hooks libraries pointer.
     ///
     /// The hooks libraries information is a vector of strings, each containing
@@ -683,13 +680,9 @@ public:
     ///
     /// @param dummy first argument is ignored, all Parser constructors
     /// accept string as first argument.
-    /// @param storage is the definition storage in which to store the parsed
-    /// definition upon "commit".
     /// @param global_context is a pointer to the global context which
     /// stores global scope parameters, options, option defintions.
-    /// @throw isc::dhcp::DhcpConfigError if storage is null.
-    OptionDefParser(const std::string& dummy, OptionDefStoragePtr storage,
-                    ParserContextPtr global_context);
+    OptionDefParser(const std::string& dummy, ParserContextPtr global_context);
 
     /// @brief Parses an entry that describes single option definition.
     ///
@@ -714,10 +707,6 @@ private:
     /// Name of the space the option definition belongs to.
     std::string option_space_name_;
 
-    /// Pointer to a storage where the option definition will be
-    /// added when \ref commit is called.
-    OptionDefStoragePtr storage_;
-
     /// Storage for boolean values.
     BooleanStoragePtr boolean_values_;
 
@@ -746,7 +735,6 @@ public:
     /// accept string as first argument.
     /// @param global_context is a pointer to the global context which
     /// stores global scope parameters, options, option defintions.
-    /// @throw isc::dhcp::DhcpConfigError if storage is null.
     OptionDefListParser(const std::string& dummy,
                         ParserContextPtr global_context);
 
@@ -767,9 +755,6 @@ public:
     /// added to the Configuration Manager in the @c build method.
     void commit();
 
-    /// @brief storage for option definitions.
-    OptionDefStoragePtr storage_;
-
     /// Parsing context which contains global values, options and option
     /// definitions.
     ParserContextPtr global_context_;

src/lib/dhcpsrv/logging.h

@@ -45,7 +45,7 @@ namespace dhcp {
 /// The data structures don't have to originate from JSON. JSON is just a
 /// convenient presentation syntax.
 ///
-/// This class uses Configuration structure to store logging configuration.
+/// This class uses @c SrvConfig object to store logging configuration.
 class LogConfigParser {
 public:
 

src/lib/dhcpsrv/option_space_container.h

@@ -74,7 +74,7 @@ public:
     /// @todo This function is likely to be removed once
     /// we create a structore of OptionSpaces defined
     /// through the configuration manager.
-    std::list<Selector> getOptionSpaceNames() {
+    std::list<Selector> getOptionSpaceNames() const {
         std::list<Selector> names;
         for (typename OptionSpaceMap::const_iterator space =
                  option_space_map_.begin();

src/lib/dhcpsrv/srv_config.cc

@@ -25,11 +25,11 @@ namespace isc {
 namespace dhcp {
 
 SrvConfig::SrvConfig()
-    : sequence_(0) {
+    : sequence_(0), cfg_option_def_(new CfgOptionDef()) {
 }
 
 SrvConfig::SrvConfig(uint32_t sequence)
-    : sequence_(sequence) {
+    : sequence_(sequence), cfg_option_def_(new CfgOptionDef()) {
 }
 
 std::string
@@ -88,6 +88,8 @@ SrvConfig::copy(SrvConfig& new_config) const {
     }
     // Replace interface configuration.
     new_config.setCfgIface(cfg_iface_);
+    // Replace option definitions.
+    cfg_option_def_->copyTo(*new_config.cfg_option_def_);
 }
 
 void
@@ -129,7 +131,8 @@ SrvConfig::equals(const SrvConfig& other) const {
         }
     }
     // Logging information is equal between objects, so check other values.
-    return (cfg_iface_ == other.cfg_iface_);
+    return ((cfg_iface_ == other.cfg_iface_) &&
+            (*cfg_option_def_ == *other.cfg_option_def_));
 }
 
 }

src/lib/dhcpsrv/srv_config.h

@@ -16,6 +16,7 @@
 #define DHCPSRV_CONFIG_H
 
 #include <dhcpsrv/cfg_iface.h>
+#include <dhcpsrv/cfg_option_def.h>
 #include <dhcpsrv/logging_info.h>
 #include <boost/shared_ptr.hpp>
 #include <vector>
@@ -104,6 +105,13 @@ public:
     /// @return true if sequence numbers are equal.
     bool sequenceEquals(const SrvConfig& other);
 
+    /// @name Modifiers and accesors for the configuration objects.
+    ///
+    /// @warning References to the objects returned by accessors are only
+    /// valid during the lifetime of the @c SrvConfig object which
+    /// returned them.
+    ///
+    //@{
     /// @brief Returns logging specific configuration.
     const LoggingInfoStorage& getLoggingInfo() const {
         return (logging_info_);
@@ -133,6 +141,30 @@ public:
         cfg_iface_ = cfg_iface;
     }
 
+    /// @brief Return pointer to non-const object representing user-defined
+    /// option definitions.
+    ///
+    /// This function returns a pointer to the object which represents the
+    /// user defined option definitions grouped by option space name.
+    ///
+    /// @return Pointer to an object holding option definitions.
+    CfgOptionDefPtr getCfgOptionDef() {
+        return (cfg_option_def_);
+    }
+
+    /// @brief Returns pointer to the const object representing user-defined
+    /// option definitions.
+    ///
+    /// This function returns a pointer to the object which represents the
+    /// user defined option definitions grouped by option space name.
+    ///
+    /// @return Pointer to an object holding option definitions.
+    ConstCfgOptionDefPtr getCfgOptionDef() const {
+        return (cfg_option_def_);
+    }
+
+    //@}
+
     /// @brief Copies the currnet configuration to a new configuration.
     ///
     /// This method copies the parameters stored in the configuration to
@@ -212,6 +244,12 @@ private:
     /// queries.
     CfgIface cfg_iface_;
 
+    /// @brief Pointer to option definitions configuration.
+    ///
+    /// This object holds the user-defined option definitions grouped
+    /// by option space name.
+    CfgOptionDefPtr cfg_option_def_;
+
 };
 
 /// @name Pointers to the @c SrvConfig object.

src/lib/dhcpsrv/tests/Makefile.am

@@ -55,6 +55,8 @@ libdhcpsrv_unittests_SOURCES  = run_unittests.cc
 libdhcpsrv_unittests_SOURCES += addr_utilities_unittest.cc
 libdhcpsrv_unittests_SOURCES += alloc_engine_unittest.cc
 libdhcpsrv_unittests_SOURCES += callout_handle_store_unittest.cc
+libdhcpsrv_unittests_SOURCES += cfg_iface_unittest.cc
+libdhcpsrv_unittests_SOURCES += cfg_option_def_unittest.cc
 libdhcpsrv_unittests_SOURCES += cfgmgr_unittest.cc
 libdhcpsrv_unittests_SOURCES += csv_lease_file4_unittest.cc
 libdhcpsrv_unittests_SOURCES += csv_lease_file6_unittest.cc
@@ -62,7 +64,6 @@ libdhcpsrv_unittests_SOURCES += d2_client_unittest.cc
 libdhcpsrv_unittests_SOURCES += d2_udp_unittest.cc
 libdhcpsrv_unittests_SOURCES += daemon_unittest.cc
 libdhcpsrv_unittests_SOURCES += dbaccess_parser_unittest.cc
-libdhcpsrv_unittests_SOURCES += cfg_iface_unittest.cc
 libdhcpsrv_unittests_SOURCES += lease_file_io.cc lease_file_io.h
 libdhcpsrv_unittests_SOURCES += lease_unittest.cc
 libdhcpsrv_unittests_SOURCES += lease_mgr_factory_unittest.cc

src/lib/dhcpsrv/tests/cfg_option_def_unittest.cc

@@ -0,0 +1,247 @@
+// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <config.h>
+#include <dhcp/dhcp4.h>
+#include <dhcp/dhcp6.h>
+#include <dhcp/option_space.h>
+#include <dhcpsrv/cfg_option_def.h>
+#include <gtest/gtest.h>
+
+using namespace isc;
+using namespace isc::dhcp;
+
+namespace {
+
+// This test checks that two option definition configurations can be
+// compared for equality.
+TEST(CfgOptionDefTest, equal) {
+    CfgOptionDef cfg1;
+    CfgOptionDef cfg2;
+
+    // Default objects must be equal.
+    ASSERT_TRUE(cfg1 == cfg2);
+    ASSERT_FALSE(cfg1 != cfg2);
+
+    // Let's add the same option but to two different option spaces.
+    cfg1.add(OptionDefinitionPtr(new OptionDefinition("option-foo",
+                                                      5, "uint16")), "isc");
+    cfg2.add(OptionDefinitionPtr(new OptionDefinition("option-foo",
+                                                      5, "uint16")), "dns");
+    // Configurations must be unequal.
+    ASSERT_FALSE(cfg1 == cfg2);
+    ASSERT_TRUE(cfg1 != cfg2);
+
+    // Now, let's add them again but to original option spaces. Both objects
+    // should now contain the same options under two option spaces. The
+    // order should not matter so configurations should be equal.
+    cfg1.add(OptionDefinitionPtr(new OptionDefinition("option-foo",
+                                                      5, "uint16")), "dns");
+    cfg2.add(OptionDefinitionPtr(new OptionDefinition("option-foo",
+                                                      5, "uint16")), "isc");
+
+    EXPECT_TRUE(cfg1 == cfg2);
+    EXPECT_FALSE(cfg1 != cfg2);
+
+}
+
+// This test verifies that multiple option definitions can be added
+// under different option spaces.
+TEST(CfgOptionDefTest, getAll) {
+    CfgOptionDef cfg;
+
+    // Create a set of option definitions with codes between 100 and 109.
+    for (uint16_t code = 100; code < 110; ++code) {
+        std::ostringstream option_name;
+        // Option name is unique, e.g. option-100, option-101 etc.
+        option_name << "option-" << code;
+        OptionDefinitionPtr def(new OptionDefinition(option_name.str(), code,
+                                                     "uint16"));
+        // Add option definition to "isc" option space.
+        // Option codes are not duplicated so expect no error
+        // when adding them.
+        ASSERT_NO_THROW(cfg.add(def, "isc"));
+    }
+
+    // Create a set of option definitions with codes between 105 and 114 and
+    // add them to the different option space.
+    for (uint16_t code = 105; code < 115; ++code) {
+        std::ostringstream option_name;
+        option_name << "option-" << code;
+        OptionDefinitionPtr def(new OptionDefinition(option_name.str(), code,
+                                                     "uint16"));
+        ASSERT_NO_THROW(cfg.add(def, "abcde"));
+    }
+
+    // Sanity check that all 10 option definitions are there.
+    OptionDefContainerPtr option_defs1 = cfg.getAll("isc");
+    ASSERT_TRUE(option_defs1);
+    ASSERT_EQ(10, option_defs1->size());
+
+    // Iterate over all option definitions and check that they have
+    // valid codes. Also, their order should be the same as they
+    // were added (codes 100-109).
+    uint16_t code = 100;
+    for (OptionDefContainer::const_iterator it = option_defs1->begin();
+         it != option_defs1->end(); ++it, ++code) {
+        OptionDefinitionPtr def(*it);
+        ASSERT_TRUE(def);
+        EXPECT_EQ(code, def->getCode());
+    }
+
+    // Sanity check that all 10 option definitions are there.
+    OptionDefContainerPtr option_defs2 = cfg.getAll("abcde");
+    ASSERT_TRUE(option_defs2);
+    ASSERT_EQ(10, option_defs2->size());
+
+    // Check that the option codes are valid.
+    code = 105;
+    for (OptionDefContainer::const_iterator it = option_defs2->begin();
+         it != option_defs2->end(); ++it, ++code) {
+        OptionDefinitionPtr def(*it);
+        ASSERT_TRUE(def);
+        EXPECT_EQ(code, def->getCode());
+    }
+
+    // Let's make one more check that the empty set is returned when
+    // invalid option space is used.
+    OptionDefContainerPtr option_defs3 = cfg.getAll("non-existing");
+    ASSERT_TRUE(option_defs3);
+    EXPECT_TRUE(option_defs3->empty());
+}
+
+// This test verifies that single option definition is correctly
+// returned with getOptionDef function.
+TEST(CfgOptionDefTest, get) {
+    CfgOptionDef cfg;
+    // Create a set of option definitions with codes between 100 and 109.
+    for (uint16_t code = 100; code < 110; ++code) {
+        std::ostringstream option_name;
+        // Option name is unique, e.g. option-100, option-101 etc.
+        option_name << "option-" << code;
+        OptionDefinitionPtr def(new OptionDefinition(option_name.str(), code,
+                                                     "uint16"));
+        // Add option definition to "isc" option space.
+        // Option codes are not duplicated so expect no error
+        // when adding them.
+        ASSERT_NO_THROW(cfg.add(def, "isc"));
+    }
+
+    // Create a set of option definitions with codes between 105 and 114 and
+    // add them to the different option space.
+    for (uint16_t code = 105; code < 115; ++code) {
+        std::ostringstream option_name;
+        option_name << "option-other-" << code;
+        OptionDefinitionPtr def(new OptionDefinition(option_name.str(), code,
+                                                     "uint16"));
+        ASSERT_NO_THROW(cfg.add(def, "abcde"));
+    }
+
+    // Try to get option definitions one by one using all codes
+    // that we expect to be there.
+    for (uint16_t code = 100; code < 110; ++code) {
+        OptionDefinitionPtr def = cfg.get("isc", code);
+        ASSERT_TRUE(def);
+        // Check that the option name is in the format of 'option-[code]'.
+        // That way we make sure that the options that have the same codes
+        // within different option spaces are different.
+        std::ostringstream option_name;
+        option_name << "option-" << code;
+        EXPECT_EQ(option_name.str(), def->getName());
+        EXPECT_EQ(code, def->getCode());
+    }
+
+    // Check that the option codes are valid.
+    for (uint16_t code = 105; code < 115; ++code) {
+        OptionDefinitionPtr def = cfg.get("abcde", code);
+        ASSERT_TRUE(def);
+        // Check that the option name is in the format of 'option-other-[code]'.
+        // That way we make sure that the options that have the same codes
+        // within different option spaces are different.
+        std::ostringstream option_name;
+        option_name << "option-other-" << code;
+        EXPECT_EQ(option_name.str(), def->getName());
+
+        EXPECT_EQ(code, def->getCode());
+    }
+
+    // Check that an option definition can be added to the standard
+    // (dhcp4 and dhcp6) option spaces when the option code is not
+    // reserved by the standard option.
+    OptionDefinitionPtr def6(new OptionDefinition("option-foo", 79, "uint16"));
+    EXPECT_NO_THROW(cfg.add(def6, DHCP6_OPTION_SPACE));
+
+    OptionDefinitionPtr def4(new OptionDefinition("option-foo", 222, "uint16"));
+    EXPECT_NO_THROW(cfg.add(def4, DHCP4_OPTION_SPACE));
+
+    // Try to query the option definition from an non-existing
+    // option space and expect NULL pointer.
+    OptionDefinitionPtr def = cfg.get("non-existing", 56);
+    EXPECT_FALSE(def);
+
+    // Try to get the non-existing option definition from an
+    // existing option space.
+    EXPECT_FALSE(cfg.get("isc", 56));
+}
+
+// This test verifies that it is not allowed to override a definition of the
+// standard option which has its definition defined in libdhcp++, but it is
+// allowed to create a definition for the standard option which doesn't have
+// its definition in libdhcp++.
+TEST(CfgOptionDefTest, overrideStdOptionDef) {
+    CfgOptionDef cfg;
+    OptionDefinitionPtr def;
+    // There is a definition for routers option in libdhcp++, so an attempt
+    // to add (override) another definition for this option should fail.
+    def.reset(new OptionDefinition("routers", DHO_ROUTERS, "uint32"));
+    EXPECT_THROW(cfg.add(def, DHCP4_OPTION_SPACE), isc::BadValue);
+
+    /// @todo There is no definition for the NIS Server Addr option in
+    /// libdhcp++. Once it is implemented it should be not allowed to
+    /// add a custom definition for it. At the moment, it should be ok
+    /// to add a definition for this option (using configuration mechanism)
+    /// because we haven't implemented the one in libdhcp++.
+    def.reset(new OptionDefinition("nis-server-addr", 65, "uint16"));
+    EXPECT_NO_THROW(cfg.add(def, DHCP4_OPTION_SPACE));
+
+    // It is not allowed to override the definition of the option which
+    // has its definition in the libdhcp++.
+    def.reset(new OptionDefinition("sntp-servers", D6O_SNTP_SERVERS,
+                                   "ipv4-address"));
+    EXPECT_THROW(cfg.add(def, DHCP6_OPTION_SPACE), isc::BadValue);
+    // There is no definition for option 59 in libdhcp++ yet, so it should
+    // be possible provide a custom definition.
+    def.reset(new OptionDefinition("bootfile-url", 59, "uint32"));
+    EXPECT_NO_THROW(cfg.add(def, DHCP6_OPTION_SPACE));
+}
+
+// This test verifies that the function that adds new option definition
+// throws exceptions when arguments are invalid.
+TEST(CfgOptionDefTest, addNegative) {
+    CfgOptionDef cfg;
+
+    OptionDefinitionPtr def(new OptionDefinition("option-foo", 1000, "uint16"));
+
+    // Try empty option space name.
+    ASSERT_THROW(cfg.add(def, ""), isc::BadValue);
+    // Try NULL option definition.
+    ASSERT_THROW(cfg.add(OptionDefinitionPtr(), "isc"),
+                 isc::dhcp::MalformedOptionDefinition);
+    // Try adding option definition twice and make sure that it
+    // fails on the second attempt.
+    ASSERT_NO_THROW(cfg.add(def, "isc"));
+    EXPECT_THROW(cfg.add(def, "isc"), DuplicateOptionDefinition);
+}
+
+}

src/lib/dhcpsrv/tests/cfgmgr_unittest.cc

@@ -284,7 +284,6 @@ public:
         CfgMgr::instance().setVerbose(false);
         CfgMgr::instance().deleteSubnets4();
         CfgMgr::instance().deleteSubnets6();
-        CfgMgr::instance().deleteOptionDefs();
         CfgMgr::instance().clear();
     }
 
@@ -305,196 +304,6 @@ TEST_F(CfgMgrTest, configuration) {
     EXPECT_TRUE(configuration->getLoggingInfo().empty());
 }
 
-// This test verifies that multiple option definitions can be added
-// under different option spaces.
-TEST_F(CfgMgrTest, getOptionDefs) {
-    CfgMgr& cfg_mgr = CfgMgr::instance();
-    // Create a set of option definitions with codes between 100 and 109.
-    for (uint16_t code = 100; code < 110; ++code) {
-        std::ostringstream option_name;
-        // Option name is unique, e.g. option-100, option-101 etc.
-        option_name << "option-" << code;
-        OptionDefinitionPtr def(new OptionDefinition(option_name.str(), code,
-                                                     "uint16"));
-        // Add option definition to "isc" option space.
-        // Option codes are not duplicated so expect no error
-        // when adding them.
-        ASSERT_NO_THROW(cfg_mgr.addOptionDef(def, "isc"));
-    }
-
-    // Create a set of option definitions with codes between 105 and 114 and
-    // add them to the different option space.
-    for (uint16_t code = 105; code < 115; ++code) {
-        std::ostringstream option_name;
-        option_name << "option-" << code;
-        OptionDefinitionPtr def(new OptionDefinition(option_name.str(), code,
-                                                     "uint16"));
-        ASSERT_NO_THROW(cfg_mgr.addOptionDef(def, "abcde"));
-    }
-
-    // Sanity check that all 10 option definitions are there.
-    OptionDefContainerPtr option_defs1 = cfg_mgr.getOptionDefs("isc");
-    ASSERT_TRUE(option_defs1);
-    ASSERT_EQ(10, option_defs1->size());
-
-    // Iterate over all option definitions and check that they have
-    // valid codes. Also, their order should be the same as they
-    // were added (codes 100-109).
-    uint16_t code = 100;
-    for (OptionDefContainer::const_iterator it = option_defs1->begin();
-         it != option_defs1->end(); ++it, ++code) {
-        OptionDefinitionPtr def(*it);
-        ASSERT_TRUE(def);
-        EXPECT_EQ(code, def->getCode());
-    }
-
-    // Sanity check that all 10 option definitions are there.
-    OptionDefContainerPtr option_defs2 = cfg_mgr.getOptionDefs("abcde");
-    ASSERT_TRUE(option_defs2);
-    ASSERT_EQ(10, option_defs2->size());
-
-    // Check that the option codes are valid.
-    code = 105;
-    for (OptionDefContainer::const_iterator it = option_defs2->begin();
-         it != option_defs2->end(); ++it, ++code) {
-        OptionDefinitionPtr def(*it);
-        ASSERT_TRUE(def);
-        EXPECT_EQ(code, def->getCode());
-    }
-
-    // Let's make one more check that the empty set is returned when
-    // invalid option space is used.
-    OptionDefContainerPtr option_defs3 = cfg_mgr.getOptionDefs("non-existing");
-    ASSERT_TRUE(option_defs3);
-    EXPECT_TRUE(option_defs3->empty());
-}
-
-// This test verifies that single option definition is correctly
-// returned with getOptionDef function.
-TEST_F(CfgMgrTest, getOptionDef) {
-    CfgMgr& cfg_mgr = CfgMgr::instance();
-    // Create a set of option definitions with codes between 100 and 109.
-    for (uint16_t code = 100; code < 110; ++code) {
-        std::ostringstream option_name;
-        // Option name is unique, e.g. option-100, option-101 etc.
-        option_name << "option-" << code;
-        OptionDefinitionPtr def(new OptionDefinition(option_name.str(), code,
-                                                     "uint16"));
-        // Add option definition to "isc" option space.
-        // Option codes are not duplicated so expect no error
-        // when adding them.
-        ASSERT_NO_THROW(cfg_mgr.addOptionDef(def, "isc"));
-    }
-
-    // Create a set of option definitions with codes between 105 and 114 and
-    // add them to the different option space.
-    for (uint16_t code = 105; code < 115; ++code) {
-        std::ostringstream option_name;
-        option_name << "option-other-" << code;
-        OptionDefinitionPtr def(new OptionDefinition(option_name.str(), code,
-                                                     "uint16"));
-        ASSERT_NO_THROW(cfg_mgr.addOptionDef(def, "abcde"));
-    }
-
-    // Try to get option definitions one by one using all codes
-    // that we expect to be there.
-    for (uint16_t code = 100; code < 110; ++code) {
-        OptionDefinitionPtr def = cfg_mgr.getOptionDef("isc", code);
-        ASSERT_TRUE(def);
-        // Check that the option name is in the format of 'option-[code]'.
-        // That way we make sure that the options that have the same codes
-        // within different option spaces are different.
-        std::ostringstream option_name;
-        option_name << "option-" << code;
-        EXPECT_EQ(option_name.str(), def->getName());
-        EXPECT_EQ(code, def->getCode());
-    }
-
-    // Check that the option codes are valid.
-    for (uint16_t code = 105; code < 115; ++code) {
-        OptionDefinitionPtr def = cfg_mgr.getOptionDef("abcde", code);
-        ASSERT_TRUE(def);
-        // Check that the option name is in the format of 'option-other-[code]'.
-        // That way we make sure that the options that have the same codes
-        // within different option spaces are different.
-        std::ostringstream option_name;
-        option_name << "option-other-" << code;
-        EXPECT_EQ(option_name.str(), def->getName());
-
-        EXPECT_EQ(code, def->getCode());
-    }
-
-    // Check that an option definition can be added to the standard
-    // (dhcp4 and dhcp6) option spaces when the option code is not
-    // reserved by the standard option.
-    OptionDefinitionPtr def6(new OptionDefinition("option-foo", 79, "uint16"));
-    EXPECT_NO_THROW(cfg_mgr.addOptionDef(def6, "dhcp6"));
-
-    OptionDefinitionPtr def4(new OptionDefinition("option-foo", 222, "uint16"));
-    EXPECT_NO_THROW(cfg_mgr.addOptionDef(def4, "dhcp4"));
-
-    // Try to query the option definition from an non-existing
-    // option space and expect NULL pointer.
-    OptionDefinitionPtr def = cfg_mgr.getOptionDef("non-existing", 56);
-    EXPECT_FALSE(def);
-
-    // Try to get the non-existing option definition from an
-    // existing option space.
-    EXPECT_FALSE(cfg_mgr.getOptionDef("isc", 56));
-
-}
-
-// This test verifies that it is not allowed to override a definition of the
-// standard option which has its definition defined in libdhcp++, but it is
-// allowed to create a definition for the standard option which doesn't have
-// its definition in libdhcp++.
-TEST_F(CfgMgrTest, overrideStdOptionDef) {
-    CfgMgr& cfg_mgr = CfgMgr::instance();
-
-    OptionDefinitionPtr def;
-    // There is a definition for routers option in libdhcp++, so an attempt
-    // to add (override) another definition for this option should fail.
-    def.reset(new OptionDefinition("routers", DHO_ROUTERS, "uint32"));
-    EXPECT_THROW(cfg_mgr.addOptionDef(def, "dhcp4"), isc::BadValue);
-
-    /// @todo There is no definition for the NIS Server Addr option in
-    /// libdhcp++. Once it is implemented it should be not allowed to
-    /// add a custom definition for it. At the moment, it should be ok
-    /// to add a definition for this option (using configuration mechanism)
-    /// because we haven't implemented the one in libdhcp++.
-    def.reset(new OptionDefinition("nis-server-addr", 65, "uint16"));
-    EXPECT_NO_THROW(cfg_mgr.addOptionDef(def, "dhcp4"));
-
-    // It is not allowed to override the definition of the option which
-    // has its definition in the libdhcp++.
-    def.reset(new OptionDefinition("sntp-servers", D6O_SNTP_SERVERS,
-                                   "ipv4-address"));
-    EXPECT_THROW(cfg_mgr.addOptionDef(def, "dhcp6"), isc::BadValue);
-    // There is no definition for option 59 in libdhcp++ yet, so it should
-    // be possible provide a custom definition.
-    def.reset(new OptionDefinition("bootfile-url", 59, "uint32"));
-    EXPECT_NO_THROW(cfg_mgr.addOptionDef(def, "dhcp6"));
-
-}
-
-// This test verifies that the function that adds new option definition
-// throws exceptions when arguments are invalid.
-TEST_F(CfgMgrTest, addOptionDefNegative) {
-    CfgMgr& cfg_mgr = CfgMgr::instance();
-
-    OptionDefinitionPtr def(new OptionDefinition("option-foo", 1000, "uint16"));
-
-    // Try empty option space name.
-    ASSERT_THROW(cfg_mgr.addOptionDef(def, ""), isc::BadValue);
-    // Try NULL option definition.
-    ASSERT_THROW(cfg_mgr.addOptionDef(OptionDefinitionPtr(), "isc"),
-                 isc::dhcp::MalformedOptionDefinition);
-    // Try adding option definition twice and make sure that it
-    // fails on the second attempt.
-    ASSERT_NO_THROW(cfg_mgr.addOptionDef(def, "isc"));
-    EXPECT_THROW(cfg_mgr.addOptionDef(def, "isc"), DuplicateOptionDefinition);
-}
-
 // This test verifies if the configuration manager is able to hold and return
 // valid subnets.
 TEST_F(CfgMgrTest, subnet4) {

src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc

@@ -312,10 +312,12 @@ public:
     /// @brief Constructor
     ParseConfigTest() {
         reset_context();
+        CfgMgr::instance().clear();
     }
 
     ~ParseConfigTest() {
         reset_context();
+        CfgMgr::instance().clear();
     }
 
     /// @brief Parses a configuration.
@@ -447,33 +449,6 @@ public:
         return (rcode_);
     }
 
-    /// @brief Find an option definition for a given space and code within
-    /// the parser context.
-    /// @param space is the space name of the desired option.
-    /// @param code is the numeric "type" of the desired option.
-    /// @return returns an OptionDefinitionPtr which points to the found
-    /// definition or is empty.
-    /// ASSERT_ tests don't work inside functions that return values
-    OptionDefinitionPtr getOptionDef(std::string space, uint32_t code)
-    {
-        OptionDefinitionPtr def;
-        OptionDefContainerPtr defs =
-                            parser_context_->option_defs_->getItems(space);
-        // Should always be able to get definitions list even if it is empty.
-        EXPECT_TRUE(defs);
-        if (defs) {
-            // Attempt to find desired definiton.
-            const OptionDefContainerTypeIndex& idx = defs->get<1>();
-            const OptionDefContainerTypeRange& range = idx.equal_range(code);
-            int cnt = std::distance(range.first, range.second);
-            EXPECT_EQ(1, cnt);
-            if (cnt == 1) {
-                def = *(idx.begin());
-            }
-        }
-        return (def);
-    }
-
     /// @brief Find an option for a given space and code within the parser
     /// context.
     /// @param space is the space name of the desired option.
@@ -511,7 +486,6 @@ public:
         // Note set context universe to V6 as it has to be something.
         CfgMgr::instance().deleteSubnets4();
         CfgMgr::instance().deleteSubnets6();
-        CfgMgr::instance().deleteOptionDefs();
         parser_context_.reset(new ParserContext(Option::V6));
 
         // Ensure no hooks libraries are loaded.
@@ -561,7 +535,8 @@ TEST_F(ParseConfigTest, basicOptionDefTest) {
 
 
     // Verify that the option definition can be retrieved.
-    OptionDefinitionPtr def = getOptionDef("isc", 100);
+    OptionDefinitionPtr def =
+        CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->get("isc", 100);
     ASSERT_TRUE(def);
 
     // Verify that the option definition is correct.
@@ -1141,21 +1116,6 @@ public:
                      values->getPosition(name).file_));
     }
 
-    /// @brief Check that option definition storage in the context holds
-    /// one option definition of the specified type.
-    ///
-    /// @param ctx A pointer to a context.
-    /// @param opt_type Expected option type.
-    void checkOptionDefinitionType(const ParserContext& ctx,
-                                   const uint16_t opt_type) {
-        OptionDefContainerPtr opt_defs =
-            ctx.option_defs_->getItems("option-space");
-        ASSERT_TRUE(opt_defs);
-        OptionDefContainerTypeIndex& idx = opt_defs->get<1>();
-        OptionDefContainerTypeRange range = idx.equal_range(opt_type);
-        EXPECT_EQ(1, std::distance(range.first, range.second));
-    }
-
     /// @brief Check that option storage in the context holds one option
     /// of the specified type.
     ///
@@ -1182,7 +1142,6 @@ public:
         ctx.uint32_values_.reset();
         ctx.string_values_.reset();
         ctx.options_.reset();
-        ctx.option_defs_.reset();
         ctx.hooks_libraries_.reset();
 
         // Even if the fields of the context are NULL, it should get
@@ -1199,7 +1158,6 @@ public:
         EXPECT_FALSE(ctx_new->uint32_values_);
         EXPECT_FALSE(ctx_new->string_values_);
         EXPECT_FALSE(ctx_new->options_);
-        EXPECT_FALSE(ctx_new->option_defs_);
         EXPECT_FALSE(ctx_new->hooks_libraries_);
 
     }
@@ -1264,11 +1222,6 @@ public:
         ASSERT_TRUE(desc1.option);
         ctx.options_->addItem(desc1, option_space);
 
-        // Add new option definition, with option code 123.
-        OptionDefinitionPtr def1(new OptionDefinition("option-foo", 123,
-                                                      "string"));
-        ctx.option_defs_->addItem(def1, option_space);
-
         // Allocate container for hooks libraries and add one library name.
         ctx.hooks_libraries_.reset(new std::vector<std::string>());
         ctx.hooks_libraries_->push_back("library1");
@@ -1356,14 +1309,6 @@ public:
             checkOptionType(*ctx_new, 10);
         }
 
-        // New context has the same option definition.
-        ASSERT_TRUE(ctx_new->option_defs_);
-        {
-            SCOPED_TRACE("check that the option definition is equal in both"
-                         " contexts");
-            checkOptionDefinitionType(*ctx_new, 123);
-        }
-
         // New context has the same hooks library.
         ASSERT_TRUE(ctx_new->hooks_libraries_);
         {
@@ -1494,20 +1439,6 @@ public:
             checkOptionType(*ctx_new, 10);
         }
 
-        // Change the option definition. This should not affect the option
-        // definition in the new context.
-        {
-            SCOPED_TRACE("Check that option definition remains the same in"
-                         " the new context when the option definition in the"
-                         " original context is changed");
-            ctx.option_defs_->clearItems();
-            OptionDefinitionPtr def(new OptionDefinition("option-foo", 234,
-                                                         "string"));
-
-            ctx.option_defs_->addItem(def, option_space);
-            checkOptionDefinitionType(*ctx_new, 123);
-        }
-
         // Change the list of libraries. this should not affect the list in the
         // new context.
         ctx.hooks_libraries_->clear();

src/lib/dhcpsrv/tests/srv_config_unittest.cc

@@ -281,6 +281,8 @@ TEST_F(SrvConfigTest, copy) {
 
     conf1.addLoggingInfo(info);
     conf1.setCfgIface(cfg_iface);
+    conf1.getCfgOptionDef()->add(OptionDefinitionPtr(new OptionDefinition("option-foo", 5,
+                                                                          "string")), "isc");
 
     // Make sure both configurations are different.
     ASSERT_TRUE(conf1 != conf2);
@@ -338,6 +340,20 @@ TEST_F(SrvConfigTest, equality) {
 
     EXPECT_TRUE(conf1 == conf2);
     EXPECT_FALSE(conf1 != conf2);
+
+    // Differ by option definitions.
+    conf1.getCfgOptionDef()->
+        add(OptionDefinitionPtr(new OptionDefinition("option-foo", 123,
+                                                     "uint16_t")), "isc");
+
+    EXPECT_FALSE(conf1 == conf2);
+    EXPECT_TRUE(conf1 != conf2);
+
+    conf2.getCfgOptionDef()->
+        add(OptionDefinitionPtr(new OptionDefinition("option-foo", 123,
+                                                     "uint16_t")), "isc");
+    EXPECT_TRUE(conf1 == conf2);
+    EXPECT_FALSE(conf1 != conf2);
 }
 
 } // end of anonymous namespace