[master] Merge branch 'trac2317'

Conflicts:
	src/bin/dhcp6/dhcp6_srv.cc
	src/lib/dhcpsrv/Makefile.am
	src/lib/dhcpsrv/cfgmgr.h

src/bin/dhcp4/config_parser.h

@@ -28,20 +28,6 @@ namespace dhcp {
 
 class Dhcpv4Srv;
 
-/// An exception that is thrown if an error occurs while configuring an
-/// @c Dhcpv4Srv object.
-class Dhcp4ConfigError : public isc::Exception {
-public:
-
-    /// @brief constructor
-    ///
-    /// @param file name of the file, where exception occurred
-    /// @param line line of the file, where exception occurred
-    /// @param what text description of the issue that caused exception
-    Dhcp4ConfigError(const char* file, size_t line, const char* what)
-        : isc::Exception(file, line, what) {}
-};
-
 /// @brief Configure DHCPv4 server (@c Dhcpv4Srv) with a set of configuration values.
 ///
 /// This function parses configuration information stored in @c config_set

src/bin/dhcp4/ctrl_dhcp4_srv.cc

@@ -19,6 +19,7 @@
 #include <cc/session.h>
 #include <config/ccsession.h>
 #include <dhcp/iface_mgr.h>
+#include <dhcpsrv/dhcp_config_parser.h>
 #include <dhcp4/ctrl_dhcp4_srv.h>
 #include <dhcp4/dhcp4_log.h>
 #include <dhcp4/spec_config.h>
@@ -121,7 +122,7 @@ void ControlledDhcpv4Srv::establishSession() {
 
     try {
         configureDhcp4Server(*this, config_session_->getFullConfig());
-    } catch (const Dhcp4ConfigError& ex) {
+    } catch (const DhcpConfigError& ex) {
         LOG_ERROR(dhcp4_logger, DHCP4_CONFIG_LOAD_FAIL).arg(ex.what());
     }
 

src/bin/dhcp4/dhcp4.spec

@@ -34,6 +34,56 @@
         "item_default": 4000
       },
 
+      { "item_name": "option-def",
+        "item_type": "list",
+        "item_optional": false,
+        "item_default": [],
+        "list_item_spec":
+        {
+          "item_name": "single-option-def",
+          "item_type": "map",
+          "item_optional": false,
+          "item_default": {},
+          "map_item_spec": [
+          {
+            "item_name": "name",
+            "item_type": "string",
+            "item_optional": false,
+            "item_default": ""
+          },
+
+          { "item_name": "code",
+            "item_type": "integer",
+            "item_optional": false,
+            "item_default": 0,
+          },
+
+          { "item_name": "type",
+            "item_type": "string",
+            "item_optional": false,
+            "item_default": "",
+          },
+
+          { "item_name": "array",
+            "item_type": "boolean",
+            "item_optional": false,
+            "item_default": False
+          },
+
+          { "item_name": "record_types",
+            "item_type": "string",
+            "item_optional": false,
+            "item_default": "",
+          },
+
+          { "item_name": "space",
+            "item_type": "string",
+            "item_optional": false,
+            "item_default": ""
+          } ]
+        }
+      },
+
       { "item_name": "option-data",
         "item_type": "list",
         "item_optional": false,
@@ -66,6 +116,11 @@
             "item_type": "boolean",
             "item_optional": false,
             "item_default": False
+          },
+          { "item_name": "space",
+            "item_type": "string",
+            "item_optional": false,
+            "item_default": "dhcp4"
           } ]
         }
       },
@@ -151,6 +206,11 @@
                       "item_type": "boolean",
                       "item_optional": false,
                       "item_default": False
+                      },
+                    { "item_name": "space",
+                      "item_type": "string",
+                      "item_optional": false,
+                      "item_default": "dhcp4"
                     } ]
                   }
                 } ]

src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -78,8 +78,8 @@ public:
     /// @brief Create the simple configuration with single option.
     ///
     /// This function allows to set one of the parameters that configure
-    /// option value. These parameters are: "name", "code", "data" and
-    /// "csv-format".
+    /// option value. These parameters are: "name", "code", "data",
+    /// "csv-format" and "space".
     ///
     /// @param param_value string holiding option parameter value to be
     /// injected into the configuration string.
