[3589] Option data configuration parsers store parsed data into cfgmgr.

src/bin/dhcp4/json_config_parser.cc

@@ -42,72 +42,6 @@ using namespace isc::asiolink;
 
 namespace {
 
-/// @brief Parser for DHCP4 option data value.
-///
-/// This parser parses configuration entries that specify value of
-/// a single option specific to DHCP4.  It provides the DHCP4-specific
-/// implementation of the abstract class OptionDataParser.
-class Dhcp4OptionDataParser : public OptionDataParser {
-public:
-    /// @brief Constructor.
-    ///
-    /// @param dummy first param, option names are always "Dhcp4/option-data[n]"
-    /// @param options is the option storage in which to store the parsed option
-    /// upon "commit".
-    /// @param global_context is a pointer to the global context which
-    /// stores global scope parameters, options, option defintions.
-    Dhcp4OptionDataParser(const std::string&,
-        OptionStoragePtr options, ParserContextPtr global_context)
-        :OptionDataParser("", options, global_context) {
-    }
-
-    /// @brief static factory method for instantiating Dhcp4OptionDataParsers
-    ///
-    /// @param param_name name of the parameter to be parsed.
-    /// @param options storage where the parameter value is to be stored.
-    /// @param global_context is a pointer to the global context which
-    /// stores global scope parameters, options, option defintions.
-    /// @return returns a pointer to a new OptionDataParser. Caller is
-    /// is responsible for deleting it when it is no longer needed.
-    static OptionDataParser* factory(const std::string& param_name,
-        OptionStoragePtr options, ParserContextPtr global_context) {
-        return (new Dhcp4OptionDataParser(param_name, options, global_context));
-    }
-
-protected:
-    /// @brief Finds an option definition within the server's option space
-    ///
-    /// Given an option space and an option code, find the correpsonding
-    /// option defintion within the server's option defintion storage.
-    ///
-    /// @param option_space name of the parameter option space
-    /// @param option_code numeric value of the parameter to find
-    /// @return OptionDefintionPtr of the option defintion or an
-    /// empty OptionDefinitionPtr if not found.
-    /// @throw DhcpConfigError if the option space requested is not valid
-    /// for this server.
-    virtual OptionDefinitionPtr findServerSpaceOptionDefinition (
-                std::string& option_space, uint32_t option_code) {
-        OptionDefinitionPtr def;
-        if (option_space == "dhcp4" &&
-            LibDHCP::isStandardOption(Option::V4, option_code)) {
-            def = LibDHCP::getOptionDef(Option::V4, option_code);
-        } else if (option_space == "dhcp6") {
-            isc_throw(DhcpConfigError, "'dhcp6' option space name is reserved"
-                     << " for DHCPv6 server");
-        } else {
-            // Check if this is a vendor-option. If it is, get vendor-specific
-            // definition.
-            uint32_t vendor_id = CfgOption::optionSpaceToVendorId(option_space);
-            if (vendor_id) {
-                def = LibDHCP::getVendorOptionDef(Option::V4, vendor_id, option_code);
-            }
-        }
-
-        return (def);
-    }
-};
-
 /// @brief Parser for IPv4 pool definitions.
 ///
 /// This is the IPv4 derivation of the PoolParser class and handles pool
@@ -245,9 +179,7 @@ protected:
         } else if (config_id.compare("relay") == 0) {
             parser = new RelayInfoParser(config_id, relay_info_, Option::V4);
         } else if (config_id.compare("option-data") == 0) {
-           parser = new OptionDataListParser(config_id, options_,
-                                             global_context_,
-                                             Dhcp4OptionDataParser::factory);
+            parser = new OptionDataListParser(config_id, options_, AF_INET);
         } else {
             isc_throw(NotImplemented, "unsupported parameter: " << config_id);
         }
