BIND 10 trac3295_2, updated. 41f5d870b1701e4efd738156ee18f61d9a5f6b8b [3295] Addressed review comments.

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Jan 29 12:03:29 UTC 2014


The branch, trac3295_2 has been updated
       via  41f5d870b1701e4efd738156ee18f61d9a5f6b8b (commit)
      from  43eabfa8ba1ebfcfdf2ad22e5425e03c9782c86f (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 41f5d870b1701e4efd738156ee18f61d9a5f6b8b
Author: Marcin Siodelski <marcin at isc.org>
Date:   Wed Jan 29 13:03:16 2014 +0100

    [3295] Addressed review comments.

-----------------------------------------------------------------------

Summary of changes:
 src/bin/dhcp6/dhcp6_messages.mes        |    8 +-------
 src/bin/dhcp6/dhcp6_srv.cc              |   27 +++++++++++++--------------
 src/bin/dhcp6/tests/fqdn_unittest.cc    |    5 +++--
 src/lib/dhcpsrv/alloc_engine.h          |   18 +++++++++++++++++-
 src/lib/dhcpsrv/tests/lease_unittest.cc |   12 ++++++------
 5 files changed, 40 insertions(+), 30 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/dhcp6/dhcp6_messages.mes b/src/bin/dhcp6/dhcp6_messages.mes
index afa04dc..4b064cb 100644
--- a/src/bin/dhcp6/dhcp6_messages.mes
+++ b/src/bin/dhcp6/dhcp6_messages.mes
@@ -100,13 +100,7 @@ in its response to the client.
 This debug message is logged when server has found the DHCPv6 Client FQDN Option
 sent by a client and started processing it.
 
-% DHCP6_DDNS_REMOVE_EMPTY_HOSTNAME FQDN for the lease being deleted is empty: %1
-This error message is issued when a lease being deleted contains an indication
-that the DNS Update has been performed for it, but the FQDN is missing for this
-lease. This is an indication that someone may have messed up in the lease
-database.
-
-% DHCP6_DDNS_REMOVE_INVALID_HOSTNAME FQDN for the lease being deleted has invalid format: %1
+% DHCP6_DDNS_REMOVE_INVALID_HOSTNAME invalid FQDN: %1 for the lease: %2 when removing DNS bindings
 This error message is issued when a lease being deleted contains an indication
 that the DNS Update has been performed for it, but the FQDN held in the lease
 database has invalid format and can't be transformed to the canonical on-wire
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 0282e01..25aa91a 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -1132,20 +1132,11 @@ Dhcpv6Srv::createRemovalNameChangeRequest(const Lease6Ptr& lease) {
         return;
     }
 
-    // When lease was added into a database the host name should have
-    // been added. The hostname can be empty if someone messed up in the
-    // lease data base and removed the hostname.
-    if (lease->hostname_.empty()) {
-        LOG_ERROR(dhcp6_logger, DHCP6_DDNS_REMOVE_EMPTY_HOSTNAME)
-            .arg(lease->addr_.toText());
-        return;
-    }
-
     // If hostname is non-empty, try to convert it to wire format so as
     // DHCID can be computed from it. This may throw an exception if hostname
-    // has invalid format. Again, this should be only possible in case of
-    // manual intervention in the database. Note that the last parameter
-    // passed to the writeFqdn function forces conversion of the FQDN
+    // has invalid format or is empty. Again, this should be only possible
+    // in case of manual intervention in the database. Note that the last
+    // parameter passed to the writeFqdn function forces conversion of the FQDN
     // to lower case. This is required by the RFC4701, section 3.5.
     // The DHCID computation is further in this function.
     std::vector<uint8_t> hostname_wire;
@@ -1153,7 +1144,8 @@ Dhcpv6Srv::createRemovalNameChangeRequest(const Lease6Ptr& lease) {
         OptionDataTypeUtil::writeFqdn(lease->hostname_, hostname_wire, true);
     } catch (const Exception& ex) {
         LOG_ERROR(dhcp6_logger, DHCP6_DDNS_REMOVE_INVALID_HOSTNAME)
-            .arg(lease->hostname_);
+            .arg(lease->hostname_.empty() ? "(empty)" : lease->hostname_)
+            .arg(lease->addr_.toText());
         return;
     }
 
@@ -2445,8 +2437,15 @@ Dhcpv6Srv::generateFqdn(const Pkt6Ptr& answer) {
                 LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA, addr);
             if (lease) {
                 lease->hostname_ = stream.str();
+                LeaseMgrFactory::instance().updateLease6(lease);
+
+            } else {
+                isc_throw(isc::Unexpected, "there is no lease in the database "
+                          " for address " << addr << ", so as it is impossible"
+                          " to update FQDN data. This is a programmatic error"
+                          " as the given address is now being handed to the"
+                          " client");
             }
-            LeaseMgrFactory::instance().updateLease6(lease);
         }
 
         // Set the generated FQDN in the Client FQDN option.