@@ -92,21 +92,31 @@ public:
         std::map<std::string, std::string> params;
         if (parameter == "name") {
             params["name"] = param_value;
+            params["space"] = "dhcp4";
+            params["code"] = "56";
+            params["data"] = "AB CDEF0105";
+            params["csv-format"] = "False";
+        } else if (parameter == "space") {
+            params["name"] = "dhcp-message";
+            params["space"] = param_value;
             params["code"] = "56";
             params["data"] = "AB CDEF0105";
             params["csv-format"] = "False";
         } else if (parameter == "code") {
-            params["name"] = "option_foo";
+            params["name"] = "dhcp-message";
+            params["space"] = "dhcp4";
             params["code"] = param_value;
             params["data"] = "AB CDEF0105";
             params["csv-format"] = "False";
         } else if (parameter == "data") {
-            params["name"] = "option_foo";
+            params["name"] = "dhcp-message";
+            params["space"] = "dhcp4";
             params["code"] = "56";
             params["data"] = param_value;
             params["csv-format"] = "False";
         } else if (parameter == "csv-format") {
-            params["name"] = "option_foo";
+            params["name"] = "dhcp-message";
+            params["space"] = "dhcp4";
             params["code"] = "56";
             params["data"] = "AB CDEF0105";
             params["csv-format"] = param_value;
@@ -142,6 +152,8 @@ public:
             }
             if (param.first == "name") {
                 stream << "\"name\": \"" << param.second << "\"";
+            } else if (param.first == "space") {
+                stream << "\"space\": \"" << param.second << "\"";
             } else if (param.first == "code") {
                 stream << "\"code\": " << param.second << "";
             } else if (param.first == "data") {
@@ -234,6 +246,7 @@ public:
             "\"renew-timer\": 1000, "
             "\"valid-lifetime\": 4000, "
             "\"subnet4\": [ ], "
+            "\"option-def\": [ ], "
             "\"option-data\": [ ] }";
 
         try {
@@ -436,6 +449,363 @@ TEST_F(Dhcp4ParserTest, poolPrefixLen) {
     EXPECT_EQ(4000, subnet->getValid());
 }
 
+// The goal of this test is to check whether an option definition
+// that defines an option carrying an IPv4 address can be created.
+TEST_F(Dhcp4ParserTest, optionDefIpv4Address) {
+
+    // Configuration string.
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo\","
+        "      \"code\": 100,"
+        "      \"type\": \"ipv4-address\","
+        "      \"array\": False,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"isc\""
+        "  } ]"
+        "}";
+    ElementPtr json = Element::fromJSON(config);
+
+    // Make sure that the particular option definition does not exist.
+    OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100);
+    ASSERT_FALSE(def);
+
+    // Use the configuration string to create new option definition.
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    ASSERT_TRUE(status);
+
+    // The option definition should now be available in the CfgMgr.
+    def = CfgMgr::instance().getOptionDef("isc", 100);
+    ASSERT_TRUE(def);
+
+    // Verify that the option definition data is valid.
+    EXPECT_EQ("foo", def->getName());
+    EXPECT_EQ(100, def->getCode());
+    EXPECT_FALSE(def->getArrayType());
+    EXPECT_EQ(OPT_IPV4_ADDRESS_TYPE, def->getType());
+}
+
+// The goal of this test is to check whether an option definiiton
+// that defines an option carrying a record of data fields can
+// be created.
+TEST_F(Dhcp4ParserTest, optionDefRecord) {
+
+    // Configuration string.
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo\","
+        "      \"code\": 100,"
+        "      \"type\": \"record\","
+        "      \"array\": False,"
+        "      \"record-types\": \"uint16, ipv4-address, ipv6-address, string\","
+        "      \"space\": \"isc\""
+        "  } ]"
+        "}";
+    ElementPtr json = Element::fromJSON(config);
+
+    // Make sure that the particular option definition does not exist.
+    OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100);
+    ASSERT_FALSE(def);
+
+    // Use the configuration string to create new option definition.
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    ASSERT_TRUE(status);
+    checkResult(status, 0);
+
+    // The option definition should now be available in the CfgMgr.
+    def = CfgMgr::instance().getOptionDef("isc", 100);
+    ASSERT_TRUE(def);
+
+    // Check the option data.
+    EXPECT_EQ("foo", def->getName());
+    EXPECT_EQ(100, def->getCode());
+    EXPECT_EQ(OPT_RECORD_TYPE, def->getType());
+    EXPECT_FALSE(def->getArrayType());
+
+    // The option comprises the record of data fields. Verify that all
+    // fields are present and they are of the expected types.
+    const OptionDefinition::RecordFieldsCollection& record_fields =
+        def->getRecordFields();
+    ASSERT_EQ(4, record_fields.size());
+    EXPECT_EQ(OPT_UINT16_TYPE, record_fields[0]);
+    EXPECT_EQ(OPT_IPV4_ADDRESS_TYPE, record_fields[1]);
+    EXPECT_EQ(OPT_IPV6_ADDRESS_TYPE, record_fields[2]);
+    EXPECT_EQ(OPT_STRING_TYPE, record_fields[3]);
+}
+
+// The goal of this test is to verify that multiple option definitions
+// can be created.
+TEST_F(Dhcp4ParserTest, optionDefMultiple) {
+    // Configuration string.
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo\","
+        "      \"code\": 100,"
+        "      \"type\": \"uint32\","
+        "      \"array\": False,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"isc\""
+        "  },"
+        "  {"
+        "      \"name\": \"foo-2\","
+        "      \"code\": 101,"
+        "      \"type\": \"ipv4-address\","
+        "      \"array\": False,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"isc\""
+        "  } ]"
+        "}";
+    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));
+
+    // Use the configuration string to create new option definitions.
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    ASSERT_TRUE(status);
+    checkResult(status, 0);
+
+    // Check the first definition we have created.
+    OptionDefinitionPtr def1 = CfgMgr::instance().getOptionDef("isc", 100);
+    ASSERT_TRUE(def1);
+
+    // Check the option data.
+    EXPECT_EQ("foo", def1->getName());
+    EXPECT_EQ(100, def1->getCode());
+    EXPECT_EQ(OPT_UINT32_TYPE, def1->getType());
+    EXPECT_FALSE(def1->getArrayType());
+
+    // Check the second option definition we have created.
+    OptionDefinitionPtr def2 = CfgMgr::instance().getOptionDef("isc", 101);
+    ASSERT_TRUE(def2);
+
+    // Check the option data.
+    EXPECT_EQ("foo-2", def2->getName());
+    EXPECT_EQ(101, def2->getCode());
+    EXPECT_EQ(OPT_IPV4_ADDRESS_TYPE, def2->getType());
+    EXPECT_FALSE(def2->getArrayType());
+}
+
+// The goal of this test is to verify that the duplicated option
+// definition is not accepted.
+TEST_F(Dhcp4ParserTest, optionDefDuplicate) {
+
+    // Configuration string. Both option definitions have
+    // the same code and belong to the same option space.
+    // This configuration should not be accepted.
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo\","
+        "      \"code\": 100,"
+        "      \"type\": \"uint32\","
+        "      \"array\": False,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"isc\""
+        "  },"
+        "  {"
+        "      \"name\": \"foo-2\","
+        "      \"code\": 100,"
+        "      \"type\": \"ipv4-address\","
+        "      \"array\": False,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"isc\""
+        "  } ]"
+        "}";
+    ElementPtr json = Element::fromJSON(config);
+
+    // Make sure that the option definition does not exist yet.
+    ASSERT_FALSE(CfgMgr::instance().getOptionDef("isc", 100));
+
+    // Use the configuration string to create new option definitions.
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    ASSERT_TRUE(status);
+    checkResult(status, 1);
+}
+
+// The goal of this test is to verify that the option definition
+// comprising an array of uint32 values can be created.
+TEST_F(Dhcp4ParserTest, optionDefArray) {
+
+    // Configuration string. Created option definition should
+    // comprise an array of uint32 values.
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo\","
+        "      \"code\": 100,"
+        "      \"type\": \"uint32\","
+        "      \"array\": True,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"isc\""
+        "  } ]"
+        "}";
+    ElementPtr json = Element::fromJSON(config);
+
+    // Make sure that the particular option definition does not exist.
+    OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100);
+    ASSERT_FALSE(def);
+
+    // Use the configuration string to create new option definition.
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    ASSERT_TRUE(status);
+    checkResult(status, 0);
+
+    // The option definition should now be available in the CfgMgr.
+    def = CfgMgr::instance().getOptionDef("isc", 100);
+    ASSERT_TRUE(def);
+
+    // Check the option data.
+    EXPECT_EQ("foo", def->getName());
+    EXPECT_EQ(100, def->getCode());
+    EXPECT_EQ(OPT_UINT32_TYPE, def->getType());
+    EXPECT_TRUE(def->getArrayType());
+}
+
+/// The purpose of this test is to verify that the option definition
+/// with invalid name is not accepted.
+TEST_F(Dhcp4ParserTest, optionDefInvalidName) {
+    // Configuration string. The option name is invalid as it
+    // contains the % character.
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"invalid%name\","
+        "      \"code\": 100,"
+        "      \"type\": \"string\","
+        "      \"array\": False,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"isc\""
+        "  } ]"
+        "}";
+    ElementPtr json = Element::fromJSON(config);
+
+    // Use the configuration string to create new option definition.
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    ASSERT_TRUE(status);
+    // Expecting parsing error (error code 1).
+    checkResult(status, 1);
+}
+
+/// The purpose of this test is to verify that the option definition
+/// with invalid type is not accepted.
+TEST_F(Dhcp4ParserTest, optionDefInvalidType) {
+    // Configuration string. The option type is invalid. It is
+    // "sting" instead of "string".
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo\","
+        "      \"code\": 100,"
+        "      \"type\": \"sting\","
+        "      \"array\": False,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"isc\""
+        "  } ]"
+        "}";
+    ElementPtr json = Element::fromJSON(config);
+
+    // Use the configuration string to create new option definition.
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    ASSERT_TRUE(status);
+    // Expecting parsing error (error code 1).
+    checkResult(status, 1);
+}
+
+/// The purpose of this test is to verify that the option definition
+/// with invalid type is not accepted.
+TEST_F(Dhcp4ParserTest, optionDefInvalidRecordType) {
+    // Configuration string. The third of the record fields
+    // is invalid. It is "sting" instead of "string".
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo\","
+        "      \"code\": 100,"
+        "      \"type\": \"record\","
+        "      \"array\": False,"
+        "      \"record-types\": \"uint32,uint8,sting\","
+        "      \"space\": \"isc\""
+        "  } ]"
+        "}";
+    ElementPtr json = Element::fromJSON(config);
+
+    // Use the configuration string to create new option definition.
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    ASSERT_TRUE(status);
+    // Expecting parsing error (error code 1).
+    checkResult(status, 1);
+}
+
+
+/// The purpose of this test is to verify that it is not allowed
+/// to override the standard option (that belongs to dhcp4 option
+/// space) and that it is allowed to define option in the dhcp4
+/// option space that has a code which is not used by any of the
+/// standard options.
+TEST_F(Dhcp4ParserTest, optionStandardDefOverride) {
+
+    // Configuration string. The option code 109 is unassigned
+    // so it can be used for a custom option definition in
+    // dhcp4 option space.
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo\","
+        "      \"code\": 109,"
+        "      \"type\": \"string\","
+        "      \"array\": False,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"dhcp4\""
+        "  } ]"
+        "}";
+    ElementPtr json = Element::fromJSON(config);
+
+    OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("dhcp4", 109);
+    ASSERT_FALSE(def);
+
+    // Use the configuration string to create new option definition.
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    ASSERT_TRUE(status);
+    checkResult(status, 0);
+
+    // The option definition should now be available in the CfgMgr.
+    def = CfgMgr::instance().getOptionDef("dhcp4", 109);
+    ASSERT_TRUE(def);
+
+    // Check the option data.
+    EXPECT_EQ("foo", def->getName());
+    EXPECT_EQ(109, def->getCode());
+    EXPECT_EQ(OPT_STRING_TYPE, def->getType());
+    EXPECT_FALSE(def->getArrayType());
+
+    // The combination of option space and code is
+    // invalid. The 'dhcp4' option space groups
+    // standard options and the code 100 is reserved
+    // for one of them.
+    config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo\","
+        "      \"code\": 100,"
+        "      \"type\": \"string\","
+        "      \"array\": False,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"dhcp4\""
+        "  } ]"
+        "}";
+    json = Element::fromJSON(config);
+
+    // Use the configuration string to create new option definition.
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    ASSERT_TRUE(status);
+    // Expecting parsing error (error code 1).
+    checkResult(status, 1);
+}
+
 // Goal of this test is to verify that global option
 // data is configured for the subnet if the subnet
 // configuration does not include options configuration.