@@ -448,10 +380,7 @@ namespace dhcp {
     } else if (config_id.compare("subnet4") == 0) {
         parser = new Subnets4ListConfigParser(config_id);
     } else if (config_id.compare("option-data") == 0) {
-        parser = new OptionDataListParser(config_id,
-                                          globalContext()->options_,
-                                          globalContext(),
-                                          Dhcp4OptionDataParser::factory);
+        parser = new OptionDataListParser(config_id, CfgOptionPtr(), AF_INET);
     } else if (config_id.compare("option-def") == 0) {
         parser  = new OptionDefListParser(config_id, globalContext());
     } else if ((config_id.compare("version") == 0) ||

src/bin/dhcp6/json_config_parser.cc

@@ -56,73 +56,6 @@ typedef boost::shared_ptr<BooleanParser> BooleanParserPtr;
 typedef boost::shared_ptr<StringParser> StringParserPtr;
 typedef boost::shared_ptr<Uint32Parser> Uint32ParserPtr;
 
-/// @brief Parser for DHCP6 option data value.
-///
-/// This parser parses configuration entries that specify value of
-/// a single option specific to DHCP6.  It provides the DHCP6-specific
-/// implementation of the abstract class OptionDataParser.
-class Dhcp6OptionDataParser : public OptionDataParser {
-public:
-    /// @brief Constructor.
-    ///
-    /// @param dummy first param, option names are always "Dhcp6/option-data[n]"
-    /// @param options is the option storage in which to store the parsed option
-    /// upon "commit".
-    /// @param global_context is a pointer to the global context which
-    /// stores global scope parameters, options, option defintions.
-    Dhcp6OptionDataParser(const std::string&, OptionStoragePtr options,
-                         ParserContextPtr global_context)
-        :OptionDataParser("", options, global_context) {
-    }
-
-    /// @brief static factory method for instantiating Dhcp4OptionDataParsers
-    ///
-    /// @param param_name name of the parameter to be parsed.
-    /// @param options storage where the parameter value is to be stored.
-    /// @param global_context is a pointer to the global context which
-    /// stores global scope parameters, options, option defintions.
-    /// @return returns a pointer to a new OptionDataParser. Caller is
-    /// is responsible for deleting it when it is no longer needed.
-    static OptionDataParser* factory(const std::string& param_name,
-    OptionStoragePtr options, ParserContextPtr global_context) {
-        return (new Dhcp6OptionDataParser(param_name, options, global_context));
-    }
-
-
-protected:
-    /// @brief Finds an option definition within the server's option space
-    ///
-    /// Given an option space and an option code, find the correpsonding
-    /// option defintion within the server's option defintion storage.
-    ///
-    /// @param option_space name of the parameter option space
-    /// @param option_code numeric value of the parameter to find
-    /// @return OptionDefintionPtr of the option defintion or an
-    /// empty OptionDefinitionPtr if not found.
-    /// @throw DhcpConfigError if the option space requested is not valid
-    /// for this server.
-    virtual OptionDefinitionPtr findServerSpaceOptionDefinition (
-                            std::string& option_space, uint32_t option_code) {
-        OptionDefinitionPtr def;
-        if (option_space == "dhcp6" &&
-            LibDHCP::isStandardOption(Option::V6, option_code)) {
-            def = LibDHCP::getOptionDef(Option::V6, option_code);
-        } else if (option_space == "dhcp4") {
-            isc_throw(DhcpConfigError, "'dhcp4' option space name is reserved"
-                     << " for DHCPv4 server");
-        } else {
-            // Check if this is a vendor-option. If it is, get vendor-specific
-            // definition.
-            uint32_t vendor_id = CfgOption::optionSpaceToVendorId(option_space);
-            if (vendor_id) {
-                def = LibDHCP::getVendorOptionDef(Option::V6, vendor_id, option_code);
-            }
-        }
-
-        return (def);
-    }
-};
-
 /// @brief Parser for IPv6 pool definitions.
 ///
 /// This is the IPv6 derivation of the PoolParser class and handles pool
@@ -456,9 +389,7 @@ protected:
         } else if (config_id.compare("pd-pools") == 0) {
             parser = new PdPoolListParser(config_id, pools_);
         } else if (config_id.compare("option-data") == 0) {
-           parser = new OptionDataListParser(config_id, options_,
-                                             global_context_,
-                                             Dhcp6OptionDataParser::factory);
+            parser = new OptionDataListParser(config_id, options_, AF_INET6);
         } else {
             isc_throw(NotImplemented, "unsupported parameter: " << config_id);
         }
