[5533a] Extra comments added

diff --git a/src/lib/dhcpsrv/host.h b/src/lib/dhcpsrv/host.h
index 20d8e4e5542dc6850222b549bf8865d0ed0c8307..a92cda079fb3e6aa69889778950acbd36ac53f2a 100644 (file)
--- a/src/lib/dhcpsrv/host.h
+++ b/src/lib/dhcpsrv/host.h
@@ -548,7 +548,7 @@ public:
 
     /// @brief Sets the negative cached flag.
     ///
-    /// @param negative New valie for negative cache flag.
+    /// @param negative sets whether this is a negative cached host.
     void setNegative(bool negative) {
         negative_ = negative;
     }
@@ -618,6 +618,10 @@ private:
     CfgOptionPtr cfg_option6_;
 
     /// @brief Negative cached flag.
+    ///
+    /// This flag determines whether this object is a negative cache, i.e.
+    /// we queried other backends for specific host and there was no
+    /// entry for it.
     bool negative_;
 };
 

diff --git a/src/lib/dhcpsrv/host_data_source_factory.cc b/src/lib/dhcpsrv/host_data_source_factory.cc
index 2eb6cd6b9cf4fde9685476f6b7278bab684fc23a..08695f54276a26ace59d432d9a891203f06c518a 100644 (file)
--- a/src/lib/dhcpsrv/host_data_source_factory.cc
+++ b/src/lib/dhcpsrv/host_data_source_factory.cc
@@ -59,8 +59,8 @@ HostDataSourceFactory::add(HostDataSourceList& sources,
 
     // No match?
     if (index == map_.end()) {
-        isc_throw(InvalidType, "Hosts database access parameter 'type': " <<
-                  db_type << " is invalid");
+        isc_throw(InvalidType, "The type of host backend: '" <<
+                  db_type << "' is not currently supported");
     }
 
     // Call the factory and push the pointer on sources.
@@ -75,8 +75,7 @@ HostDataSourceFactory::add(HostDataSourceList& sources,
 }
 
 bool
-HostDataSourceFactory::del(HostDataSourceList& sources,
-                           const string& db_type) {
+HostDataSourceFactory::del(HostDataSourceList& sources, const string& db_type) {
     for (auto it = sources.begin(); it != sources.end(); ++it) {
         if ((*it)->getType() != db_type) {
             continue;

diff --git a/src/lib/dhcpsrv/host_data_source_factory.h b/src/lib/dhcpsrv/host_data_source_factory.h
index dc399c7d705d7422fbf9caf234e2c795be847873..a4cefe75c24be70d6a228d09f51aa4567440119f 100644 (file)
--- a/src/lib/dhcpsrv/host_data_source_factory.h
+++ b/src/lib/dhcpsrv/host_data_source_factory.h
@@ -51,7 +51,7 @@ public:
     /// a keyword/value pair of the form "type=dbtype" giving the database
     /// type, e.q. "mysql" or "sqlite3".
     ///
-    /// @param sources host data source list.
+    /// @param sources host data source list (new backend will be added here)
     /// @param dbaccess Database access parameters.  These are in the form of
     ///        "keyword=value" pairs, separated by spaces. They are backend-
     ///        -end specific, although must include the "type" keyword which

diff --git a/src/lib/dhcpsrv/host_mgr.cc b/src/lib/dhcpsrv/host_mgr.cc
index 775997d12a69e7dcb662a30486a9032fbff6d5b2..f9783e9807c25e11596f535da5dfd62fb03d30bb 100644 (file)
--- a/src/lib/dhcpsrv/host_mgr.cc
+++ b/src/lib/dhcpsrv/host_mgr.cc
@@ -171,6 +171,9 @@ HostMgr::get4Any(const SubnetID& subnet_id,
                  const size_t identifier_len) const {
     ConstHostPtr host = getCfgHosts()->get4(subnet_id, identifier_type,
                                             identifier_begin, identifier_len);
+
+    // Found it the config file or there are no backends configured?
+    // Then we're done here.
     if (host || alternate_sources_.empty()) {
         return (host);
     }
@@ -181,6 +184,8 @@ HostMgr::get4Any(const SubnetID& subnet_id,
         .arg(Host::getIdentifierAsText(identifier_type, identifier_begin,
                                        identifier_len));
 
+    // Try to find a host in each configured backend. We return as soon
+    // as we find first hit.
     for (auto it = alternate_sources_.begin();
          it != alternate_sources_.end(); ++it) {
         host = (*it)->get4(subnet_id, identifier_type,

diff --git a/src/lib/dhcpsrv/host_mgr.h b/src/lib/dhcpsrv/host_mgr.h
index 1e7110adb31582454948aba3c9a0052d5e5f9b0a..1727bf20e4600a5f416e870f03369f99b415ccae 100644 (file)
--- a/src/lib/dhcpsrv/host_mgr.h
+++ b/src/lib/dhcpsrv/host_mgr.h
@@ -393,8 +393,9 @@ public:
 protected:
     /// @brief The negative caching flag.
     ///
-    /// When true and the first data source is a cache negative answers
-    /// to get[46] for aubnet and identifier are cached.
+    /// When true and the first cache source will be also
+    /// caching negative answers. This works for get[46]
+    /// for a subnet and an identifier.
     bool negative_caching_;
 
     /// @brief Cache an answer.

diff --git a/src/lib/dhcpsrv/libdhcpsrv.dox b/src/lib/dhcpsrv/libdhcpsrv.dox
index 04775b69779b02303d237a5d3403a9ecb8d26a4c..22bc8b888847ed4171d637681b3334a69fd4d971 100644 (file)
--- a/src/lib/dhcpsrv/libdhcpsrv.dox
+++ b/src/lib/dhcpsrv/libdhcpsrv.dox
@@ -106,7 +106,8 @@ host reservations in the MySQL database is required to establish
 connection to the MySQL database and issue specific queries. A factory
 method creating an instance of a base host data source object must be
 registered (at global object initialization for built-in backends,
-dynamically for backends loaded at run-time). Note, that this instance is
+dynamically for backends loaded at run-time). See host_data_source_factory.cc
+for example code that registers MySQL and PostgreSQL. Note, that this instance is
 created as "alternate host data source" as opposed to the primary data
 source which returns host reservations specified in the configuration file.
 The primary data source is implemented internally in the

diff --git a/src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc
index 3efa01163c5b7358bcb42cec5e3eca10ed72d62f..0850bbd19a13c803bb85de12c63657e1e260569c 100644 (file)
--- a/src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc
+++ b/src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc
@@ -103,7 +103,7 @@ public:
     /// Closes the database and re-open it.  Anything committed should be
     /// visible.
     ///
-    /// Parameter is ignored for CQL backend as the v4 and v6 leases share
+    /// Parameter is ignored for CQL backend as the v4 and v6 hosts share
     /// the same database.
     void reopen(Universe) {
         HostMgr::create();
@@ -116,7 +116,7 @@ public:
 ///
 /// This test checks if the CqlHostDataSource can be instantiated.  This happens
 /// only if the database can be opened.  Note that this is not part of the
-/// CqlLeaseMgr test fixure set.  This test checks that the database can be
+/// CqlHostMgr test fixure set.  This test checks that the database can be
 /// opened: the fixtures assume that and check basic operations.
 
 TEST(CqlHostDataSource, OpenDatabase) {
@@ -124,7 +124,7 @@ TEST(CqlHostDataSource, OpenDatabase) {
     destroyCqlSchema(false, true);
     createCqlSchema(false, true);
 
-    // Check that lease manager open the database opens correctly and tidy up.
+    // Check that host manager open the database opens correctly and tidy up.
     //  If it fails, print the error message.
     try {
         HostMgr::create();
@@ -137,7 +137,7 @@ TEST(CqlHostDataSource, OpenDatabase) {
                << "*** before the CQL tests will run correctly.\n";
     }
 
-    // Check that lease manager open the database opens correctly with a longer
+    // Check that host manager open the database opens correctly with a longer
     // timeout.  If it fails, print the error message.
     try {
         std::string connection_string = validCqlConnectionString() + std::string(" ") +

diff --git a/src/lib/dhcpsrv/tests/host_cache_unittest.cc b/src/lib/dhcpsrv/tests/host_cache_unittest.cc
index ec3ce0cb2b3c62f0bab13cefa8e008a34000392d..c8579f01e39eeb390a09f624cca75084e6e46929 100644 (file)
--- a/src/lib/dhcpsrv/tests/host_cache_unittest.cc
+++ b/src/lib/dhcpsrv/tests/host_cache_unittest.cc
@@ -161,7 +161,7 @@ TEST_F(HostCacheTest, identifier4) {
     // Create a host reservation.
     HostPtr host = HostDataSourceUtils::initializeHost4("192.0.2.1",
                                                         Host::IDENT_HWADDR);
-    ASSERT_TRUE(host);  // Make sure the host is generate properly.
+    ASSERT_TRUE(host);  // Make sure the host is generated properly.
     const IOAddress& address = host->getIPv4Reservation();
 
     // Try to add it to the host data source.
@@ -215,7 +215,7 @@ TEST_F(HostCacheTest, identifier6) {
     HostPtr host = HostDataSourceUtils::initializeHost6("2001:db8::1",
                                                         Host::IDENT_DUID,
                                                         false);
-    ASSERT_TRUE(host);  // Make sure the host is generate properly.
+    ASSERT_TRUE(host);  // Make sure the host is generated properly.
     
     // Get the address.
     IPv6ResrvRange resrvs = host->getIPv6Reservations();
@@ -273,7 +273,7 @@ TEST_F(HostCacheTest, address4) {
     // Create a host reservation.
     HostPtr host = HostDataSourceUtils::initializeHost4("192.0.2.1",
                                                         Host::IDENT_HWADDR);
-    ASSERT_TRUE(host);  // Make sure the host is generate properly.
+    ASSERT_TRUE(host);  // Make sure the host is generated properly.
     const IOAddress& address = host->getIPv4Reservation();
 
     // Try to add it to the host data source.
@@ -325,7 +325,7 @@ TEST_F(HostCacheTest, address6) {
     HostPtr host = HostDataSourceUtils::initializeHost6("2001:db8::1",
                                                         Host::IDENT_DUID,
                                                         false);
-    ASSERT_TRUE(host);  // Make sure the host is generate properly.
+    ASSERT_TRUE(host);  // Make sure the host is generated properly.
     
     // Get the address.
     IPv6ResrvRange resrvs = host->getIPv6Reservations();
@@ -380,11 +380,10 @@ TEST_F(HostCacheTest, negativeIdentifier4) {
     ASSERT_FALSE(HostMgr::instance().getNegativeCaching());
 
     // Create a host reservation.
-    HostPtr host = HostDataSourceUtils::initializeHost4("192.0.2.1",
-                                                        Host::IDENT_HWADDR);
-    ASSERT_TRUE(host);  // Make sure the host is generate properly.
+    // We will not add it anywhere, just will use its values.
+    HostPtr host = HostDataSourceUtils::initializeHost4("192.0.2.1", Host::IDENT_HWADDR);
+    ASSERT_TRUE(host);
 
-    // Do not add it to the host data source.
 
     // Try to get it cached.
     ConstHostPtr got = HostMgr::instance().get4(host->getIPv4SubnetID(),
@@ -403,7 +402,8 @@ TEST_F(HostCacheTest, negativeIdentifier4) {
     HostMgr::instance().setNegativeCaching(true);
     ASSERT_TRUE(HostMgr::instance().getNegativeCaching());
 
-    // Try it but it will be cached only the second time.
+    // Try it again. There is no such host, but this time negative cache is enabled,
+    // so this negative response will be added to the cache.
     got = HostMgr::instance().get4Any(host->getIPv4SubnetID(),
                                       host->getIdentifierType(),
                                       &host->getIdentifier()[0],
@@ -450,10 +450,11 @@ TEST_F(HostCacheTest, negativeIdentifier6) {
     ASSERT_TRUE(memptr_);
 
     // Create a host reservation.
+    // We will not add it anywhere, just will use its values.
     HostPtr host = HostDataSourceUtils::initializeHost6("2001:db8::1",
                                                         Host::IDENT_DUID,
                                                         false);
-    ASSERT_TRUE(host);  // Make sure the host is generate properly.
+    ASSERT_TRUE(host);  // Make sure the host is generated properly.
 
     // Do not add it to the host data source.
 
@@ -521,9 +522,10 @@ TEST_F(HostCacheTest, negativeAddress4) {
     ASSERT_TRUE(memptr_);
 
     // Create a host reservation.
+    // We will not add it anywhere, just will use its values.
     HostPtr host = HostDataSourceUtils::initializeHost4("192.0.2.1",
                                                         Host::IDENT_HWADDR);
-    ASSERT_TRUE(host);  // Make sure the host is generate properly.
+    ASSERT_TRUE(host);  // Make sure the host is generated properly.
     const IOAddress& address = host->getIPv4Reservation();
 
     // Do not add it to the host data source.
@@ -550,10 +552,11 @@ TEST_F(HostCacheTest, negativeAddress6) {
     ASSERT_TRUE(memptr_);
 
     // Create a host reservation.
+    // We will not add it anywhere, just will use its values.
     HostPtr host = HostDataSourceUtils::initializeHost6("2001:db8::1",
                                                         Host::IDENT_DUID,
                                                         false);
-    ASSERT_TRUE(host);  // Make sure the host is generate properly.
+    ASSERT_TRUE(host);  // Make sure the host is generated properly.
     
     // Get the address.
     IPv6ResrvRange resrvs = host->getIPv6Reservations();

diff --git a/src/lib/dhcpsrv/tests/host_mgr_unittest.cc b/src/lib/dhcpsrv/tests/host_mgr_unittest.cc
index b05ea47438d308f87399e4d5957744b88e9f5981..3210c84ecd37bd08906cc633f9e30d72bc4ef8ca 100644 (file)
--- a/src/lib/dhcpsrv/tests/host_mgr_unittest.cc
+++ b/src/lib/dhcpsrv/tests/host_mgr_unittest.cc
@@ -110,7 +110,7 @@ protected:
     void testGet4(BaseHostDataSource& data_source);
 
     /// @brief This test verifies that it is possible to retrieve negative
-    /// cached reservation with and only with get4Any,
+    /// cached reservation with and only with get4Any.
     void testGet4Any();
 
     /// @brief This test verifies that it is possible to retrieve an IPv6
@@ -121,7 +121,7 @@ protected:
     void testGet6(BaseHostDataSource& data_source);
 
     /// @brief This test verifies that it is possible to retrieve negative
-    /// cached reservation with and only with get6Any,
+    /// cached reservation with and only with get6Any.
     void testGet6Any();
 
     /// @brief This test verifies that it is possible to retrieve an IPv6
@@ -336,13 +336,13 @@ HostMgrTest::testGet4Any() {
     // Set the negative cache flag on the host.
     new_host->setNegative(true);
 
-    // Get4 can't get it.
+    // get4 is not supposed to get it.
     host = HostMgr::instance().get4(SubnetID(1), Host::IDENT_DUID,
                                     &duids_[0]->getDuid()[0],
                                     duids_[0]->getDuid().size());
     EXPECT_FALSE(host);
 
-    // But Get4Any can.
+    // But get4Any should.
     host = HostMgr::instance().get4Any(SubnetID(1), Host::IDENT_DUID,
                                        &duids_[0]->getDuid()[0],
                                        duids_[0]->getDuid().size());
@@ -409,13 +409,13 @@ HostMgrTest::testGet6Any() {
     // Set the negative cache flag on the host.
     new_host->setNegative(true);
 
-    // Get6 can't get it.
+    // get6 is not supposed to get it.
     host = HostMgr::instance().get6(SubnetID(2), Host::IDENT_HWADDR,
                                     &hwaddrs_[0]->hwaddr_[0],
                                     hwaddrs_[0]->hwaddr_.size());
     EXPECT_FALSE(host);
 
-    // But Get4Any can.
+    // But get6Any should.
     host = HostMgr::instance().get6Any(SubnetID(2), Host::IDENT_HWADDR,
                                        &hwaddrs_[0]->hwaddr_[0],
                                        hwaddrs_[0]->hwaddr_.size());

diff --git a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc
index b7b46affc9e92ed5a0a3f2c3bdbb20a788f084a7..36f7fe8e5fa230b002151ce0e240519bb8521f45 100644 (file)
--- a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc
+++ b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc
@@ -148,7 +148,7 @@ TEST(MySqlHostDataSource, OpenDatabase) {
     destroyMySQLSchema();
     createMySQLSchema();
 
-    // Check that lease manager open the database opens correctly and tidy up.
+    // Check that host manager open the database opens correctly and tidy up.
     //  If it fails, print the error message.
     try {
         HostMgr::create();
@@ -161,7 +161,7 @@ TEST(MySqlHostDataSource, OpenDatabase) {
                << "*** before the MySQL tests will run correctly.\n";
     }
 
-    // Check that lease manager open the database opens correctly with a longer
+    // Check that host manager open the database opens correctly with a longer
     // timeout.  If it fails, print the error message.
     try {
         string connection_string = validMySQLConnectionString() + string(" ") +
@@ -176,7 +176,7 @@ TEST(MySqlHostDataSource, OpenDatabase) {
                << "*** before the MySQL tests will run correctly.\n";
     }
 
-    // Check that attempting to get an instance of the lease manager when
+    // Check that attempting to get an instance of the host manager when
     // none is set throws an exception.
     EXPECT_FALSE(HostMgr::instance().getHostDataSource());
 

diff --git a/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc
index 559065589b6504112b192fd93ba61f430bf1edee..2824728702bd48c0b22abbd3f3f7edf202c63f68 100644 (file)
--- a/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc
+++ b/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc
@@ -148,7 +148,7 @@ TEST(PgSqlHostDataSource, OpenDatabase) {
     destroyPgSQLSchema();
     createPgSQLSchema();
 
-    // Check that lease manager open the database opens correctly and tidy up.
+    // Check that host manager open the database opens correctly and tidy up.
     //  If it fails, print the error message.
     try {
         HostMgr::create();
@@ -161,7 +161,7 @@ TEST(PgSqlHostDataSource, OpenDatabase) {
                << "*** before the PostgreSQL tests will run correctly.\n";
     }
 
-    // Check that lease manager open the database opens correctly with a longer
+    // Check that host manager open the database opens correctly with a longer
     // timeout.  If it fails, print the error message.
     try {
         string connection_string = validPgSQLConnectionString() + string(" ") +
@@ -175,7 +175,7 @@ TEST(PgSqlHostDataSource, OpenDatabase) {
                << "*** before the PostgreSQL tests will run correctly.\n";
     }
 
-    // Check that attempting to get an instance of the lease manager when
+    // Check that attempting to get an instance of the host manager when
     // none is set throws an exception.
     EXPECT_FALSE(HostMgr::instance().getHostDataSource());
 

diff --git a/src/lib/dhcpsrv/testutils/memory_host_data_source.h b/src/lib/dhcpsrv/testutils/memory_host_data_source.h
index 9fda2839edcb512a38184210dc48d7c91dbf3796..02182ed4380095d2e76402fcb5facf2507f3dd91 100644 (file)
--- a/src/lib/dhcpsrv/testutils/memory_host_data_source.h
+++ b/src/lib/dhcpsrv/testutils/memory_host_data_source.h
@@ -31,6 +31,10 @@ public:
 
     /// @brief Return all hosts for the specified HW address or DUID.
     ///
+    /// This may return hosts from multiple subnets.
+    ///
+    /// Currently not implemented.
+    ///
     /// @param hwaddr HW address of the client or NULL if no HW address
     /// available.
     /// @param duid client id or NULL if not available, e.g. DHCPv4 client case.
@@ -41,6 +45,10 @@ public:
     /// @brief Return all hosts connected to any subnet for which reservations
     /// have been made using a specified identifier.
     ///
+    /// This may return hosts from multiple subnets.
+    ///
+    /// Currently not implemented.
+    ///
     /// @param identifier_type Identifier type.
     /// @param identifier_begin Pointer to a beginning of a buffer containing
     /// an identifier.
@@ -53,6 +61,8 @@ public:
 
     /// @brief Returns a collection of hosts using the specified IPv4 address.
     ///
+    /// Currently not implemented.
+    ///
     /// @param address IPv4 address for which the @c Host object is searched.
     /// @return Empty collection of const @c Host objects.
     virtual ConstHostCollection
@@ -60,6 +70,8 @@ public:
 
     /// @brief Returns a host connected to the IPv4 subnet.
     ///
+    /// Currently not implemented.
+    ///
     /// @param subnet_id Subnet identifier.
     /// @param hwaddr HW address of the client or NULL if no HW address
     /// available.
@@ -97,6 +109,8 @@ public:
 
     /// @brief Returns a host connected to the IPv6 subnet.
     ///
+    /// Currently not implemented.
+    ///
     /// @param subnet_id Subnet identifier.
     /// @param hwaddr HW address of the client or NULL if no HW address
     /// available.