@@ -445,13 +815,15 @@ TEST_F(Dhcp4ParserTest, optionDataDefaults) {
         "\"rebind-timer\": 2000,"
         "\"renew-timer\": 1000,"
         "\"option-data\": [ {"
-        "    \"name\": \"option_foo\","
+        "    \"name\": \"dhcp-message\","
+        "    \"space\": \"dhcp4\","
         "    \"code\": 56,"
         "    \"data\": \"AB CDEF0105\","
         "    \"csv-format\": False"
         " },"
         " {"
-        "    \"name\": \"option_foo2\","
+        "    \"name\": \"default-ip-ttl\","
+        "    \"space\": \"dhcp4\","
         "    \"code\": 23,"
         "    \"data\": \"01\","
         "    \"csv-format\": False"
@@ -500,6 +872,74 @@ TEST_F(Dhcp4ParserTest, optionDataDefaults) {
     testOption(*range.first, 23, foo2_expected, sizeof(foo2_expected));
 }
 
+/// The goal of this test is to verify that two options having the same
+/// option code can be added to different option spaces.
+TEST_F(Dhcp4ParserTest, optionDataTwoSpaces) {
+
+    // This configuration string is to configure two options
+    // sharing the code 56 and having different definitions
+    // and belonging to the different option spaces.
+    // The option definition must be provided for the
+    // option that belongs to the 'isc' option space.
+    // The definition is not required for the option that
+    // belongs to the 'dhcp4' option space as it is the
+    // standard option.
+    string config = "{ \"interface\": [ \"all\" ],"
+        "\"rebind-timer\": 2000,"
+        "\"renew-timer\": 1000,"
+        "\"option-data\": [ {"
+        "    \"name\": \"dhcp-message\","
+        "    \"space\": \"dhcp4\","
+        "    \"code\": 56,"
+        "    \"data\": \"AB CDEF0105\","
+        "    \"csv-format\": False"
+        " },"
+        " {"
+        "    \"name\": \"foo\","
+        "    \"space\": \"isc\","
+        "    \"code\": 56,"
+        "    \"data\": \"1234\","
+        "    \"csv-format\": True"
+        " } ],"
+        "\"option-def\": [ {"
+        "    \"name\": \"foo\","
+        "    \"code\": 56,"
+        "    \"type\": \"uint32\","
+        "    \"array\": False,"
+        "    \"record-types\": \"\","
+        "    \"space\": \"isc\""
+        " } ],"
+        "\"subnet4\": [ { "
+        "    \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+        "    \"subnet\": \"192.0.2.0/24\""
+        " } ]"
+        "}";
+
+    ConstElementPtr status;
+
+    ElementPtr json = Element::fromJSON(config);
+
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    ASSERT_TRUE(status);
+    checkResult(status, 0);
+
+    // Options should be now availabe for the subnet.
+    Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.200"));
+    ASSERT_TRUE(subnet);
+    // Try to get the option from the space dhcp4.
+    Subnet::OptionDescriptor desc1 = subnet->getOptionDescriptor("dhcp4", 56);
+    ASSERT_TRUE(desc1.option);
+    EXPECT_EQ(56, desc1.option->getType());
+    // Try to get the option from the space isc.
+    Subnet::OptionDescriptor desc2 = subnet->getOptionDescriptor("isc", 56);
+    ASSERT_TRUE(desc2.option);
+    EXPECT_EQ(56, desc1.option->getType());
+    // Try to get the non-existing option from the non-existing
+    // option space and  expect that option is not returned.
+    Subnet::OptionDescriptor desc3 = subnet->getOptionDescriptor("non-existing", 56);
+    ASSERT_FALSE(desc3.option);
+}
+
 // Goal of this test is to verify options configuration
 // for a single subnet. In particular this test checks
 // that local options configuration overrides global
@@ -510,7 +950,8 @@ TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) {
         "\"rebind-timer\": 2000, "
         "\"renew-timer\": 1000, "
         "\"option-data\": [ {"
-        "      \"name\": \"option_foo\","
+        "      \"name\": \"dhcp-message\","
+        "      \"space\": \"dhcp4\","
         "      \"code\": 56,"
         "      \"data\": \"AB\","
         "      \"csv-format\": False"
@@ -519,13 +960,15 @@ TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) {
         "    \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
         "    \"subnet\": \"192.0.2.0/24\", "
         "    \"option-data\": [ {"
-        "          \"name\": \"option_foo\","
+        "          \"name\": \"dhcp-message\","
+        "          \"space\": \"dhcp4\","
         "          \"code\": 56,"
         "          \"data\": \"AB CDEF0105\","
         "          \"csv-format\": False"
         "        },"
         "        {"
-        "          \"name\": \"option_foo2\","
+        "          \"name\": \"default-ip-ttl\","
+        "          \"space\": \"dhcp4\","
         "          \"code\": 23,"
         "          \"data\": \"01\","
         "          \"csv-format\": False"
@@ -582,7 +1025,8 @@ TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) {
         "    \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
         "    \"subnet\": \"192.0.2.0/24\", "
         "    \"option-data\": [ {"
-        "          \"name\": \"option_foo\","
+        "          \"name\": \"dhcp-message\","
+        "          \"space\": \"dhcp4\","
         "          \"code\": 56,"
         "          \"data\": \"0102030405060708090A\","
         "          \"csv-format\": False"
@@ -592,7 +1036,8 @@ TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) {
         "    \"pool\": [ \"192.0.3.101 - 192.0.3.150\" ],"
         "    \"subnet\": \"192.0.3.0/24\", "
         "    \"option-data\": [ {"
-        "          \"name\": \"option_foo2\","
+        "          \"name\": \"default-ip-ttl\","
+        "          \"space\": \"dhcp4\","
         "          \"code\": 23,"
         "          \"data\": \"FF\","
         "          \"csv-format\": False"
@@ -745,6 +1190,7 @@ TEST_F(Dhcp4ParserTest, stdOptionData) {
     ConstElementPtr x;
     std::map<std::string, std::string> params;
     params["name"] = "nis-servers";
+    params["space"] = "dhcp4";
     // Option code 41 means nis-servers.
     params["code"] = "41";
     // Specify option values in a CSV (user friendly) format.

src/bin/dhcp6/config_parser.h

@@ -27,20 +27,6 @@ namespace dhcp {
 
 class Dhcpv6Srv;
 
-/// An exception that is thrown if an error occurs while configuring an
-/// @c Dhcpv6Srv object.
-class Dhcp6ConfigError : public isc::Exception {
-public:
-
-    /// @brief constructor
-    ///
-    /// @param file name of the file, where exception occurred
-    /// @param line line of the file, where exception occurred
-    /// @param what text description of the issue that caused exception
-    Dhcp6ConfigError(const char* file, size_t line, const char* what)
-        : isc::Exception(file, line, what) {}
-};
-
 /// @brief Configures DHCPv6 server
 ///
 /// This function is called every time a new configuration is received. The extra

src/bin/dhcp6/ctrl_dhcp6_srv.cc

@@ -19,6 +19,7 @@
 #include <cc/session.h>
 #include <config/ccsession.h>
 #include <dhcp/iface_mgr.h>
+#include <dhcpsrv/dhcp_config_parser.h>
 #include <dhcp6/config_parser.h>
 #include <dhcp6/ctrl_dhcp6_srv.h>
 #include <dhcp6/dhcp6_log.h>
@@ -121,7 +122,7 @@ void ControlledDhcpv6Srv::establishSession() {
 
     try {
         configureDhcp6Server(*this, config_session_->getFullConfig());
-    } catch (const Dhcp6ConfigError& ex) {
+    } catch (const DhcpConfigError& ex) {
         LOG_ERROR(dhcp6_logger, DHCP6_CONFIG_LOAD_FAIL).arg(ex.what());
     }
 

src/bin/dhcp6/dhcp6.spec

@@ -40,6 +40,56 @@
         "item_default": 4000
       },
 
+      { "item_name": "option-def",
+        "item_type": "list",
+        "item_optional": false,
+        "item_default": [],
+        "list_item_spec":
+        {
+          "item_name": "single-option-def",
+          "item_type": "map",
+          "item_optional": false,
+          "item_default": {},
+          "map_item_spec": [
+          {
+            "item_name": "name",
+            "item_type": "string",
+            "item_optional": false,
+            "item_default": ""
+          },
+
+          { "item_name": "code",
+            "item_type": "integer",
+            "item_optional": false,
+            "item_default": 0,
+          },
+
+          { "item_name": "type",
+            "item_type": "string",
+            "item_optional": false,
+            "item_default": "",
+          },
+
+          { "item_name": "array",
+            "item_type": "boolean",
+            "item_optional": false,
+            "item_default": False
+          },
+
+          { "item_name": "record_types",
+            "item_type": "string",
+            "item_optional": false,
+            "item_default": "",
+          },
+
+          { "item_name": "space",
+            "item_type": "string",
+            "item_optional": false,
+            "item_default": ""
+          } ]
+        }
+      },
+
       { "item_name": "option-data",
         "item_type": "list",
         "item_optional": false,
@@ -72,6 +122,11 @@
             "item_type": "boolean",
             "item_optional": false,
             "item_default": False
+          },
+          { "item_name": "space",
+            "item_type": "string",
+            "item_optional": false,
+            "item_default": "dhcp6"
           } ]
         }
       },
@@ -162,6 +217,11 @@
                       "item_type": "boolean",
                       "item_optional": false,
                       "item_default": False
+                    },
+                    { "item_name": "space",
+                      "item_type": "string",
+                      "item_optional": false,
+                      "item_default": "dhcp6"
                     } ]
                   }
                 } ]

src/bin/dhcp6/dhcp6_srv.cc