diff --git a/src/bin/dhcp6/tests/fqdn_unittest.cc b/src/bin/dhcp6/tests/fqdn_unittest.cc
index 8b975c9..952420d 100644
--- a/src/bin/dhcp6/tests/fqdn_unittest.cc
+++ b/src/bin/dhcp6/tests/fqdn_unittest.cc
@@ -302,8 +302,9 @@ public:
         OptionPtr srvid = srv.getServerID();
         // Set the appropriate FQDN type. It must be partial if hostname is
         // empty.
-        Option6ClientFqdn::DomainNameType fqdn_type = hostname.empty() ?
-            Option6ClientFqdn::PARTIAL : Option6ClientFqdn::FULL;
+        Option6ClientFqdn::DomainNameType fqdn_type = (hostname.empty() ?
+            Option6ClientFqdn::PARTIAL : Option6ClientFqdn::FULL);
+
         Pkt6Ptr req = generateMessage(msg_type, Option6ClientFqdn::FLAG_S,
                                       hostname, fqdn_type, include_oro, srvid);
 
diff --git a/src/lib/dhcpsrv/alloc_engine.h b/src/lib/dhcpsrv/alloc_engine.h
index 7aab2b6..ed2a767 100644
--- a/src/lib/dhcpsrv/alloc_engine.h
+++ b/src/lib/dhcpsrv/alloc_engine.h
@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2014 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
@@ -496,6 +496,22 @@ private:
                                 const isc::hooks::CalloutHandlePtr& callout_handle,
                                 bool fake_allocation = false);
 
+    /// @brief Updates FQDN data for a collection of leases.
+    ///
+    /// @param leases Collection of leases for which FQDN data should be
+    /// updated.
+    /// @param fwd_dns_update Boolean value which indicates whether forward FQDN
+    /// update was performed for each lease (true) or not (false).
+    /// @param rev_dns_update Boolean value which indicates whether reverse FQDN
+    /// update was performed for each lease (true) or not (false).
+    /// @param hostname Client hostname associated with a lease.
+    /// @param fake_allocation Boolean value which indicates that it is a real
+    /// lease allocation, e.g. Request message is processed (false), or address
+    /// is just being picked as a result of processing Solicit (true). In the
+    /// latter case, the FQDN data should not be updated in the lease database.
+    ///
+    /// @return Collection of leases with updated FQDN data. Note that returned
+    /// collection holds updated FQDN data even for fake allocation.
     Lease6Collection updateFqdnData(const Lease6Collection& leases,
                                     const bool fwd_dns_update,
                                     const bool rev_dns_update,
diff --git a/src/lib/dhcpsrv/tests/lease_unittest.cc b/src/lib/dhcpsrv/tests/lease_unittest.cc
index 0b57dac..4e7451f 100644
--- a/src/lib/dhcpsrv/tests/lease_unittest.cc
+++ b/src/lib/dhcpsrv/tests/lease_unittest.cc
@@ -736,17 +736,17 @@ TEST(Lease6, getDuidVector) {
 // Verify the behavior of the function which checks FQDN data for equality.
 TEST(Lease6, hasIdenticalFqdn) {
     Lease6 lease = createLease6("myhost.example.com.", true, true);
-    EXPECT_TRUE(lease.hasIdenticalFqdn(createLease4("myhost.example.com.",
+    EXPECT_TRUE(lease.hasIdenticalFqdn(createLease6("myhost.example.com.",
                                                     true, true)));
-    EXPECT_FALSE(lease.hasIdenticalFqdn(createLease4("other.example.com.",
+    EXPECT_FALSE(lease.hasIdenticalFqdn(createLease6("other.example.com.",
                                                      true, true)));
-    EXPECT_FALSE(lease.hasIdenticalFqdn(createLease4("myhost.example.com.",
+    EXPECT_FALSE(lease.hasIdenticalFqdn(createLease6("myhost.example.com.",
                                                      false, true)));
-    EXPECT_FALSE(lease.hasIdenticalFqdn(createLease4("myhost.example.com.",
+    EXPECT_FALSE(lease.hasIdenticalFqdn(createLease6("myhost.example.com.",
                                                      true, false)));
-    EXPECT_FALSE(lease.hasIdenticalFqdn(createLease4("myhost.example.com.",
+    EXPECT_FALSE(lease.hasIdenticalFqdn(createLease6("myhost.example.com.",
                                                      false, false)));
-    EXPECT_FALSE(lease.hasIdenticalFqdn(createLease4("other.example.com.",
+    EXPECT_FALSE(lease.hasIdenticalFqdn(createLease6("other.example.com.",
                                                      false, false)));
 }
 



More information about the bind10-changes mailing list