[2995] Changes after review

src/lib/dhcpsrv/alloc_engine.cc

@@ -491,26 +491,24 @@ Lease6Ptr AllocEngine::reuseExpiredLease(Lease6Ptr& expired,
     /// @todo: log here that the lease was reused (there's ticket #2524 for
     /// logging in libdhcpsrv)
 
-    // Let's execute all callouts registered for lease6_ia_added
+    // Let's execute all callouts registered for lease6_select
     if (callout_handle &&
         HooksManager::getHooksManager().calloutsPresent(hook_index_lease6_select_)) {
 
         // Delete all previous arguments
         callout_handle->deleteAllArguments();
 
-        // Clear skip flag if it was set in previous callouts
-        callout_handle->setSkip(false);
-
         // Pass necessary arguments
-
         // Subnet from which we do the allocation
         callout_handle->setArgument("subnet6", subnet);
 
         // Is this solicit (fake = true) or request (fake = false)
         callout_handle->setArgument("fake_allocation", fake_allocation);
+
+        // The lease that will be assigned to a client
         callout_handle->setArgument("lease6", expired);
 
-        // This is the first callout, so no need to clear any arguments
+        // Call the callouts
         HooksManager::getHooksManager().callCallouts(hook_index_lease6_select_,
                                                      *callout_handle);
 
@@ -518,7 +516,7 @@ Lease6Ptr AllocEngine::reuseExpiredLease(Lease6Ptr& expired,
         // assigned, so the client will get NoAddrAvail as a result. The lease
         // won't be inserted into the
         if (callout_handle->getSkip()) {
-            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_HOOKS, DHCPSRV_HOOK_LEASE6_IA_ADD_SKIP);
+            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS, DHCPSRV_HOOK_LEASE6_IA_ADD_SKIP);
             return (Lease6Ptr());
         }
 
@@ -617,7 +615,7 @@ Lease6Ptr AllocEngine::createLease6(const Subnet6Ptr& subnet,
         // assigned, so the client will get NoAddrAvail as a result. The lease
         // won't be inserted into the
         if (callout_handle->getSkip()) {
-            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_HOOKS, DHCPSRV_HOOK_LEASE6_IA_ADD_SKIP);
+            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS, DHCPSRV_HOOK_LEASE6_IA_ADD_SKIP);
             return (Lease6Ptr());
         }
 

src/lib/dhcpsrv/cfgmgr.cc

@@ -262,10 +262,6 @@ void CfgMgr::deleteSubnets6() {
     subnets6_.clear();
 }
 
-const Subnet6Collection& CfgMgr::getSubnets6() {
-    return (subnets6_);
-}
-
 
 std::string CfgMgr::getDataDir() {
     return (datadir_);

src/lib/dhcpsrv/cfgmgr.h

@@ -208,7 +208,10 @@ public:
     /// to choose a different subnet. Server code has to offer a list
     /// of possible choices (i.e. all subnets).
     /// @return const reference to Subnet6 collection
-    const Subnet6Collection& getSubnets6();
+    inline const Subnet6Collection& getSubnets6() {
+    return (subnets6_);
+}
+
 
     /// @brief get IPv4 subnet by address
     ///

src/lib/dhcpsrv/dhcpsrv_log.h

@@ -51,7 +51,7 @@ const int DHCPSRV_DBG_TRACE_DETAIL = DBGLVL_TRACE_DETAIL;
 const int DHCPSRV_DBG_TRACE_DETAIL_DATA = DBGLVL_TRACE_DETAIL_DATA;
 
 // Trace hook related operations
-const int DHCPSRV_HOOKS = DBGLVL_TRACE_BASIC;
+const int DHCPSRV_DBG_HOOKS = DBGLVL_TRACE_BASIC;
 
 ///@}
 

src/lib/dhcpsrv/dhcpsrv_messages.mes

@@ -122,9 +122,9 @@ server closes the currently open database, and opens a database using
 the new parameters.
 
 % DHCPSRV_HOOK_LEASE6_IA_ADD_SKIP Lease6 (non-temporary) creation was skipped, because of callout skip flag.
-This debug message is printed when a callout installed on lease6_ia_assign
+This debug message is printed when a callout installed on lease6_assign
 hook point sets a skip flag. It means that the server was told that no lease6
-should be assigned. Server will not put that lease in its database and the client
+should be assigned. The server will not put that lease in its database and the client
 will get a NoAddrsAvail for that IA_NA option.
 
 % DHCPSRV_INVALID_ACCESS invalid database access string: %1

src/lib/dhcpsrv/tests/alloc_engine_unittest.cc

@@ -1165,7 +1165,7 @@ TEST_F(HookAllocEngine6Test, lease6_select) {
     ASSERT_TRUE(from_mgr);
 
     // Check that callouts were indeed called
-    EXPECT_EQ(callback_name_, "lease6_select");
+    EXPECT_EQ("lease6_select", callback_name_);
 
     // Now check that the lease in LeaseMgr has the same parameters
     ASSERT_TRUE(callback_lease6_);
@@ -1223,10 +1223,10 @@ TEST_F(HookAllocEngine6Test, change_lease6_select) {
 
     // See if the values overridden by callout are there
     EXPECT_TRUE(lease->addr_.equals(addr_override_));
-    EXPECT_EQ(lease->t1_, t1_override_);
-    EXPECT_EQ(lease->t2_, t2_override_);
-    EXPECT_EQ(lease->preferred_lft_, pref_override_);
-    EXPECT_EQ(lease->valid_lft_, valid_override_);
+    EXPECT_EQ(t1_override_, lease->t1_);
+    EXPECT_EQ(t2_override_, lease->t2_);
+    EXPECT_EQ(pref_override_, lease->preferred_lft_);
+    EXPECT_EQ(valid_override_, lease->valid_lft_);
 
     // Now check if the lease is in the database
     Lease6Ptr from_mgr = LeaseMgrFactory::instance().getLease6(lease->addr_);
@@ -1234,10 +1234,10 @@ TEST_F(HookAllocEngine6Test, change_lease6_select) {
 
     // Check if values in the database are overridden
     EXPECT_TRUE(from_mgr->addr_.equals(addr_override_));
-    EXPECT_EQ(from_mgr->t1_, t1_override_);
-    EXPECT_EQ(from_mgr->t2_, t2_override_);
-    EXPECT_EQ(from_mgr->preferred_lft_, pref_override_);
-    EXPECT_EQ(from_mgr->valid_lft_, valid_override_);
+    EXPECT_EQ(t1_override_, from_mgr->t1_);
+    EXPECT_EQ(t2_override_, from_mgr->t2_);
+    EXPECT_EQ(pref_override_, from_mgr->preferred_lft_);
+    EXPECT_EQ(valid_override_, from_mgr->valid_lft_);
 }
 
 }; // End of anonymous namespace