@@ -405,7 +405,7 @@ void Dhcpv6Srv::sanityCheck(const Pkt6Ptr& pkt, RequirementLevel clientid,
     Option::OptionCollection server_ids = pkt->getOptions(D6O_SERVERID);
     switch (serverid) {
     case FORBIDDEN:
-        if (server_ids.size() > 0) {
+        if (!server_ids.empty()) {
             isc_throw(RFCViolation, "Server-id option was not expected, but "
                       << server_ids.size() << " received in " << pkt->getName());
         }

src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -53,6 +53,15 @@ public:
         resetConfiguration();
     };
 
+    // Checks if config_result (result of DHCP server configuration) has
+    // expected code (0 for success, other for failures).
+    // Also stores result in rcode_ and comment_.
+    void checkResult(ConstElementPtr status, int expected_code) {
+        ASSERT_TRUE(status);
+        comment_ = parseAnswer(rcode_, status);
+        EXPECT_EQ(expected_code, rcode_);
+    }
+
     /// @brief Create the simple configuration with single option.
     ///
     /// This function allows to set one of the parameters that configure
@@ -68,22 +77,32 @@ public:
         std::map<std::string, std::string> params;
         if (parameter == "name") {
             params["name"] = param_value;
-            params["code"] = "80";
+            params["space"] = "dhcp6";
+            params["code"] = "38";
+            params["data"] = "AB CDEF0105";
+            params["csv-format"] = "False";
+        } else if (parameter == "space") {
+            params["name"] = "subscriber-id";
+            params["space"] = param_value;
+            params["code"] = "38";
             params["data"] = "AB CDEF0105";
             params["csv-format"] = "False";
         } else if (parameter == "code") {
-            params["name"] = "option_foo";
+            params["name"] = "subscriber-id";
+            params["space"] = "dhcp6";
             params["code"] = param_value;
             params["data"] = "AB CDEF0105";
             params["csv-format"] = "False";
         } else if (parameter == "data") {
-            params["name"] = "option_foo";
-            params["code"] = "80";
+            params["name"] = "subscriber-id";
+            params["space"] = "dhcp6";
+            params["code"] = "38";
             params["data"] = param_value;
             params["csv-format"] = "False";
         } else if (parameter == "csv-format") {
-            params["name"] = "option_foo";
-            params["code"] = "80";
+            params["name"] = "subscriber-id";
+            params["space"] = "dhcp6";
+            params["code"] = "38";
             params["data"] = "AB CDEF0105";
             params["csv-format"] = param_value;
         }
@@ -113,6 +132,8 @@ public:
             }
             if (param.first == "name") {
                 stream << "\"name\": \"" << param.second << "\"";
+            } else if (param.first == "space") {
+                stream << "\"space\": \"" << param.second << "\"";
             } else if (param.first == "code") {
                 stream << "\"code\": " << param.second;;
             } else if (param.first == "data") {
@@ -144,6 +165,7 @@ public:
             "\"renew-timer\": 1000, "
             "\"valid-lifetime\": 4000, "
             "\"subnet6\": [ ], "
+            "\"option-def\": [ ], "
             "\"option-data\": [ ] }";
 
         try {
@@ -427,6 +449,363 @@ TEST_F(Dhcp6ParserTest, poolPrefixLen) {
     EXPECT_EQ(4000, subnet->getValid());
 }
 
+// The goal of this test is to check whether an option definition
+// that defines an option carrying an IPv6 address can be created.
+TEST_F(Dhcp6ParserTest, optionDefIpv6Address) {
+
+    // Configuration string.
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo\","
+        "      \"code\": 100,"
+        "      \"type\": \"ipv6-address\","
+        "      \"array\": False,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"isc\""
+        "  } ]"
+        "}";
+    ElementPtr json = Element::fromJSON(config);
+
+    // Make sure that the particular option definition does not exist.
+    OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100);
+    ASSERT_FALSE(def);
+
+    // Use the configuration string to create new option definition.
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
+    ASSERT_TRUE(status);
+
+    // The option definition should now be available in the CfgMgr.
+    def = CfgMgr::instance().getOptionDef("isc", 100);
+    ASSERT_TRUE(def);
+
+    // Verify that the option definition data is valid.
+    EXPECT_EQ("foo", def->getName());
+    EXPECT_EQ(100, def->getCode());
+    EXPECT_FALSE(def->getArrayType());
+    EXPECT_EQ(OPT_IPV6_ADDRESS_TYPE, def->getType());
+}
+
+// The goal of this test is to check whether an option definiiton
+// that defines an option carrying a record of data fields can
+// be created.
+TEST_F(Dhcp6ParserTest, optionDefRecord) {
+
+    // Configuration string.
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo\","
+        "      \"code\": 100,"
+        "      \"type\": \"record\","
+        "      \"array\": False,"
+        "      \"record-types\": \"uint16, ipv4-address, ipv6-address, string\","
+        "      \"space\": \"isc\""
+        "  } ]"
+        "}";
+    ElementPtr json = Element::fromJSON(config);
+
+    // Make sure that the particular option definition does not exist.
+    OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100);
+    ASSERT_FALSE(def);
+
+    // Use the configuration string to create new option definition.
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
+    ASSERT_TRUE(status);
+    checkResult(status, 0);
+
+    // The option definition should now be available in the CfgMgr.
+    def = CfgMgr::instance().getOptionDef("isc", 100);
+    ASSERT_TRUE(def);
+
+    // Check the option data.
+    EXPECT_EQ("foo", def->getName());
+    EXPECT_EQ(100, def->getCode());
+    EXPECT_EQ(OPT_RECORD_TYPE, def->getType());
+    EXPECT_FALSE(def->getArrayType());
+
+    // The option comprises the record of data fields. Verify that all
+    // fields are present and they are of the expected types.
+    const OptionDefinition::RecordFieldsCollection& record_fields =
+        def->getRecordFields();
+    ASSERT_EQ(4, record_fields.size());
+    EXPECT_EQ(OPT_UINT16_TYPE, record_fields[0]);
+    EXPECT_EQ(OPT_IPV4_ADDRESS_TYPE, record_fields[1]);
+    EXPECT_EQ(OPT_IPV6_ADDRESS_TYPE, record_fields[2]);
+    EXPECT_EQ(OPT_STRING_TYPE, record_fields[3]);
+}
+
+// The goal of this test is to verify that multiple option definitions
+// can be created.
+TEST_F(Dhcp6ParserTest, optionDefMultiple) {
+    // Configuration string.
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo\","
+        "      \"code\": 100,"
+        "      \"type\": \"uint32\","
+        "      \"array\": False,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"isc\""
+        "  },"
+        "  {"
+        "      \"name\": \"foo-2\","
+        "      \"code\": 101,"
+        "      \"type\": \"ipv4-address\","
+        "      \"array\": False,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"isc\""
+        "  } ]"
+        "}";
+    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));
+
+    // Use the configuration string to create new option definitions.
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
+    ASSERT_TRUE(status);
+    checkResult(status, 0);
+
+    // Check the first definition we have created.
+    OptionDefinitionPtr def1 = CfgMgr::instance().getOptionDef("isc", 100);
+    ASSERT_TRUE(def1);
+
+    // Check the option data.
+    EXPECT_EQ("foo", def1->getName());
+    EXPECT_EQ(100, def1->getCode());
+    EXPECT_EQ(OPT_UINT32_TYPE, def1->getType());
+    EXPECT_FALSE(def1->getArrayType());
+
+    // Check the second option definition we have created.
+    OptionDefinitionPtr def2 = CfgMgr::instance().getOptionDef("isc", 101);
+    ASSERT_TRUE(def2);
+
+    // Check the option data.
+    EXPECT_EQ("foo-2", def2->getName());
+    EXPECT_EQ(101, def2->getCode());
+    EXPECT_EQ(OPT_IPV4_ADDRESS_TYPE, def2->getType());
+    EXPECT_FALSE(def2->getArrayType());
+}
+
+// The goal of this test is to verify that the duplicated option
+// definition is not accepted.
+TEST_F(Dhcp6ParserTest, optionDefDuplicate) {
+
+    // Configuration string. Both option definitions have
+    // the same code and belong to the same option space.
+    // This configuration should not be accepted.
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo\","
+        "      \"code\": 100,"
+        "      \"type\": \"uint32\","
+        "      \"array\": False,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"isc\""
+        "  },"
+        "  {"
+        "      \"name\": \"foo-2\","
+        "      \"code\": 100,"
+        "      \"type\": \"ipv4-address\","
+        "      \"array\": False,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"isc\""
+        "  } ]"
+        "}";
+    ElementPtr json = Element::fromJSON(config);
+
+    // Make sure that the option definition does not exist yet.
+    ASSERT_FALSE(CfgMgr::instance().getOptionDef("isc", 100));
+
+    // Use the configuration string to create new option definitions.
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
+    ASSERT_TRUE(status);
+    checkResult(status, 1);
+}
+
+// The goal of this test is to verify that the option definition
+// comprising an array of uint32 values can be created.
+TEST_F(Dhcp6ParserTest, optionDefArray) {
+
+    // Configuration string. Created option definition should
+    // comprise an array of uint32 values.
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo\","
+        "      \"code\": 100,"
+        "      \"type\": \"uint32\","
+        "      \"array\": True,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"isc\""
+        "  } ]"
+        "}";
+    ElementPtr json = Element::fromJSON(config);
+
+    // Make sure that the particular option definition does not exist.
+    OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("isc", 100);
+    ASSERT_FALSE(def);
+
+    // Use the configuration string to create new option definition.
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
+    ASSERT_TRUE(status);
+    checkResult(status, 0);
+
+    // The option definition should now be available in the CfgMgr.
+    def = CfgMgr::instance().getOptionDef("isc", 100);
+    ASSERT_TRUE(def);
+
+    // Check the option data.
+    EXPECT_EQ("foo", def->getName());
+    EXPECT_EQ(100, def->getCode());
+    EXPECT_EQ(OPT_UINT32_TYPE, def->getType());
+    EXPECT_TRUE(def->getArrayType());
+}
+
+/// The purpose of this test is to verify that the option definition
+/// with invalid name is not accepted.
+TEST_F(Dhcp6ParserTest, optionDefInvalidName) {
+    // Configuration string. The option name is invalid as it
+    // contains the % character.
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"invalid%name\","
+        "      \"code\": 100,"
+        "      \"type\": \"string\","
+        "      \"array\": False,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"isc\""
+        "  } ]"
+        "}";
+    ElementPtr json = Element::fromJSON(config);
+
+    // Use the configuration string to create new option definition.
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
+    ASSERT_TRUE(status);
+    // Expecting parsing error (error code 1).
+    checkResult(status, 1);
+}
+
+/// The purpose of this test is to verify that the option definition
+/// with invalid type is not accepted.
+TEST_F(Dhcp6ParserTest, optionDefInvalidType) {
+    // Configuration string. The option type is invalid. It is
+    // "sting" instead of "string".
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo\","
+        "      \"code\": 100,"
+        "      \"type\": \"sting\","
+        "      \"array\": False,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"isc\""
+        "  } ]"
+        "}";
+    ElementPtr json = Element::fromJSON(config);
+
+    // Use the configuration string to create new option definition.
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
+    ASSERT_TRUE(status);
+    // Expecting parsing error (error code 1).
+    checkResult(status, 1);
+}
+
+/// The purpose of this test is to verify that the option definition
+/// with invalid type is not accepted.
+TEST_F(Dhcp6ParserTest, optionDefInvalidRecordType) {
+    // Configuration string. The third of the record fields
+    // is invalid. It is "sting" instead of "string".
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo\","
+        "      \"code\": 100,"
+        "      \"type\": \"record\","
+        "      \"array\": False,"
+        "      \"record-types\": \"uint32,uint8,sting\","
+        "      \"space\": \"isc\""
+        "  } ]"
+        "}";
+    ElementPtr json = Element::fromJSON(config);
+
+    // Use the configuration string to create new option definition.
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
+    ASSERT_TRUE(status);
+    // Expecting parsing error (error code 1).
+    checkResult(status, 1);
+}
+
+
+/// The purpose of this test is to verify that it is not allowed
+/// to override the standard option (that belongs to dhcp6 option
+/// space) and that it is allowed to define option in the dhcp6
+/// option space that has a code which is not used by any of the
+/// standard options.
+TEST_F(Dhcp6ParserTest, optionStandardDefOverride) {
+
+    // Configuration string. The option code 100 is unassigned
+    // so it can be used for a custom option definition in
+    // dhcp6 option space.
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo\","
+        "      \"code\": 100,"
+        "      \"type\": \"string\","
+        "      \"array\": False,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"dhcp6\""
+        "  } ]"
+        "}";
+    ElementPtr json = Element::fromJSON(config);
+
+    OptionDefinitionPtr def = CfgMgr::instance().getOptionDef("dhcp6", 100);
+    ASSERT_FALSE(def);
+
+    // Use the configuration string to create new option definition.
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
+    ASSERT_TRUE(status);
+    checkResult(status, 0);
+
+    // The option definition should now be available in the CfgMgr.
+    def = CfgMgr::instance().getOptionDef("dhcp6", 100);
+    ASSERT_TRUE(def);
+
+    // Check the option data.
+    EXPECT_EQ("foo", def->getName());
+    EXPECT_EQ(100, def->getCode());
+    EXPECT_EQ(OPT_STRING_TYPE, def->getType());
+    EXPECT_FALSE(def->getArrayType());
+
+    // The combination of option space and code is
+    // invalid. The 'dhcp6' option space groups
+    // standard options and the code 3 is reserved
+    // for one of them.
+    config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo\","
+        "      \"code\": 3,"
+        "      \"type\": \"string\","
+        "      \"array\": False,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"dhcp6\""
+        "  } ]"
+        "}";
+    json = Element::fromJSON(config);
+
+    // Use the configuration string to create new option definition.
+    EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
+    ASSERT_TRUE(status);
+    // Expecting parsing error (error code 1).
+    checkResult(status, 1);
+}
+
 // Goal of this test is to verify that global option
 // data is configured for the subnet if the subnet
 // configuration does not include options configuration.
