BIND 10 trac2327, updated. 62a23854f619349d319d02c3a385d9bc55442d5e [2327] Changes after review - detailedLeaseCompare unified (alloc_engine and mysql_lease_mgr) - comments clarified
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Dec 18 15:32:53 UTC 2012
The branch, trac2327 has been updated
via 62a23854f619349d319d02c3a385d9bc55442d5e (commit)
from 89f2d7507297c5e3c1daf815f45e54499d029e27 (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 62a23854f619349d319d02c3a385d9bc55442d5e
Author: Tomek Mrugalski <tomasz at isc.org>
Date: Tue Dec 18 16:32:34 2012 +0100
[2327] Changes after review
- detailedLeaseCompare unified (alloc_engine and mysql_lease_mgr)
- comments clarified
-----------------------------------------------------------------------
Summary of changes:
src/lib/dhcpsrv/alloc_engine.cc | 22 ++++++--
src/lib/dhcpsrv/tests/Makefile.am | 1 +
src/lib/dhcpsrv/tests/alloc_engine_unittest.cc | 35 ++++---------
src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc | 44 +---------------
src/lib/dhcpsrv/tests/test_utils.cc | 58 +++++++++++++++++++++
src/lib/dhcpsrv/tests/test_utils.h | 49 +++++++++++++++++
6 files changed, 138 insertions(+), 71 deletions(-)
create mode 100644 src/lib/dhcpsrv/tests/test_utils.cc
create mode 100644 src/lib/dhcpsrv/tests/test_utils.h
-----------------------------------------------------------------------
diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index fc4b581..1a2a057 100644
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -208,6 +208,22 @@ AllocEngine::allocateAddress6(const Subnet6Ptr& subnet,
}
}
+ // Hint is in the pool but is not available. Search the pool until first of
+ // the following occurs:
+ // - we find a free address
+ // - we find an address for which the lease has expired
+ // - we exhaust number of tries
+ //
+ // @todo: Current code does not handle pool exhaustion well. It will be
+ // improved. Current problems:
+ // 1. with attempts set to too large value (e.g. 1000) and a small pool (e.g.
+ // 10 addresses), we will iterate over it 100 times before giving up
+ // 2. attempts 0 mean unlimited (this is really UINT_MAX, not infinite)
+ // 3. the whole concept of infinite attempts is just asking for infinite loop
+ // We may consider some form or reference counting (this pool has X addresses
+ // left), but this has one major problem. We exactly control allocation
+ // moment, but we currently do not control expiration time at all
+
unsigned int i = attempts_;
do {
IOAddress candidate = allocator_->pickAddress(subnet, duid, hint);
@@ -216,9 +232,9 @@ AllocEngine::allocateAddress6(const Subnet6Ptr& subnet,
/// implemented
Lease6Ptr existing = LeaseMgrFactory::instance().getLease6(candidate);
- // there's no existing lease for selected candidate, so it is
- // free. Let's allocate it.
if (!existing) {
+ // there's no existing lease for selected candidate, so it is
+ // free. Let's allocate it.
Lease6Ptr lease = createLease(subnet, duid, iaid, candidate,
fake_allocation);
if (lease) {
@@ -274,7 +290,7 @@ Lease6Ptr AllocEngine::reuseExpiredLease(Lease6Ptr& expired,
if (!fake_allocation) {
// for REQUEST we do update the lease
LeaseMgrFactory::instance().updateLease6(expired);
- }
+ }
// We do nothing for SOLICIT. We'll just update database when
// the client gets back to us with REQUEST message.
diff --git a/src/lib/dhcpsrv/tests/Makefile.am b/src/lib/dhcpsrv/tests/Makefile.am
index 8de5fda..11e1d75 100644
--- a/src/lib/dhcpsrv/tests/Makefile.am
+++ b/src/lib/dhcpsrv/tests/Makefile.am
@@ -40,6 +40,7 @@ libdhcpsrv_unittests_SOURCES += pool_unittest.cc
libdhcpsrv_unittests_SOURCES += schema_copy.h
libdhcpsrv_unittests_SOURCES += subnet_unittest.cc
libdhcpsrv_unittests_SOURCES += triplet_unittest.cc
+libdhcpsrv_unittests_SOURCES += test_utils.cc test_utils.h
libdhcpsrv_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) $(LOG4CPLUS_INCLUDES)
if HAVE_MYSQL
diff --git a/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc
index 72d6ece..95eabb2 100644
--- a/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc
@@ -22,6 +22,8 @@
#include <dhcpsrv/lease_mgr_factory.h>
#include <dhcpsrv/memfile_lease_mgr.h>
+#include <dhcpsrv/tests/test_utils.h>
+
#include <boost/shared_ptr.hpp>
#include <boost/scoped_ptr.hpp>
#include <gtest/gtest.h>
@@ -35,6 +37,7 @@ using namespace std;
using namespace isc;
using namespace isc::asiolink;
using namespace isc::dhcp;
+using namespace isc::dhcp::test;
namespace {
@@ -108,26 +111,6 @@ TEST_F(AllocEngineTest, constructor) {
ASSERT_NO_THROW(x.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100)));
}
-/// @todo: This method is taken from mysql_lease_mgr_utilities.cc from ticket
-/// #2342. Get rid of one instance once the code is merged
-void
-detailCompareLease6(const Lease6Ptr& first, const Lease6Ptr& second) {
- EXPECT_EQ(first->type_, second->type_);
-
- // Compare address strings - odd things happen when they are different
- // as the EXPECT_EQ appears to call the operator uint32_t() function,
- // which causes an exception to be thrown for IPv6 addresses.
- EXPECT_EQ(first->addr_.toText(), second->addr_.toText());
- EXPECT_EQ(first->prefixlen_, second->prefixlen_);
- EXPECT_EQ(first->iaid_, second->iaid_);
- EXPECT_TRUE(*first->duid_ == *second->duid_);
- EXPECT_EQ(first->preferred_lft_, second->preferred_lft_);
- EXPECT_EQ(first->valid_lft_, second->valid_lft_);
- EXPECT_EQ(first->cltt_, second->cltt_);
- EXPECT_EQ(first->subnet_id_, second->subnet_id_);
-}
-
-
// This test checks if the simple allocation can succeed
TEST_F(AllocEngineTest, simpleAlloc) {
boost::scoped_ptr<AllocEngine> engine;
@@ -148,7 +131,7 @@ TEST_F(AllocEngineTest, simpleAlloc) {
ASSERT_TRUE(from_mgr);
// Now check that the lease in LeaseMgr has the same parameters
- detailCompareLease6(lease, from_mgr);
+ detailCompareLease(lease, from_mgr);
}
// This test checks if the fake allocation (for SOLICIT) can succeed
@@ -196,7 +179,7 @@ TEST_F(AllocEngineTest, allocWithValidHint) {
ASSERT_TRUE(from_mgr);
// Now check that the lease in LeaseMgr has the same parameters
- detailCompareLease6(lease, from_mgr);
+ detailCompareLease(lease, from_mgr);
}
// This test checks if the allocation with a hint that is in range,
@@ -235,7 +218,7 @@ TEST_F(AllocEngineTest, allocWithUsedHint) {
ASSERT_TRUE(from_mgr);
// Now check that the lease in LeaseMgr has the same parameters
- detailCompareLease6(lease, from_mgr);
+ detailCompareLease(lease, from_mgr);
}
// This test checks if the allocation with a hint that is out the blue
@@ -265,7 +248,7 @@ TEST_F(AllocEngineTest, allocBogusHint) {
ASSERT_TRUE(from_mgr);
// Now check that the lease in LeaseMgr has the same parameters
- detailCompareLease6(lease, from_mgr);
+ detailCompareLease(lease, from_mgr);
}
// This test verifies that the allocator picks addresses that belong to the
@@ -370,7 +353,7 @@ TEST_F(AllocEngineTest, smallPool) {
ASSERT_TRUE(from_mgr);
// Now check that the lease in LeaseMgr has the same parameters
- detailCompareLease6(lease, from_mgr);
+ detailCompareLease(lease, from_mgr);
}
// This test checks if all addresses in a pool are currently used, the attempt
@@ -487,7 +470,7 @@ TEST_F(AllocEngineTest, requestReuseExpiredLease) {
ASSERT_TRUE(from_mgr);
// Now check that the lease in LeaseMgr has the same parameters
- detailCompareLease6(lease, from_mgr);
+ detailCompareLease(lease, from_mgr);
}
}; // end of anonymous namespace
diff --git a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc
index 746ef00..89cacef 100644
--- a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc
+++ b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc
@@ -17,6 +17,7 @@
#include <asiolink/io_address.h>
#include <dhcpsrv/lease_mgr_factory.h>
#include <dhcpsrv/mysql_lease_mgr.h>
+#include <dhcpsrv/tests/test_utils.h>
#include <gtest/gtest.h>
@@ -29,6 +30,7 @@
using namespace isc;
using namespace isc::asiolink;
using namespace isc::dhcp;
+using namespace isc::dhcp::test;
using namespace std;
namespace {
@@ -537,48 +539,6 @@ public:
vector<IOAddress> ioaddress6_; ///< IOAddress forms of IPv6 addresses
};
-///@{
-/// @brief Test Utilities
-///
-/// The follow are a set of functions used during the tests.
-
-/// @brief Compare two Lease4 structures for equality
-void
-detailCompareLease(const Lease4Ptr& first, const Lease4Ptr& second) {
- // Compare address strings. Comparison of address objects is not used, as
- // odd things happen when they are different: the EXPECT_EQ macro appears to
- // call the operator uint32_t() function, which causes an exception to be
- // thrown for IPv6 addresses.
- EXPECT_EQ(first->addr_.toText(), second->addr_.toText());
- EXPECT_TRUE(first->hwaddr_ == second->hwaddr_);
- EXPECT_TRUE(*first->client_id_ == *second->client_id_);
- EXPECT_EQ(first->valid_lft_, second->valid_lft_);
- EXPECT_EQ(first->cltt_, second->cltt_);
- EXPECT_EQ(first->subnet_id_, second->subnet_id_);
-}
-
-/// @brief Compare two Lease6 structures for equality
-void
-detailCompareLease(const Lease6Ptr& first, const Lease6Ptr& second) {
- EXPECT_EQ(first->type_, second->type_);
-
- // Compare address strings. Comparison of address objects is not used, as
- // odd things happen when they are different: the EXPECT_EQ macro appears to
- // call the operator uint32_t() function, which causes an exception to be
- // thrown for IPv6 addresses.
- EXPECT_EQ(first->addr_.toText(), second->addr_.toText());
- EXPECT_EQ(first->prefixlen_, second->prefixlen_);
- EXPECT_EQ(first->iaid_, second->iaid_);
- EXPECT_TRUE(*first->duid_ == *second->duid_);
- EXPECT_EQ(first->preferred_lft_, second->preferred_lft_);
- EXPECT_EQ(first->valid_lft_, second->valid_lft_);
- EXPECT_EQ(first->cltt_, second->cltt_);
- EXPECT_EQ(first->subnet_id_, second->subnet_id_);
-}
-
-///@}
-
-
/// @brief Check that database can be opened
///
/// This test checks if the MySqlLeaseMgr can be instantiated. This happens
diff --git a/src/lib/dhcpsrv/tests/test_utils.cc b/src/lib/dhcpsrv/tests/test_utils.cc
new file mode 100644
index 0000000..3c69dbe
--- /dev/null
+++ b/src/lib/dhcpsrv/tests/test_utils.cc
@@ -0,0 +1,58 @@
+// Copyright (C) 2012 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
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include "test_utils.h"
+#include <gtest/gtest.h>
+
+namespace isc {
+namespace dhcp {
+namespace test {
+
+void
+detailCompareLease(const Lease4Ptr& first, const Lease4Ptr& second) {
+ // Compare address strings. Comparison of address objects is not used, as
+ // odd things happen when they are different: the EXPECT_EQ macro appears to
+ // call the operator uint32_t() function, which causes an exception to be
+ // thrown for IPv6 addresses.
+ EXPECT_EQ(first->addr_.toText(), second->addr_.toText());
+ EXPECT_TRUE(first->hwaddr_ == second->hwaddr_);
+ EXPECT_TRUE(*first->client_id_ == *second->client_id_);
+ EXPECT_EQ(first->valid_lft_, second->valid_lft_);
+ EXPECT_EQ(first->cltt_, second->cltt_);
+ EXPECT_EQ(first->subnet_id_, second->subnet_id_);
+}
+
+void
+detailCompareLease(const Lease6Ptr& first, const Lease6Ptr& second) {
+ EXPECT_EQ(first->type_, second->type_);
+
+ // Compare address strings. Comparison of address objects is not used, as
+ // odd things happen when they are different: the EXPECT_EQ macro appears to
+ // call the operator uint32_t() function, which causes an exception to be
+ // thrown for IPv6 addresses.
+ EXPECT_EQ(first->addr_.toText(), second->addr_.toText());
+ EXPECT_EQ(first->prefixlen_, second->prefixlen_);
+ EXPECT_EQ(first->iaid_, second->iaid_);
+ ASSERT_TRUE(first->duid_);
+ ASSERT_TRUE(second->duid_);
+ EXPECT_TRUE(*first->duid_ == *second->duid_);
+ EXPECT_EQ(first->preferred_lft_, second->preferred_lft_);
+ EXPECT_EQ(first->valid_lft_, second->valid_lft_);
+ EXPECT_EQ(first->cltt_, second->cltt_);
+ EXPECT_EQ(first->subnet_id_, second->subnet_id_);
+}
+
+};
+};
+};
diff --git a/src/lib/dhcpsrv/tests/test_utils.h b/src/lib/dhcpsrv/tests/test_utils.h
new file mode 100644
index 0000000..46df9fc
--- /dev/null
+++ b/src/lib/dhcpsrv/tests/test_utils.h
@@ -0,0 +1,49 @@
+// Copyright (C) 2012 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
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef LIBDHCPSRV_TEST_UTILS_H
+#define LIBDHCPSRV_TEST_UTILS_H
+
+#include <dhcpsrv/lease_mgr.h>
+
+namespace isc {
+namespace dhcp {
+namespace test {
+
+// @brief performs details comparison between two IPv6 leases
+//
+// @param first first lease to compare
+// @param second second lease to compare
+//
+// This method is intended to be run from gtest tests as it
+// uses gtest macros and possibly reports gtest failures.
+void
+detailCompareLease(const Lease6Ptr& first, const Lease6Ptr& second);
+
+// @brief performs details comparison between two IPv4 leases
+//
+// @param first first lease to compare
+// @param second second lease to compare
+//
+// This method is intended to be run from gtest tests as it
+// uses gtest macros and possibly reports gtest failures.
+void
+detailCompareLease(const Lease4Ptr& first, const Lease4Ptr& second);
+
+
+};
+};
+};
+
+#endif
More information about the bind10-changes
mailing list