@@ -667,10 +598,7 @@ namespace dhcp {
     } else if (config_id.compare("subnet6") == 0) {
         parser = new Subnets6ListConfigParser(config_id);
     } else if (config_id.compare("option-data") == 0) {
-        parser = new OptionDataListParser(config_id,
-                                          globalContext()->options_,
-                                          globalContext(),
-                                          Dhcp6OptionDataParser::factory);
+        parser = new OptionDataListParser(config_id, CfgOptionPtr(), AF_INET6);
     } else if (config_id.compare("option-def") == 0) {
         parser  = new OptionDefListParser(config_id, globalContext());
     } else if (config_id.compare("version") == 0) {

src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -2741,7 +2741,11 @@ TEST_F(Dhcp6ParserTest, vendorOptionsCsv) {
 
 // The goal of this test is to verify that the standard option can
 // be configured to encapsulate multiple other options.
-TEST_F(Dhcp6ParserTest, stdOptionDataEncapsulate) {
+/// @todo This test is currently disabled because it relies on the option
+/// 17 which is treated differently than all other options. There are no
+/// other standard options used by Kea which would encapsulate other
+/// options and for which values could be configured here.
+TEST_F(Dhcp6ParserTest, DISABLED_stdOptionDataEncapsulate) {
 
     // The configuration is two stage process in this test.
     // In the first stahe we create definitions of suboptions

src/lib/dhcpsrv/cfg_option.cc

@@ -69,6 +69,33 @@ CfgOption::copy(CfgOption& other) const {
     other = new_cfg;
 }
 
+void
+CfgOption::encapsulate() {
+    // Append sub-options to the top level "dhcp4" option space.
+    encapsulateInternal(DHCP4_OPTION_SPACE);
+    // Append sub-options to the top level "dhcp6" option space.
+    encapsulateInternal(DHCP6_OPTION_SPACE);
+}
+
+void
+CfgOption::encapsulateInternal(const std::string& option_space) {
+    OptionContainerPtr options = getAll(option_space);
+    for (OptionContainer::const_iterator opt = options->begin();
+         opt != options->end(); ++opt) {
+        const std::string& encap_space = opt->option->getEncapsulatedSpace();
+        if (!encap_space.empty()) {
+            OptionContainerPtr encap_options = getAll(encap_space);
+            for (OptionContainer::const_iterator encap_opt =
+                     encap_options->begin(); encap_opt != encap_options->end();
+                 ++encap_opt) {
+                if (!opt->option->getOption(encap_opt->option->getType())) {
+                    opt->option->addOption(encap_opt->option);
+                }
+            }
+        }
+    }
+}
+
 template <typename Selector>
 void
 CfgOption::mergeInternal(const OptionSpaceContainer<OptionContainer,

src/lib/dhcpsrv/cfg_option.h

@@ -269,6 +269,14 @@ public:
     /// @param [out] other An object to copy the configuration to.
     void copy(CfgOption& other) const;
 
+    /// @brief Appends encapsulated options to top-level options.
+    ///
+    /// This method iterates over the top-level options (from "dhcp4"
+    /// and "dhcp6" option space) and checks which option spaces these
+    /// options encapsulate. For each encapsulated option space, the
+    /// options from this option space are appended to top-level options.
+    void encapsulate();
+
     /// @brief Returns all options for the specified option space.
     ///
     /// This method will not return vendor options, i.e. having option space
@@ -332,6 +340,12 @@ public:
 
 private:
 
+    /// @brief Appends encapsulated options to the options in an option space.
+    ///
+    /// @param option_space Name of the option space containing optionn to
+    /// which encapsulated options are appended.
+    void encapsulateInternal(const std::string& option_space);
+
     /// @brief Merges data from two option containers.
     ///
     /// This method merges options from one option container to another

src/lib/dhcpsrv/dhcp_parsers.cc

@@ -43,7 +43,6 @@ ParserContext::ParserContext(Option::Universe universe):
     boolean_values_(new BooleanStorage()),
     uint32_values_(new Uint32Storage()),
     string_values_(new StringStorage()),
-    options_(new OptionStorage()),
     hooks_libraries_(),
     universe_(universe)
 {
@@ -53,7 +52,6 @@ ParserContext::ParserContext(const ParserContext& rhs):
     boolean_values_(),
     uint32_values_(),
     string_values_(),
-    options_(),
     hooks_libraries_(),
     universe_(rhs.universe_)
 {
@@ -77,7 +75,6 @@ ParserContext::copyContext(const ParserContext& ctx) {
     copyContextPointer(ctx.boolean_values_, boolean_values_);
     copyContextPointer(ctx.uint32_values_, uint32_values_);
     copyContextPointer(ctx.string_values_, string_values_);
-    copyContextPointer(ctx.options_, options_);
     copyContextPointer(ctx.hooks_libraries_, hooks_libraries_);
     // Copy universe.
     universe_ = ctx.universe_;
@@ -281,20 +278,16 @@ HooksLibrariesParser::getLibraries(std::vector<std::string>& libraries,
 }
 
 // **************************** OptionDataParser *************************
-OptionDataParser::OptionDataParser(const std::string&, OptionStoragePtr options,
-                                  ParserContextPtr global_context)
+OptionDataParser::OptionDataParser(const std::string&, const CfgOptionPtr& cfg,
+                                   const uint16_t address_family)
     : boolean_values_(new BooleanStorage()),
-    string_values_(new StringStorage()), uint32_values_(new Uint32Storage()),
-    options_(options), option_descriptor_(false),
-    global_context_(global_context) {
-    if (!options_) {
-        isc_throw(isc::dhcp::DhcpConfigError, "parser logic error: "
-             << "options storage may not be NULL");
-    }
-
-    if (!global_context_) {
-        isc_throw(isc::dhcp::DhcpConfigError, "parser logic error: "
-             << "context may may not be NULL");
+      string_values_(new StringStorage()), uint32_values_(new Uint32Storage()),
+      option_descriptor_(false), cfg_(cfg),
+      address_family_(address_family) {
+    // If configuration not specified, then it is a global configuration
+    // scope.
+    if (!cfg_) {
+        cfg_ = CfgMgr::instance().getStagingCfg()->getCfgOption();
     }
 }
 
@@ -333,39 +326,57 @@ OptionDataParser::build(ConstElementPtr option_data_entries) {
 
     // Try to create the option instance.
     createOption(option_data_entries);
-}
 
-void
-OptionDataParser::commit() {
     if (!option_descriptor_.option) {
-        // Before we can commit the new option should be configured. If it is
-        // not than somebody must have called commit() before build().
         isc_throw(isc::InvalidOperation,
             "parser logic error: no option has been configured and"
             " thus there is nothing to commit. Has build() been called?");
     }
 
-    uint16_t opt_type = option_descriptor_.option->getType();
-    OptionContainerPtr options = options_->getItems(option_space_);
-    // The getItems() should never return NULL pointer. If there are no
-    // options configured for the particular option space a pointer
-    // to an empty container should be returned.
-    assert(options);
-    OptionContainerTypeIndex& idx = options->get<1>();
-    // Try to find options with the particular option code in the main
-    // storage. If found, remove these options because they will be
-    // replaced with new one.
-    OptionContainerTypeRange range = idx.equal_range(opt_type);
-    if (std::distance(range.first, range.second) > 0) {
-        idx.erase(range.first, range.second);
+    cfg_->add(option_descriptor_.option, option_descriptor_.persistent,
+              option_space_);
+}
+
+void
+OptionDataParser::commit() {
+    // Does nothing
+}
+
+OptionDefinitionPtr
+OptionDataParser::findServerSpaceOptionDefinition(const std::string& option_space,
+                                                  const uint32_t option_code) const {
+    const Option::Universe u = address_family_ == AF_INET ?
+        Option::V4 : Option::V6;
+
+    if ((option_space == DHCP4_OPTION_SPACE) && (u == Option::V6)) {
+        isc_throw(DhcpConfigError, "'" << DHCP4_OPTION_SPACE
+                  << "' option space name is reserved for DHCPv4 server");
+    } else if ((option_space == DHCP6_OPTION_SPACE) && (u == Option::V4)) {
+        isc_throw(DhcpConfigError, "'" << DHCP6_OPTION_SPACE
+                  << "' option space name is reserved for DHCPv6 server");
     }
 
-    // Append new option to the main storage.
-    options_->addItem(option_descriptor_, option_space_);
+    OptionDefinitionPtr def;
+    if (((option_space == DHCP4_OPTION_SPACE) || (option_space == DHCP6_OPTION_SPACE)) &&
+        LibDHCP::isStandardOption(u, option_code)) {
+        def = LibDHCP::getOptionDef(u, option_code);
+
+    } else {
+        // Check if this is a vendor-option. If it is, get vendor-specific
+        // definition.
+        uint32_t vendor_id = CfgOption::optionSpaceToVendorId(option_space);
+        if (vendor_id) {
+            def = LibDHCP::getVendorOptionDef(u, vendor_id, option_code);
+        }
+    }
+    return (def);
 }
 
+
 void
 OptionDataParser::createOption(ConstElementPtr option_data) {
+    const Option::Universe universe = address_family_ == AF_INET ?
+        Option::V4 : Option::V6;
     // Check if mandatory parameters are specified.
     uint32_t code;
     std::string name;
@@ -379,8 +390,8 @@ OptionDataParser::createOption(ConstElementPtr option_data) {
                   ex.what() << "(" << option_data->getPosition() << ")");
     }
     // Check parameters having default values.
-    std::string space = string_values_->getOptionalParam("space",
-              global_context_->universe_ == Option::V4 ? "dhcp4" : "dhcp6");
+    std::string space = string_values_->getOptionalParam("space", universe == Option::V4 ?
+                                                         "dhcp4" : "dhcp6");
     bool csv_format = boolean_values_->getOptionalParam("csv-format", false);
 
     // Option code is held in the uint32_t storage but is supposed to
@@ -391,14 +402,14 @@ OptionDataParser::createOption(ConstElementPtr option_data) {
         isc_throw(DhcpConfigError, "option code must not be zero "
                   "(" << uint32_values_->getPosition("code") << ")");
 
-    } else if (global_context_->universe_ == Option::V4 &&
+    } else if (universe == Option::V4 &&
                code > std::numeric_limits<uint8_t>::max()) {
         isc_throw(DhcpConfigError, "invalid option code '" << code
                 << "', it must not exceed '"
                   << static_cast<int>(std::numeric_limits<uint8_t>::max())
                   << "' (" << uint32_values_->getPosition("code") << ")");
 
-    } else if (global_context_->universe_ == Option::V6 &&
+    } else if (universe == Option::V6 &&
                code > std::numeric_limits<uint16_t>::max()) {
         isc_throw(DhcpConfigError, "invalid option code '" << code
                 << "', it must not exceed '"
@@ -443,18 +454,7 @@ OptionDataParser::createOption(ConstElementPtr option_data) {
         // need to search for its definition among user-configured
         // options. They are expected to be in the global storage
         // already.
-        OptionDefContainerPtr defs =
-            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
-        // to an empty container should be returned.
-        assert(defs);
-        const OptionDefContainerTypeIndex& idx = defs->get<1>();
-        OptionDefContainerTypeRange range = idx.equal_range(code);
-        if (std::distance(range.first, range.second) > 0) {
-            def = *range.first;
-        }
+        def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->get(space, code);
 
         // It's ok if we don't have option format if the option is
         // specified as hex
@@ -510,8 +510,8 @@ OptionDataParser::createOption(ConstElementPtr option_data) {
         // for all options.  Consequently an error will be issued if an option
         // definition does not exist for a particular option code. For now it is
         // ok to create generic option if definition does not exist.
-        OptionPtr option(new Option(global_context_->universe_,
-                        static_cast<uint16_t>(code), binary));
+        OptionPtr option(new Option(universe,
+                                    static_cast<uint16_t>(code), binary));
         // The created option is stored in option_descriptor_ class member
         // until the commit stage when it is inserted into the main storage.
         // If an option with the same code exists in main storage already the
@@ -537,13 +537,12 @@ OptionDataParser::createOption(ConstElementPtr option_data) {
         // an instance of our option.
         try {
             OptionPtr option = csv_format ?
-                def->optionFactory(global_context_->universe_,
-                                  code, data_tokens) :
-                def->optionFactory(global_context_->universe_,
-                                   code, binary);
+                def->optionFactory(universe, code, data_tokens) :
+                def->optionFactory(universe, code, binary);
             OptionDescriptor desc(option, false);
             option_descriptor_.option = option;
             option_descriptor_.persistent = false;
+
         } catch (const isc::Exception& ex) {
             isc_throw(DhcpConfigError, "option data does not match"
                       << " option definition (space: " << space
@@ -559,39 +558,18 @@ OptionDataParser::createOption(ConstElementPtr option_data) {
 
 // **************************** OptionDataListParser *************************
 OptionDataListParser::OptionDataListParser(const std::string&,
-    OptionStoragePtr options, ParserContextPtr global_context,
-    OptionDataParserFactory* optionDataParserFactory)
-    : options_(options), local_options_(new OptionStorage()),
-    global_context_(global_context),
-    optionDataParserFactory_(optionDataParserFactory) {
-    if (!options_) {
-        isc_throw(isc::dhcp::DhcpConfigError, "parser logic error: "
-             << "options storage may not be NULL");
-    }
-
-    if (!options_) {
-        isc_throw(isc::dhcp::DhcpConfigError, "parser logic error: "
-             << "context may not be NULL");
-    }
-
-    if (!optionDataParserFactory_) {
-        isc_throw(isc::dhcp::DhcpConfigError, "parser logic error: "
-             << "option data parser factory may not be NULL");
-    }
+                                           const CfgOptionPtr& cfg,
+                                           const uint16_t address_family)
+    : cfg_(cfg), address_family_(address_family) {
 }
 
 void
 OptionDataListParser::build(ConstElementPtr option_data_list) {
     BOOST_FOREACH(ConstElementPtr option_value, option_data_list->listValue()) {
         boost::shared_ptr<OptionDataParser>
-            parser((*optionDataParserFactory_)("option-data",
-                    local_options_, global_context_));
+            parser(new OptionDataParser("option-data", cfg_, address_family_));
 
-        // options_ member will hold instances of all options thus
-        // each OptionDataParser takes it as a storage.
-        // Build the instance of a single option.
         parser->build(option_value);
-        // Store a parser as it will be used to commit.
         parsers_.push_back(parser);
     }
 }
@@ -601,11 +579,6 @@ OptionDataListParser::commit() {
     BOOST_FOREACH(ParserPtr parser, parsers_) {
         parser->commit();
     }
-
-    // Parsing was successful and we have all configured
-    // options in local storage. We can now replace old values
-    // with new values.
-    std::swap(*local_options_, *options_);
 }
 
 // ******************************** OptionDefParser ****************************
@@ -977,15 +950,16 @@ SubnetConfigParser::SubnetConfigParser(const std::string&,
                                        ParserContextPtr global_context,
                                        const isc::asiolink::IOAddress& default_addr)
     : uint32_values_(new Uint32Storage()), string_values_(new StringStorage()),
-    pools_(new PoolStorage()), options_(new OptionStorage()),
-    global_context_(global_context),
-    relay_info_(new isc::dhcp::Subnet::RelayInfo(default_addr)) {
+      pools_(new PoolStorage()), global_context_(global_context),
+      relay_info_(new isc::dhcp::Subnet::RelayInfo(default_addr)),
+      options_(new CfgOption()) {
     // The first parameter should always be "subnet", but we don't check
     // against that here in case some wants to reuse this parser somewhere.
     if (!global_context_) {
         isc_throw(isc::dhcp::DhcpConfigError, "parser logic error: "
                  << "context storage may not be NULL");
     }
+
 }
 
 void
@@ -1028,61 +1002,6 @@ SubnetConfigParser::build(ConstElementPtr subnet) {
 }
 
 void
-SubnetConfigParser::appendSubOptions(const std::string& option_space,
-                                     OptionPtr& option) {
-    // Only non-NULL options are stored in option container.
-    // If this option pointer is NULL this is a serious error.
-    assert(option);
-
-    OptionDefinitionPtr def;
-    if (isServerStdOption(option_space, option->getType())) {
-        def = getServerStdOptionDefinition(option->getType());
-        // Definitions for some of the standard options hasn't been
-        // implemented so it is ok to leave here.
-        if (!def) {
-            return;
-        }
-    } else {
-        OptionDefContainerPtr defs = CfgMgr::instance().getStagingCfg()
-            ->getCfgOptionDef()->getAll(option_space);
-
-        const OptionDefContainerTypeIndex& idx = defs->get<1>();
-        const OptionDefContainerTypeRange& range =
-        idx.equal_range(option->getType());
-        // There is no definition so we have to leave.
-        if (std::distance(range.first, range.second) == 0) {
-            return;
-        }
-
-        def = *range.first;
-
-        // If the definition exists, it must be non-NULL.
-        // Otherwise it is a programming error.
-        assert(def);
-    }
-
-    // We need to get option definition for the particular option space
-    // and code. This definition holds the information whether our
-    // option encapsulates any option space.
-    // Get the encapsulated option space name.
-    std::string encapsulated_space = def->getEncapsulatedSpace();
-    // If option space name is empty it means that our option does not
-    // encapsulate any option space (does not include sub-options).
-    if (!encapsulated_space.empty()) {
-        // Get the sub-options that belong to the encapsulated
-        // option space.
-        const OptionContainerPtr sub_opts =
-                global_context_->options_->getItems(encapsulated_space);
-        // Append sub-options to the option.
-        BOOST_FOREACH(OptionDescriptor desc, *sub_opts) {
-            if (desc.option) {
-                option->addOption(desc.option);
-            }
-        }
-    }
-}
-
-void
 SubnetConfigParser::createSubnet() {
     std::string subnet_txt;
     try {
@@ -1146,64 +1065,12 @@ SubnetConfigParser::createSubnet() {
         subnet_->setIface(iface);
     }
 
-    // We are going to move configured options to the Subnet object.
-    // Configured options reside in the container where options
-    // are grouped by space names. Thus we need to get all space names
-    // and iterate over all options that belong to them.
-    std::list<std::string> space_names = options_->getOptionSpaceNames();
-    BOOST_FOREACH(std::string option_space, space_names) {
-        // Get all options within a particular option space.
-        BOOST_FOREACH(OptionDescriptor desc,
-                      *options_->getItems(option_space)) {
-            // The pointer should be non-NULL. The validation is expected
-            // to be performed by the OptionDataParser before adding an
-            // option descriptor to the container.
-            assert(desc.option);
-            // We want to check whether an option with the particular
-            // option code has been already added. If so, we want
-            // to issue a warning.
-            OptionDescriptor existing_desc =
-                subnet_->getCfgOption()->get("option_space", desc.option->getType());
-
-            if (existing_desc.option) {
-                duplicate_option_warning(desc.option->getType(), addr);
-            }
-            // Add sub-options (if any).
-            appendSubOptions(option_space, desc.option);
-
-            subnet_->getCfgOption()->add(desc.option, false, option_space);
-        }
-    }
-
-    // Check all global options and add them to the subnet object if
-    // they have been configured in the global scope. If they have been
-    // configured in the subnet scope we don't add global option because
-    // the one configured in the subnet scope always takes precedence.
-    space_names = global_context_->options_->getOptionSpaceNames();
-    BOOST_FOREACH(std::string option_space, space_names) {
-        // Get all global options for the particular option space.
-        BOOST_FOREACH(OptionDescriptor desc,
-                *(global_context_->options_->getItems(option_space))) {
-            // The pointer should be non-NULL. The validation is expected
-            // to be performed by the OptionDataParser before adding an
-            // option descriptor to the container.
-            assert(desc.option);
-            // Check if the particular option has been already added.
-            // This would mean that it has been configured in the
-            // subnet scope. Since option values configured in the
-            // subnet scope take precedence over globally configured
-            // values we don't add option from the global storage
-            // if there is one already.
-            OptionDescriptor existing_desc =  subnet_->getCfgOption()->
-                get(option_space, desc.option->getType());
-            if (!existing_desc.option) {
-                // Add sub-options (if any).
-                appendSubOptions(option_space, desc.option);
-
-                subnet_->getCfgOption()->add(desc.option, false, option_space);
-            }
-        }
-    }
+    // Merge globally defined options to the subnet specific options.
+    CfgMgr::instance().getStagingCfg()->getCfgOption()->merge(*options_);
+    // Copy all options to the subnet configuration.
+    options_->copy(*subnet_->getCfgOption());
+    // Append suboptions to the top-level options.
+    subnet_->getCfgOption()->encapsulate();
 }
 
 isc::dhcp::Triplet<uint32_t>

src/lib/dhcpsrv/dhcp_parsers.h

@@ -214,9 +214,6 @@ public:
     /// @brief Storage for string parameters.
     StringStoragePtr string_values_;
 
-    /// @brief Storage for options.
-    OptionStoragePtr options_;
-
     /// @brief Hooks libraries pointer.
     ///
     /// The hooks libraries information is a vector of strings, each containing
@@ -514,20 +511,19 @@ public:
     ///
     /// @param dummy first argument is ignored, all Parser constructors
     /// accept string as first argument.
-    /// @param options is the option storage in which to store the parsed option
-    /// upon "commit".
-    /// @param global_context is a pointer to the global context which
-    /// stores global scope parameters, options, option defintions.
+    /// @param [out] cfg Pointer to the configuration object where parsed option
+    /// should be stored or NULL if this is a global option.
+    /// @param address_family Address family: @c AF_INET or @c AF_INET6.
     /// @throw isc::dhcp::DhcpConfigError if options or global_context are null.
-    OptionDataParser(const std::string& dummy, OptionStoragePtr options,
-                    ParserContextPtr global_context);
+    OptionDataParser(const std::string& dummy, const CfgOptionPtr& cfg,
+                     const uint16_t address_family);
 
     /// @brief Parses the single option data.
     ///
     /// This method parses the data of a single option from the configuration.
     /// The option data includes option name, option code and data being
     /// carried by this option. Eventually it creates the instance of the
-    /// option.
+    /// option and adds it to the Configuration Manager.
     ///
     /// @param option_data_entries collection of entries that define value
     /// for a particular option.
@@ -537,15 +533,7 @@ public:
     /// calling build.
     virtual void build(isc::data::ConstElementPtr option_data_entries);
 
-    /// @brief Commits option value.
-    ///
-    /// This function adds a new option to the storage or replaces an existing
-    /// option with the same code.
-    ///
-    /// @throw isc::InvalidOperation if failed to set pointer to storage or
-    /// failed
-    /// to call build() prior to commit. If that happens data in the storage
-    /// remain un-modified.
+    /// @brief Does nothing.
     virtual void commit();
 
     /// @brief virtual destructor to ensure orderly destruction of derivations.
@@ -555,9 +543,7 @@ protected:
     /// @brief Finds an option definition within the server's option space
     ///
     /// Given an option space and an option code, find the correpsonding
-    /// option defintion within the server's option defintion storage. This
-    /// method is pure virtual requiring derivations to manage which option
-    /// space(s) is valid for search.
+    /// option defintion within the server's option defintion storage.
     ///
     /// @param option_space name of the parameter option space
     /// @param option_code numeric value of the parameter to find
@@ -565,8 +551,9 @@ protected:
     /// empty OptionDefinitionPtr if not found.
     /// @throw DhcpConfigError if the option space requested is not valid
     /// for this server.
-    virtual OptionDefinitionPtr findServerSpaceOptionDefinition (
-            std::string& option_space, uint32_t option_code) = 0;
+    virtual OptionDefinitionPtr
+    findServerSpaceOptionDefinition(const std::string& option_space,
+                                    const uint32_t option_code) const;
 
 private:
 
@@ -596,19 +583,18 @@ private:
     /// Storage for uint32 values (e.g. option code).
     Uint32StoragePtr uint32_values_;
 
-    /// Pointer to options storage. This storage is provided by
-    /// the calling class and is shared by all OptionDataParser objects.
-    OptionStoragePtr options_;
-
     /// Option descriptor holds newly configured option.
     OptionDescriptor option_descriptor_;
 
     /// Option space name where the option belongs to.
     std::string option_space_;
 
-    /// Parsing context which contains global values, options and option
-    /// definitions.
-    ParserContextPtr global_context_;
+    /// @brief Configuration holding option being parsed or NULL if the option
+    /// is global.
+    CfgOptionPtr cfg_;
+
+    /// @brief Address family: @c AF_INET or @c AF_INET6.
+    uint16_t address_family_;
 };
 
 ///@brief Function pointer for OptionDataParser factory methods
@@ -626,15 +612,11 @@ public:
     /// @brief Constructor.
     ///
     /// @param dummy nominally would be param name, this is always ignored.
-    /// @param options parsed option storage for options in this list
-    /// @param global_context is a pointer to the global context which
-    /// stores global scope parameters, options, option defintions.
-    /// @param optionDataParserFactory factory method for creating individual
-    /// option parsers
-    /// @throw isc::dhcp::DhcpConfigError if options or global_context are null.
-    OptionDataListParser(const std::string& dummy, OptionStoragePtr options,
-                        ParserContextPtr global_context,
-                        OptionDataParserFactory *optionDataParserFactory);
+    /// @param [out] cfg Pointer to the configuration object where options
+    /// should be stored or NULL if this is global option scope.
+    /// @param address_family Address family: @c AF_INET or AF_INET6
+    OptionDataListParser(const std::string& dummy, const CfgOptionPtr& cfg,
+                         const uint16_t address_family);
 
     /// @brief Parses entries that define options' data for a subnet.
     ///
@@ -651,24 +633,16 @@ public:
     void commit();
 
 private:
-    /// Pointer to options instances storage.
-    OptionStoragePtr options_;
-
-    /// Intermediate option storage. This storage is used by
-    /// lower level parsers to add new options.  Values held
-    /// in this storage are assigned to main storage (options_)
-    /// if overall parsing was successful.
-    OptionStoragePtr local_options_;
 
     /// Collection of parsers;
     ParserCollection parsers_;
 
-    /// Parsing context which contains global values, options and option
-    /// definitions.
-    ParserContextPtr global_context_;
+    /// @brief Pointer to a configuration where options are stored.
+    CfgOptionPtr cfg_;
+
+    /// @brief Address family: @c AF_INET or @c AF_INET6
+    uint16_t address_family_;
 
-    /// Factory to create server-specific option data parsers
-    OptionDataParserFactory *optionDataParserFactory_;
 };
 
 
@@ -962,15 +936,6 @@ public:
     /// @brief Adds the created subnet to a server's configuration.
     virtual void commit() = 0;
 
-    /// @brief tries to convert option_space string to numeric vendor_id
-    ///
-    /// This will work if the option_space has format "vendor-X", where
-    /// X can be any value between 1 and MAX_UINT32.
-    /// This is used to detect whether a given option-space is a vendor
-    /// space or not. Returns 0 if the format is different.
-    /// @return numeric vendor-id (or 0 if the format does not match)
-    static uint32_t optionSpaceToVendorId(const std::string& option_space);
-
 protected:
     /// @brief creates parsers for entries in subnet definition
     ///
@@ -1040,12 +1005,6 @@ protected:
 
 private:
 
-    /// @brief Append sub-options to an option.
-    ///
-    /// @param option_space a name of the encapsulated option space.
-    /// @param option option instance to append sub-options to.
-    void appendSubOptions(const std::string& option_space, OptionPtr& option);
-
     /// @brief Create a new subnet using a data from child parsers.
     ///
     /// @throw isc::dhcp::DhcpConfigError if subnet configuration parsing
@@ -1063,9 +1022,6 @@ protected:
     /// Storage for pools belonging to this subnet.
     PoolStoragePtr pools_;
 
-    /// Storage for options belonging to this subnet.
-    OptionStoragePtr options_;
-
     /// Parsers are stored here.
     ParserCollection parsers_;
 
@@ -1078,6 +1034,9 @@ protected:
 
     /// Pointer to relay information
     isc::dhcp::Subnet::RelayInfoPtr relay_info_;
+
+    /// Pointer to the options configuration.
+    CfgOptionPtr options_;
 };
 
 /// @brief Parser for  D2ClientConfig

src/lib/dhcpsrv/tests/cfg_option_unittest.cc

@@ -14,7 +14,10 @@
 
 #include <config.h>
 #include <dhcp/option.h>
+#include <dhcp/option_int.h>
+#include <dhcp/option_space.h>
 #include <dhcpsrv/cfg_option.h>
+#include <boost/pointer_cast.hpp>
 #include <gtest/gtest.h>
 
 using namespace isc;
@@ -239,6 +242,48 @@ TEST(CfgOptionTest, copy) {
     EXPECT_EQ(10, container->size());
 }
 
+// This test verifies that encapsulated options are added as sub-options
+// to the top level options on request.
+TEST(CfgOptionTest, encapsulate) {
+    CfgOption cfg;
+    // Create top-level options. These options encapsulate "foo" option space.
+    for (uint16_t code = 1000; code < 1020; ++code) {
+        OptionUint16Ptr option = OptionUint16Ptr(new OptionUint16(Option::V6,
+                                                                  code, 1234));
+        option->setEncapsulatedSpace("foo");
+        ASSERT_NO_THROW(cfg.add(option, false, DHCP6_OPTION_SPACE));
+    }
+
+    // Create sub-options belonging to "foo" option space.
+    for (uint16_t code = 1; code < 20; ++code) {
+        OptionUint8Ptr option = OptionUint8Ptr(new OptionUint8(Option::V6, code,
+                                                               0x01));
+        ASSERT_NO_THROW(cfg.add(option, false, "foo"));
+    }
+
+    // Append options from "foo" space as sub-options.
+    ASSERT_NO_THROW(cfg.encapsulate());
+
+    // Verify that we have 20 top-level options.
+    OptionContainerPtr options = cfg.getAll(DHCP6_OPTION_SPACE);
+    ASSERT_EQ(20, options->size());
+
+    // Verify that each of them contains expected sub-options.
+    for (uint16_t code = 1000; code < 1020; ++code) {
+        OptionUint16Ptr option = boost::dynamic_pointer_cast<
+            OptionUint16>(cfg.get(DHCP6_OPTION_SPACE, code).option);
+        ASSERT_TRUE(option) << "option with code " << code << " not found";
+        EXPECT_EQ(1234, option->getValue());
+        for (uint16_t subcode = 1; subcode < 20; ++subcode) {
+            OptionUint8Ptr suboption = boost::dynamic_pointer_cast<
+                OptionUint8>(option->getOption(subcode));
+            ASSERT_TRUE(suboption) << "suboption with code " << subcode
+                                   << " not found";
+            EXPECT_EQ(0x01, suboption->getValue());
+        }
+    }
+}
+
 // This test verifies that single option can be retrieved from the configuration
 // using option code and option space.
 TEST(CfgOption, get) {

src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc

@@ -269,31 +269,6 @@ TEST_F(DhcpParserTest, interfaceListParserTest) {
     EXPECT_TRUE(test_config.socketOpen("eth1", AF_INET));
 }
 
-/// @brief Test Implementation of abstract OptionDataParser class. Allows
-/// testing basic option parsing.
-class UtestOptionDataParser : public OptionDataParser {
-public:
-
-    UtestOptionDataParser(const std::string&,
-        OptionStoragePtr options, ParserContextPtr global_context)
-        :OptionDataParser("", options, global_context) {
-    }
-
-    static OptionDataParser* factory(const std::string& param_name,
-        OptionStoragePtr options, ParserContextPtr global_context) {
-        return (new UtestOptionDataParser(param_name, options, global_context));
-    }
-
-protected:
-    // Dummy out last two params since test derivation doesn't use them.
-    virtual OptionDefinitionPtr findServerSpaceOptionDefinition (
-                std::string&, uint32_t) {
-        OptionDefinitionPtr def;
-        // always return empty
-        return (def);
-    }
-};
-
 /// @brief Test Fixture class which provides basic structure for testing
 /// configuration parsing.  This is essentially the same structure provided
 /// by dhcp servers.
@@ -387,10 +362,8 @@ public:
     ParserPtr createConfigParser(const std::string& config_id) {
         ParserPtr parser;
         if (config_id.compare("option-data") == 0) {
-            parser.reset(new OptionDataListParser(config_id,
-                                              parser_context_->options_,
-                                              parser_context_,
-                                              UtestOptionDataParser::factory));
+            parser.reset(new OptionDataListParser(config_id, CfgOptionPtr(),
+                                                  AF_INET));
 
         } else if (config_id.compare("option-def") == 0) {
             parser.reset(new OptionDefListParser(config_id,
@@ -449,15 +422,14 @@ public:
     OptionPtr getOptionPtr(std::string space, uint32_t code)
     {
         OptionPtr option_ptr;
-        OptionContainerPtr options =
-                            parser_context_->options_->getItems(space);
+        OptionContainerPtr options = CfgMgr::instance().getStagingCfg()->
+            getCfgOption()->getAll(space);
         // Should always be able to get options list even if it is empty.
         EXPECT_TRUE(options);
         if (options) {
             // Attempt to find desired option.
             const OptionContainerTypeIndex& idx = options->get<1>();
-            const OptionContainerTypeRange& range =
-                                                        idx.equal_range(code);
+            const OptionContainerTypeRange& range = idx.equal_range(code);
             int cnt = std::distance(range.first, range.second);
             EXPECT_EQ(1, cnt);
             if (cnt == 1) {
@@ -1106,20 +1078,6 @@ public:
                      values->getPosition(name).file_));
     }
 
-    /// @brief Check that option storage in the context holds one option
-    /// of the specified type.
-    ///
-    /// @param ctx A pointer to a context.
-    /// @param opt_type Expected option type.
-    void checkOptionType(const ParserContext& ctx, const uint16_t opt_type) {
-        OptionContainerPtr options =
-            ctx.options_->getItems("option-space");
-        ASSERT_TRUE(options);
-        OptionContainerTypeIndex& idx = options->get<1>();
-        OptionContainerTypeRange range = idx.equal_range(opt_type);
-        ASSERT_EQ(1, std::distance(range.first, range.second));
-    }
-
     /// @brief Test copy constructor or assignment operator when values
     /// being copied are NULL.
     ///
@@ -1131,7 +1089,6 @@ public:
         ctx.boolean_values_.reset();
         ctx.uint32_values_.reset();
         ctx.string_values_.reset();
-        ctx.options_.reset();
         ctx.hooks_libraries_.reset();
 
         // Even if the fields of the context are NULL, it should get
@@ -1147,7 +1104,6 @@ public:
         EXPECT_FALSE(ctx_new->boolean_values_);
         EXPECT_FALSE(ctx_new->uint32_values_);
         EXPECT_FALSE(ctx_new->string_values_);
-        EXPECT_FALSE(ctx_new->options_);
         EXPECT_FALSE(ctx_new->hooks_libraries_);
 
     }
@@ -1204,14 +1160,6 @@ public:
                                       Element::Position("kea.conf", 100, 200));
 
 
-        // Add new option, with option code 10, to the context.
-        ASSERT_TRUE(ctx.options_);
-        OptionPtr opt1(new Option(Option::V6, 10));
-        OptionDescriptor desc1(opt1, false);
-        std::string option_space = "option-space";
-        ASSERT_TRUE(desc1.option);
-        ctx.options_->addItem(desc1, 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");
@@ -1291,14 +1239,6 @@ public:
                             ctx_new->string_values_);
         }
 
-        // New context has the same option.
-        ASSERT_TRUE(ctx_new->options_);
-        {
-            SCOPED_TRACE("Check that the option values are equal in both"
-                         " contexts");
-            checkOptionType(*ctx_new, 10);
-        }
-
         // New context has the same hooks library.
         ASSERT_TRUE(ctx_new->hooks_libraries_);
         {
@@ -1414,21 +1354,6 @@ public:
 
         }
 
-        // Change the option. This should not affect the option instance in the
-        // new context.
-        {
-            SCOPED_TRACE("Check that option remains the same in the new context"
-                         " when the option in the original context is changed");
-            ctx.options_->clearItems();
-            OptionDescriptor desc(OptionPtr(new Option(Option::V6,
-                                                               100)),
-                                          false);
-
-            ASSERT_TRUE(desc.option);
-            ctx.options_->addItem(desc, "option-space");
-            checkOptionType(*ctx_new, 10);
-        }
-
         // Change the list of libraries. this should not affect the list in the
         // new context.
         ctx.hooks_libraries_->clear();