@@ -437,16 +816,18 @@ TEST_F(Dhcp6ParserTest, optionDataDefaults) {
         "\"rebind-timer\": 2000,"
         "\"renew-timer\": 1000,"
         "\"option-data\": [ {"
-        "    \"name\": \"option_foo\","
-        "    \"code\": 100,"
+        "    \"name\": \"subscriber-id\","
+        "    \"space\": \"dhcp6\","
+        "    \"code\": 38,"
         "    \"data\": \"AB CDEF0105\","
         "    \"csv-format\": False"
         " },"
         " {"
-        "    \"name\": \"option_foo2\","
-        "    \"code\": 101,"
+        "    \"name\": \"preference\","
+        "    \"space\": \"dhcp6\","
+        "    \"code\": 7,"
         "    \"data\": \"01\","
-        "    \"csv-format\": False"
+        "    \"csv-format\": True"
         " } ],"
         "\"subnet6\": [ { "
         "    \"pool\": [ \"2001:db8:1::/80\" ],"
@@ -474,31 +855,101 @@ TEST_F(Dhcp6ParserTest, optionDataDefaults) {
     // code so we get the range.
     std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
               Subnet::OptionContainerTypeIndex::const_iterator> range =
-        idx.equal_range(100);
-    // Expect single option with the code equal to 100.
+        idx.equal_range(D6O_SUBSCRIBER_ID);
+    // Expect single option with the code equal to 38.
     ASSERT_EQ(1, std::distance(range.first, range.second));
-    const uint8_t foo_expected[] = {
+    const uint8_t subid_expected[] = {
         0xAB, 0xCD, 0xEF, 0x01, 0x05
     };
     // Check if option is valid in terms of code and carried data.
-    testOption(*range.first, 100, foo_expected, sizeof(foo_expected));
+    testOption(*range.first, D6O_SUBSCRIBER_ID, subid_expected,
+               sizeof(subid_expected));
 
-    range = idx.equal_range(101);
+    range = idx.equal_range(D6O_PREFERENCE);
     ASSERT_EQ(1, std::distance(range.first, range.second));
     // Do another round of testing with second option.
-    const uint8_t foo2_expected[] = {
+    const uint8_t pref_expected[] = {
         0x01
     };
-    testOption(*range.first, 101, foo2_expected, sizeof(foo2_expected));
+    testOption(*range.first, D6O_PREFERENCE, pref_expected,
+               sizeof(pref_expected));
 
     // Check that options with other option codes are not returned.
-    for (uint16_t code = 102; code < 110; ++code) {
+    for (uint16_t code = 47; code < 57; ++code) {
         range = idx.equal_range(code);
         EXPECT_EQ(0, std::distance(range.first, range.second));
     }
 }
 
-// Goal of this test is to verify options configuration
+/// The goal of this test is to verify that two options having the same
+/// option code can be added to different option spaces.
+TEST_F(Dhcp6ParserTest, optionDataTwoSpaces) {
+
+    // This configuration string is to configure two options
+    // sharing the code 56 and having different definitions
+    // and belonging to the different option spaces.
+    // The option definition must be provided for the
+    // option that belongs to the 'isc' option space.
+    // The definition is not required for the option that
+    // belongs to the 'dhcp6' option space as it is the
+    // standard option.
+    string config = "{ \"interface\": [ \"all\" ],"
+        "\"rebind-timer\": 2000,"
+        "\"renew-timer\": 1000,"
+        "\"option-data\": [ {"
+        "    \"name\": \"subscriber-id\","
+        "    \"space\": \"dhcp6\","
+        "    \"code\": 38,"
+        "    \"data\": \"AB CDEF0105\","
+        "    \"csv-format\": False"
+        " },"
+        " {"
+        "    \"name\": \"foo\","
+        "    \"space\": \"isc\","
+        "    \"code\": 38,"
+        "    \"data\": \"1234\","
+        "    \"csv-format\": True"
+        " } ],"
+        "\"option-def\": [ {"
+        "    \"name\": \"foo\","
+        "    \"code\": 38,"
+        "    \"type\": \"uint32\","
+        "    \"array\": False,"
+        "    \"record-types\": \"\","
+        "    \"space\": \"isc\""
+        " } ],"
+        "\"subnet6\": [ { "
+        "    \"pool\": [ \"2001:db8:1::/80\" ],"
+        "    \"subnet\": \"2001:db8:1::/64\""
+        " } ]"
+        "}";
+
+    ConstElementPtr status;
+
+    ElementPtr json = Element::fromJSON(config);
+
+    EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
+    ASSERT_TRUE(status);
+    checkResult(status, 0);
+
+    // Options should be now availabe for the subnet.
+    Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"));
+    ASSERT_TRUE(subnet);
+    // Try to get the option from the space dhcp6.
+    Subnet::OptionDescriptor desc1 = subnet->getOptionDescriptor("dhcp6", 38);
+    ASSERT_TRUE(desc1.option);
+    EXPECT_EQ(38, desc1.option->getType());
+    // Try to get the option from the space isc.
+    Subnet::OptionDescriptor desc2 = subnet->getOptionDescriptor("isc", 38);
+    ASSERT_TRUE(desc2.option);
+    EXPECT_EQ(38, desc1.option->getType());
+    // Try to get the non-existing option from the non-existing
+    // option space and  expect that option is not returned.
+    Subnet::OptionDescriptor desc3 = subnet->getOptionDescriptor("non-existing", 38);
+    ASSERT_FALSE(desc3.option);
+}
+
+// The goal of this test is to verify options configuration
 // for a single subnet. In particular this test checks
 // that local options configuration overrides global
 // option setting.
@@ -509,8 +960,9 @@ TEST_F(Dhcp6ParserTest, optionDataInSingleSubnet) {
         "\"rebind-timer\": 2000, "
         "\"renew-timer\": 1000, "
         "\"option-data\": [ {"
-        "      \"name\": \"option_foo\","
-        "      \"code\": 100,"
+        "      \"name\": \"subscriber-id\","
+        "      \"space\": \"dhcp6\","
+        "      \"code\": 38,"
         "      \"data\": \"AB\","
         "      \"csv-format\": False"
         " } ],"
@@ -518,14 +970,16 @@ TEST_F(Dhcp6ParserTest, optionDataInSingleSubnet) {
         "    \"pool\": [ \"2001:db8:1::/80\" ],"
         "    \"subnet\": \"2001:db8:1::/64\", "
         "    \"option-data\": [ {"
-        "          \"name\": \"option_foo\","
-        "          \"code\": 100,"
+        "          \"name\": \"subscriber-id\","
+        "          \"space\": \"dhcp6\","
+        "          \"code\": 38,"
         "          \"data\": \"AB CDEF0105\","
         "          \"csv-format\": False"
         "        },"
         "        {"
-        "          \"name\": \"option_foo2\","
-        "          \"code\": 101,"
+        "          \"name\": \"preference\","
+        "          \"space\": \"dhcp6\","
+        "          \"code\": 7,"
         "          \"data\": \"01\","
         "          \"csv-format\": False"
         "        } ]"
@@ -552,22 +1006,24 @@ TEST_F(Dhcp6ParserTest, optionDataInSingleSubnet) {
     // code so we get the range.
     std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
               Subnet::OptionContainerTypeIndex::const_iterator> range =
-        idx.equal_range(100);
-    // Expect single option with the code equal to 100.
+        idx.equal_range(D6O_SUBSCRIBER_ID);
+    // Expect single option with the code equal to 38.
     ASSERT_EQ(1, std::distance(range.first, range.second));
-    const uint8_t foo_expected[] = {
+    const uint8_t subid_expected[] = {
         0xAB, 0xCD, 0xEF, 0x01, 0x05
     };
     // Check if option is valid in terms of code and carried data.
-    testOption(*range.first, 100, foo_expected, sizeof(foo_expected));
+    testOption(*range.first, D6O_SUBSCRIBER_ID, subid_expected,
+               sizeof(subid_expected));
 
-    range = idx.equal_range(101);
+    range = idx.equal_range(D6O_PREFERENCE);
     ASSERT_EQ(1, std::distance(range.first, range.second));
     // Do another round of testing with second option.
-    const uint8_t foo2_expected[] = {
+    const uint8_t pref_expected[] = {
         0x01
     };
-    testOption(*range.first, 101, foo2_expected, sizeof(foo2_expected));
+    testOption(*range.first, D6O_PREFERENCE, pref_expected,
+               sizeof(pref_expected));
 }
 
 // Goal of this test is to verify options configuration
@@ -582,8 +1038,9 @@ TEST_F(Dhcp6ParserTest, optionDataInMultipleSubnets) {
         "    \"pool\": [ \"2001:db8:1::/80\" ],"
         "    \"subnet\": \"2001:db8:1::/64\", "
         "    \"option-data\": [ {"
-        "          \"name\": \"option_foo\","
-        "          \"code\": 100,"
+        "          \"name\": \"subscriber-id\","
+        "          \"space\": \"dhcp6\","
+        "          \"code\": 38,"
         "          \"data\": \"0102030405060708090A\","
         "          \"csv-format\": False"
         "        } ]"
@@ -592,8 +1049,9 @@ TEST_F(Dhcp6ParserTest, optionDataInMultipleSubnets) {
         "    \"pool\": [ \"2001:db8:2::/80\" ],"
         "    \"subnet\": \"2001:db8:2::/64\", "
         "    \"option-data\": [ {"
-        "          \"name\": \"option_foo2\","
-        "          \"code\": 101,"
+        "          \"name\": \"user-class\","
+        "          \"space\": \"dhcp6\","
+        "          \"code\": 15,"
         "          \"data\": \"FFFEFDFCFB\","
         "          \"csv-format\": False"
         "        } ]"
@@ -620,15 +1078,16 @@ TEST_F(Dhcp6ParserTest, optionDataInMultipleSubnets) {
     // code so we get the range.
     std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
               Subnet::OptionContainerTypeIndex::const_iterator> range1 =
-        idx1.equal_range(100);
-    // Expect single option with the code equal to 100.
+        idx1.equal_range(D6O_SUBSCRIBER_ID);
+    // Expect single option with the code equal to 38.
     ASSERT_EQ(1, std::distance(range1.first, range1.second));
-    const uint8_t foo_expected[] = {
+    const uint8_t subid_expected[] = {
         0x01, 0x02, 0x03, 0x04, 0x05,
         0x06, 0x07, 0x08, 0x09, 0x0A
     };
     // Check if option is valid in terms of code and carried data.
-    testOption(*range1.first, 100, foo_expected, sizeof(foo_expected));
+    testOption(*range1.first, D6O_SUBSCRIBER_ID, subid_expected,
+               sizeof(subid_expected));
 
     // Test another subnet in the same way.
     Subnet6Ptr subnet2 = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:2::4"));
@@ -639,13 +1098,14 @@ TEST_F(Dhcp6ParserTest, optionDataInMultipleSubnets) {
     const Subnet::OptionContainerTypeIndex& idx2 = options2->get<1>();
     std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
               Subnet::OptionContainerTypeIndex::const_iterator> range2 =
-        idx2.equal_range(101);
+        idx2.equal_range(D6O_USER_CLASS);
     ASSERT_EQ(1, std::distance(range2.first, range2.second));
 
-    const uint8_t foo2_expected[] = {
+    const uint8_t user_class_expected[] = {
         0xFF, 0xFE, 0xFD, 0xFC, 0xFB
     };
-    testOption(*range2.first, 101, foo2_expected, sizeof(foo2_expected));
+    testOption(*range2.first, D6O_USER_CLASS, user_class_expected,
+               sizeof(user_class_expected));
 }
 
 // Verify that empty option name is rejected in the configuration.
@@ -738,14 +1198,15 @@ TEST_F(Dhcp6ParserTest, optionDataLowerCase) {
     // code so we get the range.
     std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
               Subnet::OptionContainerTypeIndex::const_iterator> range =
-        idx.equal_range(80);
-    // Expect single option with the code equal to 100.
+        idx.equal_range(D6O_SUBSCRIBER_ID);
+    // Expect single option with the code equal to 38.
     ASSERT_EQ(1, std::distance(range.first, range.second));
-    const uint8_t foo_expected[] = {
+    const uint8_t subid_expected[] = {
         0x0A, 0x0B, 0x0C, 0x0D
     };
     // Check if option is valid in terms of code and carried data.
-    testOption(*range.first, 80, foo_expected, sizeof(foo_expected));
+    testOption(*range.first, D6O_SUBSCRIBER_ID, subid_expected,
+               sizeof(subid_expected));
 }
 
 // Verify that specific option object is returned for standard
@@ -753,7 +1214,8 @@ TEST_F(Dhcp6ParserTest, optionDataLowerCase) {
 TEST_F(Dhcp6ParserTest, stdOptionData) {
     ConstElementPtr x;
     std::map<std::string, std::string> params;
-    params["name"] = "OPTION_IA_NA";
+    params["name"] = "ia-na";
+    params["space"] = "dhcp6";
     // Option code 3 means OPTION_IA_NA.
     params["code"] = "3";
     params["data"] = "12345, 6789, 1516";

src/bin/dhcp6/tests/dhcp6_srv_unittest.cc

@@ -370,14 +370,16 @@ TEST_F(Dhcpv6SrvTest, advertiseOptions) {
         "    \"pool\": [ \"2001:db8:1::/64\" ],"
         "    \"subnet\": \"2001:db8:1::/48\", "
         "    \"option-data\": [ {"
-        "          \"name\": \"OPTION_DNS_SERVERS\","
+        "          \"name\": \"dns-servers\","
+        "          \"space\": \"dhcp6\","
         "          \"code\": 23,"
         "          \"data\": \"2001:db8:1234:FFFF::1, 2001:db8:1234:FFFF::2\","
         "          \"csv-format\": True"
         "        },"
         "        {"
-        "          \"name\": \"OPTION_FOO\","
-        "          \"code\": 1000,"
+        "          \"name\": \"subscriber-id\","
+        "          \"space\": \"dhcp6\","
+        "          \"code\": 38,"
         "          \"data\": \"1234\","
         "          \"csv-format\": False"
         "        } ]"
@@ -406,18 +408,18 @@ TEST_F(Dhcpv6SrvTest, advertiseOptions) {
     // check if we get response at all
     ASSERT_TRUE(adv);
 
-    // We have not requested option with code 1000 so it should not
+    // We have not requested any options so they should not
     // be included in the response.
-    ASSERT_FALSE(adv->getOption(1000));
+    ASSERT_FALSE(adv->getOption(D6O_SUBSCRIBER_ID));
     ASSERT_FALSE(adv->getOption(D6O_NAME_SERVERS));
 
-    // Let's now request option with code 1000.
-    // We expect that server will include this option in its reply.
+    // Let's now request some options. We expect that the server
+    // will include them in its response.
     boost::shared_ptr<OptionIntArray<uint16_t> >
         option_oro(new OptionIntArray<uint16_t>(Option::V6, D6O_ORO));
     // Create vector with two option codes.
     std::vector<uint16_t> codes(2);
-    codes[0] = 1000;
+    codes[0] = D6O_SUBSCRIBER_ID;
     codes[1] = D6O_NAME_SERVERS;
     // Pass this code to option.
     option_oro->setValues(codes);
@@ -442,7 +444,7 @@ TEST_F(Dhcpv6SrvTest, advertiseOptions) {
 
     // There is a dummy option with code 1000 we requested from a server.
     // Expect that this option is in server's response.
-    tmp = adv->getOption(1000);
+    tmp = adv->getOption(D6O_SUBSCRIBER_ID);
     ASSERT_TRUE(tmp);
 
     // Check that the option contains valid data (from configuration).

src/lib/dhcp/option_definition.cc

@@ -23,6 +23,8 @@
 #include <dhcp/option_int_array.h>
 #include <util/encode/hex.h>
 #include <util/strutil.h>
+#include <boost/algorithm/string/classification.hpp>
+#include <boost/algorithm/string/predicate.hpp>
 
 using namespace std;
 using namespace isc::util;
@@ -207,16 +209,29 @@ OptionDefinition::sanityCheckUniverse(const Option::Universe expected_universe,
 
 void
 OptionDefinition::validate() const {
+
+    using namespace boost::algorithm;
+
     std::ostringstream err_str;
-    if (name_.empty()) {
-        // Option name must not be empty.
-        err_str << "option name must not be empty.";
-    } else if (name_.find(" ") != string::npos) {
-        // Option name must not contain spaces.
-        err_str << "option name must not contain spaces.";
+
+    // Allowed characters in the option name are: lower or
+    // upper case letters, digits, underscores and hyphens.
+    // Empty option spaces are not allowed.
+    if (!all(name_, boost::is_from_range('a', 'z') ||
+             boost::is_from_range('A', 'Z') ||
+             boost::is_digit() ||
+             boost::is_any_of(std::string("-_"))) ||
+        name_.empty() ||
+        // Hyphens and underscores are not allowed at the beginning
+        // and at the end of the option name.
+        all(find_head(name_, 1), boost::is_any_of(std::string("-_"))) ||
+        all(find_tail(name_, 1), boost::is_any_of(std::string("-_")))) {
+        err_str << "invalid option name '" << name_ << "'";
+
     } else if (type_ >= OPT_UNKNOWN_TYPE) {
         // Option definition must be of a known type.
         err_str << "option type value " << type_ << " is out of range.";
+
     } else if (array_type_) {
         if (type_ == OPT_STRING_TYPE) {
             // Array of strings is not allowed because there is no way
@@ -225,9 +240,12 @@ OptionDefinition::validate() const {
             err_str << "array of strings is not a valid option definition.";
         } else if (type_ == OPT_BINARY_TYPE) {
             err_str << "array of binary values is not a valid option definition.";
+
         } else if (type_ == OPT_EMPTY_TYPE) {
             err_str << "array of empty value is not a valid option definition.";
+
         }
+
     } else if (type_ == OPT_RECORD_TYPE) {
         // At least two data fields should be added to the record. Otherwise
         // non-record option definition could be used.
@@ -235,6 +253,7 @@ OptionDefinition::validate() const {
             err_str << "invalid number of data fields: " << getRecordFields().size()
                     << " specified for the option of type 'record'. Expected at"
                     << " least 2 fields.";
+
         } else {
             // If the number of fields is valid we have to check if their order
             // is valid too. We check that string or binary data fields are not

src/lib/dhcp/tests/option_definition_unittest.cc

@@ -164,29 +164,55 @@ TEST_F(OptionDefinitionTest, validate) {
     EXPECT_THROW(opt_def5.validate(), MalformedOptionDefinition);
 
     // Option name must not contain spaces.
-    OptionDefinition opt_def6("OPTION CLIENTID", D6O_CLIENTID, "string", true);
+    OptionDefinition opt_def6("OPTION CLIENTID", D6O_CLIENTID, "string");
     EXPECT_THROW(opt_def6.validate(), MalformedOptionDefinition);
 
+    // Option name may contain lower case letters.
+    OptionDefinition opt_def7("option_clientid", D6O_CLIENTID, "string");
+    EXPECT_NO_THROW(opt_def7.validate());
+
+    // Using digits in option name is legal.
+    OptionDefinition opt_def8("option_123", D6O_CLIENTID, "string");
+    EXPECT_NO_THROW(opt_def8.validate());
+
+    // Using hyphen is legal.
+    OptionDefinition opt_def9("option-clientid", D6O_CLIENTID, "string");
+    EXPECT_NO_THROW(opt_def9.validate());
+
+    // Using hyphen or undescore at the beginning or at the end
+    // of the option name is not allowed.
+    OptionDefinition opt_def10("-option-clientid", D6O_CLIENTID, "string");
+    EXPECT_THROW(opt_def10.validate(), MalformedOptionDefinition);
+
+    OptionDefinition opt_def11("_option-clientid", D6O_CLIENTID, "string");
+    EXPECT_THROW(opt_def11.validate(), MalformedOptionDefinition);
+
+    OptionDefinition opt_def12("option-clientid_", D6O_CLIENTID, "string");
+    EXPECT_THROW(opt_def12.validate(), MalformedOptionDefinition);
+
+    OptionDefinition opt_def13("option-clientid-", D6O_CLIENTID, "string");
+    EXPECT_THROW(opt_def13.validate(), MalformedOptionDefinition);
+
     // Having array of strings does not make sense because there is no way
     // to determine string's length.
-    OptionDefinition opt_def7("OPTION_CLIENTID", D6O_CLIENTID, "string", true);
-    EXPECT_THROW(opt_def7.validate(), MalformedOptionDefinition);
+    OptionDefinition opt_def14("OPTION_CLIENTID", D6O_CLIENTID, "string", true);
+    EXPECT_THROW(opt_def14.validate(), MalformedOptionDefinition);
 
     // It does not make sense to have string field within the record before
     // other fields because there is no way to determine the length of this
     // string and thus there is no way to determine where the other field
     // begins.
-    OptionDefinition opt_def8("OPTION_STATUS_CODE", D6O_STATUS_CODE,
-                              "record");
-    opt_def8.addRecordField("string");
-    opt_def8.addRecordField("uint16");
-    EXPECT_THROW(opt_def8.validate(), MalformedOptionDefinition);
+    OptionDefinition opt_def15("OPTION_STATUS_CODE", D6O_STATUS_CODE,
+                               "record");
+    opt_def15.addRecordField("string");
+    opt_def15.addRecordField("uint16");
+    EXPECT_THROW(opt_def15.validate(), MalformedOptionDefinition);
 
     // ... but it is ok if the string value is the last one.
-    OptionDefinition opt_def9("OPTION_STATUS_CODE", D6O_STATUS_CODE,
-                              "record");
-    opt_def9.addRecordField("uint8");
-    opt_def9.addRecordField("string");
+    OptionDefinition opt_def16("OPTION_STATUS_CODE", D6O_STATUS_CODE,
+                               "record");
+    opt_def16.addRecordField("uint8");
+    opt_def16.addRecordField("string");
 }
 
 

src/lib/dhcpsrv/Makefile.am

@@ -43,6 +43,7 @@ if HAVE_MYSQL
 libb10_dhcpsrv_la_SOURCES += mysql_lease_mgr.cc mysql_lease_mgr.h
 endif
 libb10_dhcpsrv_la_SOURCES += option_space.cc option_space.h
+libb10_dhcpsrv_la_SOURCES += option_space_container.h
 libb10_dhcpsrv_la_SOURCES += pool.cc pool.h
 libb10_dhcpsrv_la_SOURCES += subnet.cc subnet.h
 libb10_dhcpsrv_la_SOURCES += triplet.h

src/lib/dhcpsrv/cfgmgr.cc

@@ -86,30 +86,15 @@ CfgMgr::addOptionDef(const OptionDefinitionPtr& def,
                   << option_space << "'.");
 
     }
-    // Get existing option definitions for the option space.
-    OptionDefContainerPtr defs = getOptionDefs(option_space);
-    // getOptionDefs always returns a valid pointer to
-    // the container. Let's make an assert to make sure.
-    assert(defs);
-    // Actually add the new definition.
-    defs->push_back(def);
-    option_def_spaces_[option_space] = defs;
+    // 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.
 
-    // Get all option definitions for the particular option space.
-    const OptionDefsMap::const_iterator& defs =
-        option_def_spaces_.find(option_space);
-    // If there are no option definitions for the particular option space
-    // then return empty container.
-    if (defs == option_def_spaces_.end()) {
-        return (OptionDefContainerPtr(new OptionDefContainer()));
-    }
-    // If option definitions found, return them.
-    return (defs->second);
+    return (option_def_spaces_.getItems(option_space));
 }
 
 OptionDefinitionPtr
@@ -229,7 +214,7 @@ void CfgMgr::addSubnet4(const Subnet4Ptr& subnet) {
 }
 
 void CfgMgr::deleteOptionDefs() {
-    option_def_spaces_.clear();
+    option_def_spaces_.clearItems();
 }
 
 void CfgMgr::deleteSubnets4() {

src/lib/dhcpsrv/cfgmgr.h

@@ -19,6 +19,7 @@
 #include <dhcp/option.h>
 #include <dhcp/option_definition.h>
 #include <dhcpsrv/option_space.h>
+#include <dhcpsrv/option_space_container.h>
 #include <dhcpsrv/pool.h>
 #include <dhcpsrv/subnet.h>
 #include <util/buffer.h>
@@ -66,6 +67,7 @@ namespace dhcp {
 /// Parameter inheritance is likely to be implemented in configuration handling
 /// routines, so there is no storage capability in a global scope for
 /// subnet-specific parameters.
+///
 /// @todo: Implement Subnet4 support (ticket #2237)
 /// @todo: Implement option definition support
 /// @todo: Implement parameter inheritance
@@ -250,15 +252,12 @@ protected:
 
 private:
 
-    /// A map containing option definitions for various option spaces.
-    /// They key of this map is the name of the option space. The
-    /// value is the the option container holding option definitions
-    /// for the particular option space.
-    typedef std::map<std::string, OptionDefContainerPtr> OptionDefsMap;
-
-    /// A map containing option definitions for different option spaces.
-    /// The map key holds an option space name.
-    OptionDefsMap option_def_spaces_;
+    /// @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> option_def_spaces_;
 
     /// @brief Container for defined DHCPv6 option spaces.
     OptionSpaceCollection spaces6_;

src/lib/dhcpsrv/dhcp_config_parser.h

@@ -18,6 +18,20 @@
 namespace isc {
 namespace dhcp {
 
+/// An exception that is thrown if an error occurs while configuring
+/// DHCP server.
+class DhcpConfigError : public isc::Exception {
+public:
+
+    /// @brief constructor
+    ///
+    /// @param file name of the file, where exception occurred
+    /// @param line line of the file, where exception occurred
+    /// @param what text description of the issue that caused exception
+    DhcpConfigError(const char* file, size_t line, const char* what)
+        : isc::Exception(file, line, what) {}
+};
+
 /// @brief Forward declaration to DhcpConfigParser class.
 ///
 /// It is only needed here to define types that are
@@ -105,6 +119,34 @@ public:
     /// This method is expected to be called after @c build(), and only once.
     /// The result is undefined otherwise.
     virtual void commit() = 0;
+
+protected:
+
+    /// @brief Return the parsed entry from the provided storage.
+    ///
+    /// This method returns the parsed entry from the provided
+    /// storage. If the entry is not found, then exception is
+    /// thrown.
+    ///
+    /// @param param_id name of the configuration entry.
+    /// @param storage storage where the entry should be searched.
+    /// @tparam ReturnType type of the returned value.
+    /// @tparam StorageType type of the storage.
+    ///
+    /// @throw DhcpConfigError if the entry has not been found
+    /// in the storage.
+    template<typename ReturnType, typename StorageType>
+    static ReturnType getParam(const std::string& param_id,
+                        const StorageType& storage) {
+        typename StorageType::const_iterator param = storage.find(param_id);
+        if (param == storage.end()) {
+            isc_throw(DhcpConfigError, "missing parameter '"
+                      << param_id << "'");
+        }
+        ReturnType value = param->second;
+        return (value);
+    }
+
 };
 
 

src/lib/dhcpsrv/option_space_container.h

@@ -0,0 +1,102 @@
+// Copyright (C) 2013 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 OPTION_SPACE_CONTAINER_H
+#define OPTION_SPACE_CONTAINER_H
+
+#include <list>
+#include <string>
+
+namespace isc {
+namespace dhcp {
+
+/// @brief Simple container for option spaces holding various items.
+///
+/// This helper class is used to store items of various types in
+/// that are grouped by option space names. Each option space is
+/// mapped to a container that holds items which specifically can
+/// be OptionDefinition objects or Subnet::OptionDescriptor structures.
+///
+/// @tparam ContainerType of the container holding items within
+/// option space.
+/// @tparam ItemType type of the item being held by the container.
+template<typename ContainerType, typename ItemType>
+class OptionSpaceContainer {
+public:
+
+    /// Pointer to the container.
+    typedef boost::shared_ptr<ContainerType> ItemsContainerPtr;
+
+    /// @brief Adds a new item to the option_space.
+    ///
+    /// @param item reference to the item being added.
+    /// @param name of the option space.
+    void addItem(const ItemType& item, const std::string& option_space) {
+        ItemsContainerPtr items = getItems(option_space);
+        items->push_back(item);
+        option_space_map_[option_space] = items;
+    }
+
+    /// @brief Get all items for the particular option space.
+    ///
+    /// @warning when there are no items for the specified option
+    /// space an empty container is created and returned. However
+    /// this container is not added to the list of option spaces.
+    ///
+    /// @param option_space name of the option space.
+    ///
+    /// @return pointer to the container holding items.
+    ItemsContainerPtr getItems(const std::string& option_space) const {
+        const typename OptionSpaceMap::const_iterator& items =
+            option_space_map_.find(option_space);
+        if (items == option_space_map_.end()) {
+            return (ItemsContainerPtr(new ContainerType()));
+        }
+        return (items->second);
+    }
+
+    /// @brief Get a list of existing option spaces.
+    ///
+    /// @return a list of option spaces.
+    ///
+    /// @todo This function is likely to be removed once
+    /// we create a structore of OptionSpaces defined
+    /// through the configuration manager.
+    std::list<std::string> getOptionSpaceNames() {
+        std::list<std::string> names;
+        for (typename OptionSpaceMap::const_iterator space =
+                 option_space_map_.begin();
+             space != option_space_map_.end(); ++space) {
+            names.push_back(space->first);
+        }
+        return (names);
+    }
+
+    /// @brief Remove all items from the container.
+    void clearItems() {
+        option_space_map_.clear();
+    }
+
+private:
+
+    /// A map holding container (option space name is the key).
+    typedef std::map<std::string, ItemsContainerPtr> OptionSpaceMap;
+    OptionSpaceMap option_space_map_;
+};
+
+
+} // end of isc::dhcp namespace
+} // end of isc namespace
+
+#endif // OPTION_SPACE_CONTAINER_H

src/lib/dhcpsrv/subnet.cc

@@ -55,36 +55,18 @@ Subnet::addOption(OptionPtr& option, bool persistent,
     }
     validateOption(option);
 
-    OptionContainerPtr container = getOptionDescriptors(option_space);
-    // getOptionDescriptors is expected to return the pointer to the
-    // valid container. Let's make sure it does by performing an assert.
-    assert(container);
-    // Actually add the new descriptor.
-    container->push_back(OptionDescriptor(option, persistent));
-    option_spaces_[option_space] = container;
+    // Actually add new option descriptor.
+    option_spaces_.addItem(OptionDescriptor(option, persistent), option_space);
 }
 
 void
 Subnet::delOptions() {
-    option_spaces_.clear();
+    option_spaces_.clearItems();
 }
 
 Subnet::OptionContainerPtr
 Subnet::getOptionDescriptors(const std::string& option_space) const {
-    // Search the map to get the options container for the particular
-    // option space.
-    const OptionSpacesPtr::const_iterator& options =
-        option_spaces_.find(option_space);
-    // If the option space has not been found it means that no option
-    // has been configured for this option space yet. Thus we have to
-    // return an empty container to the caller.
-    if (options == option_spaces_.end()) {
-        // The default constructor creates an empty container.
-        return (OptionContainerPtr(new OptionContainer()));
-    }
-    // We found some option container for the option space specified.
-    // Let's return a const reference to it.
-    return (options->second);
+    return (option_spaces_.getItems(option_space));
 }
 
 Subnet::OptionDescriptor

src/lib/dhcpsrv/subnet.h

@@ -24,6 +24,7 @@
 
 #include <asiolink/io_address.h>
 #include <dhcp/option.h>
+#include <dhcpsrv/option_space_container.h>
 #include <dhcpsrv/pool.h>
 #include <dhcpsrv/triplet.h>
 
@@ -417,12 +418,11 @@ protected:
 
 private:
 
-    /// Container holding options grouped by option space names.
-    typedef std::map<std::string, OptionContainerPtr> OptionSpacesPtr;
+    /// A collection of option spaces grouping option descriptors.
+    typedef OptionSpaceContainer<OptionContainer,
+                                 OptionDescriptor> OptionSpaceCollection;
+    OptionSpaceCollection option_spaces_;
 
-    /// @brief a collection of DHCP option spaces holding options
-    /// configured for a subnet.
-    OptionSpacesPtr option_spaces_;
 };
 
 /// @brief A generic pointer to either Subnet4 or Subnet6 object