[4303] Address second round of review comments.

- No additional check for HR mode in findReservations
- Additional commentary in the loop collecting host ids.

src/bin/dhcp4/dhcp4_srv.cc

@@ -245,6 +245,8 @@ void
 Dhcpv4Exchange::setHostIdentifiers() {
     const ConstCfgHostOperationsPtr cfg =
         CfgMgr::instance().getCurrentCfg()->getCfgHostOperations4();
+    // Collect host identifiers. The identifiers are stored in order of preference.
+    // The server will use them in that order to search for host reservations.
     BOOST_FOREACH(const Host::IdentifierType& id_type,
                   cfg->getIdentifierTypes()) {
         switch (id_type) {

src/bin/dhcp6/dhcp6_srv.cc

@@ -281,7 +281,9 @@ Dhcpv6Srv::createContext(const Pkt6Ptr& pkt) {
     ctx.duid_ = pkt->getClientId();
     ctx.hwaddr_ = getMAC(pkt);
 
-    // Collect host identifiers if host reservations enabled.
+    // Collect host identifiers if host reservations enabled. The identifiers
+    // are stored in order of preference. The server will use them in that
+    // order to search for host reservations.
     if (ctx.subnet_ &&
         (ctx.subnet_->getHostReservationMode() != Subnet::HR_DISABLED)) {
         const ConstCfgHostOperationsPtr cfg =

src/lib/dhcpsrv/alloc_engine.cc

@@ -314,9 +314,6 @@ AllocEngine::findReservationInternal(ContextType& ctx,
         // Check which host reservation mode is supported in this subnet.
         Subnet::HRMode hr_mode = ctx.subnet_->getHostReservationMode();
 
-        // Check if there is a host reseravtion for this client. Attempt to
-        // get host information
-        if (hr_mode != Subnet::HR_DISABLED) {
         // Iterate over configured identifiers in the order of preference
         // and try to use each of them to search for the reservations.
         BOOST_FOREACH(const IdentifierPair& id_pair, ctx.host_identifiers_) {
@@ -329,7 +326,6 @@ AllocEngine::findReservationInternal(ContextType& ctx,
             }
         }
     }
-    }
 }
 
 

src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc

@@ -1669,12 +1669,12 @@ TEST_F(AllocEngine4Test, findReservation) {
     EXPECT_TRUE(ctx.host_);
     EXPECT_EQ(ctx.host_->getIPv4Reservation(), host->getIPv4Reservation());
 
-    // If the host reservation mode for the subnet is disabled, the
-    // host should not be returned, even though it exists in the
-    // host database.
+    // Regardless of the host reservation mode, the host should be
+    // always returned when findReservation() is called.
     subnet_->setHostReservationMode(Subnet::HR_DISABLED);
     ASSERT_NO_THROW(engine.findReservation(ctx));
-    EXPECT_FALSE(ctx.host_);
+    EXPECT_TRUE(ctx.host_);
+    EXPECT_EQ(ctx.host_->getIPv4Reservation(), host->getIPv4Reservation());
 
     // Check the third possible reservation mode.
     subnet_->setHostReservationMode(Subnet::HR_OUT_OF_POOL);