From: Marcin Siodelski Date: Tue, 20 Aug 2013 16:00:13 +0000 (+0200) Subject: [3083] Return the old lease instance when new lease is acquired. X-Git-Tag: bind10-1.2.0beta1-release~242^2~6 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=3bc0e24e4d1692c73124fe9727141cc2552e5aa3;p=thirdparty%2Fkea.git [3083] Return the old lease instance when new lease is acquired. --- diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index 9c9017c350..531fff9666 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -329,7 +329,10 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet, const HWAddrPtr& hwaddr, const IOAddress& hint, bool fake_allocation, - const isc::hooks::CalloutHandlePtr& callout_handle) { + const isc::hooks::CalloutHandlePtr& callout_handle, + Lease4Ptr& old_lease) { + + old_lease.reset(); try { // Allocator is always created in AllocEngine constructor and there is @@ -349,6 +352,7 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet, // Check if there's existing lease for that subnet/clientid/hwaddr combination. Lease4Ptr existing = LeaseMgrFactory::instance().getLease4(*hwaddr, subnet->getID()); if (existing) { + old_lease.reset(new Lease4(*existing)); // We have a lease already. This is a returning client, probably after // its reboot. existing = renewLease4(subnet, clientid, hwaddr, existing, fake_allocation); @@ -363,6 +367,7 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet, if (clientid) { existing = LeaseMgrFactory::instance().getLease4(*clientid, subnet->getID()); if (existing) { + old_lease.reset(new Lease4(*existing)); // we have a lease already. This is a returning client, probably after // its reboot. existing = renewLease4(subnet, clientid, hwaddr, existing, fake_allocation); @@ -393,6 +398,7 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet, } } else { if (existing->expired()) { + old_lease.reset(new Lease4(*existing)); return (reuseExpiredLease(existing, subnet, clientid, hwaddr, callout_handle, fake_allocation)); } diff --git a/src/lib/dhcpsrv/alloc_engine.h b/src/lib/dhcpsrv/alloc_engine.h index f655cf30e6..9a919dce49 100644 --- a/src/lib/dhcpsrv/alloc_engine.h +++ b/src/lib/dhcpsrv/alloc_engine.h @@ -196,6 +196,10 @@ protected: /// an address for DISCOVER that is not really allocated (true) /// @param callout_handle a callout handle (used in hooks). A lease callouts /// will be executed if this parameter is passed. + /// @param [out] old_lease Holds the pointer to a previous instance of a + /// lease. The NULL pointer indicates that lease didn't exist prior + /// to calling this function (e.g. new lease has been allocated). + /// /// @return Allocated IPv4 lease (or NULL if allocation failed) Lease4Ptr allocateAddress4(const SubnetPtr& subnet, @@ -203,7 +207,8 @@ protected: const HWAddrPtr& hwaddr, const isc::asiolink::IOAddress& hint, bool fake_allocation, - const isc::hooks::CalloutHandlePtr& callout_handle); + const isc::hooks::CalloutHandlePtr& callout_handle, + Lease4Ptr& old_lease); /// @brief Renews a IPv4 lease /// diff --git a/src/lib/dhcpsrv/lease_mgr.cc b/src/lib/dhcpsrv/lease_mgr.cc index 2310dd4cd5..c4810fd5f9 100644 --- a/src/lib/dhcpsrv/lease_mgr.cc +++ b/src/lib/dhcpsrv/lease_mgr.cc @@ -38,6 +38,52 @@ Lease::Lease(const isc::asiolink::IOAddress& addr, uint32_t t1, uint32_t t2, subnet_id_(subnet_id), fixed_(false), fqdn_fwd_(false), fqdn_rev_(false) { } +Lease4::Lease4(const Lease4& other) + : Lease(other.addr_, other.t1_, other.t2_, other.valid_lft_, + other.subnet_id_, other.cltt_), ext_(other.ext_), + hwaddr_(other.hwaddr_) { + + fixed_ = other.fixed_; + fqdn_fwd_ = other.fqdn_fwd_; + fqdn_rev_ = other.fqdn_rev_; + hostname_ = other.hostname_; + comments_ = other.comments_; + + if (other.client_id_) { + client_id_.reset(new ClientId(other.client_id_->getClientId())); + + } else { + client_id_.reset(); + + } +} + +Lease4& +Lease4::operator=(const Lease4& other) { + if (this != &other) { + addr_ = other.addr_; + t1_ = other.t1_; + t2_ = other.t2_; + valid_lft_ = other.valid_lft_; + cltt_ = other.cltt_; + subnet_id_ = other.subnet_id_; + fixed_ = other.fixed_; + hostname_ = other.hostname_; + fqdn_fwd_ = other.fqdn_fwd_; + fqdn_rev_ = other.fqdn_rev_; + comments_ = other.comments_; + ext_ = other.ext_; + hwaddr_ = other.hwaddr_; + + if (other.client_id_) { + client_id_.reset(new ClientId(other.client_id_->getClientId())); + } else { + client_id_.reset(); + } + } + return (*this); +} + Lease6::Lease6(LeaseType type, const isc::asiolink::IOAddress& addr, DuidPtr duid, uint32_t iaid, uint32_t preferred, uint32_t valid, uint32_t t1, uint32_t t2, SubnetID subnet_id, uint8_t prefixlen) diff --git a/src/lib/dhcpsrv/lease_mgr.h b/src/lib/dhcpsrv/lease_mgr.h index 02e517e4ab..3e0e092682 100644 --- a/src/lib/dhcpsrv/lease_mgr.h +++ b/src/lib/dhcpsrv/lease_mgr.h @@ -1,4 +1,4 @@ -// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2013 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 @@ -259,6 +259,16 @@ struct Lease4 : public Lease { Lease4() : Lease(0, 0, 0, 0, 0, 0) { } + /// @brief Copy constructor + /// + /// @param other the @c Lease4 object to be copied. + Lease4(const Lease4& other); + + /// @brief Assignment operator. + /// + /// @param other the @c Lease4 object to be assigned. + Lease4& operator=(const Lease4& other); + /// @brief Compare two leases for equality /// /// @param other lease6 object with which to compare diff --git a/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc index 65997d29fe..6c6abdd3dc 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc @@ -188,6 +188,7 @@ public: Subnet4Ptr subnet_; ///< Subnet4 (used in tests) Pool4Ptr pool_; ///< Pool belonging to subnet_ LeaseMgrFactory factory_; ///< Pointer to LeaseMgr factory + Lease4Ptr old_lease_; ///< Holds previous instance of the lease. }; // This test checks if the Allocation Engine can be instantiated and that it @@ -613,7 +614,8 @@ TEST_F(AllocEngine4Test, simpleAlloc4) { Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"), false, - CalloutHandlePtr()); + CalloutHandlePtr(), + old_lease_); // Check that we got a lease ASSERT_TRUE(lease); @@ -637,7 +639,8 @@ TEST_F(AllocEngine4Test, fakeAlloc4) { Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"), true, - CalloutHandlePtr()); + CalloutHandlePtr(), + old_lease_); // Check that we got a lease ASSERT_TRUE(lease); @@ -660,7 +663,8 @@ TEST_F(AllocEngine4Test, allocWithValidHint4) { Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress("192.0.2.105"), - false, CalloutHandlePtr()); + false, CalloutHandlePtr(), + old_lease_); // Check that we got a lease ASSERT_TRUE(lease); @@ -700,7 +704,8 @@ TEST_F(AllocEngine4Test, allocWithUsedHint4) { // twice. Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress("192.0.2.106"), - false, CalloutHandlePtr()); + false, CalloutHandlePtr(), + old_lease_); // Check that we got a lease ASSERT_TRUE(lease); @@ -734,7 +739,8 @@ TEST_F(AllocEngine4Test, allocBogusHint4) { // with the normal allocation Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress("10.1.1.1"), - false, CalloutHandlePtr()); + false, CalloutHandlePtr(), + old_lease_); // Check that we got a lease ASSERT_TRUE(lease); @@ -762,18 +768,22 @@ TEST_F(AllocEngine4Test, allocateAddress4Nulls) { // Allocations without subnet are not allowed Lease4Ptr lease = engine->allocateAddress4(SubnetPtr(), clientid_, hwaddr_, IOAddress("0.0.0.0"), false, - CalloutHandlePtr()); + CalloutHandlePtr(), old_lease_); EXPECT_FALSE(lease); // Allocations without HW address are not allowed lease = engine->allocateAddress4(subnet_, clientid_, HWAddrPtr(), - IOAddress("0.0.0.0"), false, CalloutHandlePtr()); + IOAddress("0.0.0.0"), false, + CalloutHandlePtr(), + old_lease_); EXPECT_FALSE(lease); // Allocations without client-id are allowed clientid_ = ClientIdPtr(); lease = engine->allocateAddress4(subnet_, ClientIdPtr(), hwaddr_, - IOAddress("0.0.0.0"), false, CalloutHandlePtr()); + IOAddress("0.0.0.0"), false, + CalloutHandlePtr(), + old_lease_); // Check that we got a lease ASSERT_TRUE(lease); @@ -877,7 +887,8 @@ TEST_F(AllocEngine4Test, smallPool4) { cfg_mgr.addSubnet4(subnet_); Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"), - false, CalloutHandlePtr()); + false, CalloutHandlePtr(), + old_lease_); // Check that we got that single lease ASSERT_TRUE(lease); @@ -926,7 +937,8 @@ TEST_F(AllocEngine4Test, outOfAddresses4) { Lease4Ptr lease2 = engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"), false, - CalloutHandlePtr()); + CalloutHandlePtr(), + old_lease_); EXPECT_FALSE(lease2); } @@ -959,7 +971,8 @@ TEST_F(AllocEngine4Test, discoverReuseExpiredLease4) { // CASE 1: Asking for any address lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"), - true, CalloutHandlePtr()); + true, CalloutHandlePtr(), + old_lease_); // Check that we got that single lease ASSERT_TRUE(lease); EXPECT_EQ(addr.toText(), lease->addr_.toText()); @@ -969,7 +982,8 @@ TEST_F(AllocEngine4Test, discoverReuseExpiredLease4) { // CASE 2: Asking specifically for this address lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress(addr.toText()), - true, CalloutHandlePtr()); + true, CalloutHandlePtr(), + old_lease_); // Check that we got that single lease ASSERT_TRUE(lease); EXPECT_EQ(addr.toText(), lease->addr_.toText()); @@ -997,7 +1011,8 @@ TEST_F(AllocEngine4Test, requestReuseExpiredLease4) { // A client comes along, asking specifically for this address lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress(addr.toText()), false, - CalloutHandlePtr()); + CalloutHandlePtr(), + old_lease_); // Check that he got that single lease ASSERT_TRUE(lease); @@ -1399,7 +1414,8 @@ TEST_F(HookAllocEngine4Test, lease4_select) { Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"), - false, callout_handle); + false, callout_handle, + old_lease_); // Check that we got a lease ASSERT_TRUE(lease); @@ -1462,7 +1478,8 @@ TEST_F(HookAllocEngine4Test, change_lease4_select) { // Call allocateAddress4. Callouts should be triggered here. Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"), - false, callout_handle); + false, callout_handle, + old_lease_); // Check that we got a lease ASSERT_TRUE(lease); diff --git a/src/lib/dhcpsrv/tests/lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/lease_mgr_unittest.cc index d5535ae53f..883874a220 100644 --- a/src/lib/dhcpsrv/tests/lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/lease_mgr_unittest.cc @@ -245,7 +245,7 @@ TEST(LeaseMgr, getParameter) { /// /// Lease4 is also defined in lease_mgr.h, so is tested in this file as well. // This test checks if the Lease4 structure can be instantiated correctly -TEST(Lease4, Lease4Constructor) { +TEST(Lease4, constructor) { // Random values for the tests const uint8_t HWADDR[] = {0x08, 0x00, 0x2b, 0x02, 0x3f, 0x4e}; @@ -292,12 +292,79 @@ TEST(Lease4, Lease4Constructor) { } } +// This test verfies that copy constructor copies Lease4 fields correctly. +TEST(Lease4, copyConstructor) { + + // Random values for the tests + const uint8_t HWADDR[] = {0x08, 0x00, 0x2b, 0x02, 0x3f, 0x4e}; + std::vector hwaddr(HWADDR, HWADDR + sizeof(HWADDR)); + + const uint8_t CLIENTID[] = {0x17, 0x34, 0xe2, 0xff, 0x09, 0x92, 0x54}; + std::vector clientid_vec(CLIENTID, CLIENTID + sizeof(CLIENTID)); + ClientId clientid(clientid_vec); + + // ...and a time + const time_t current_time = time(NULL); + + // Other random constants. + const uint32_t SUBNET_ID = 42; + const uint32_t VALID_LIFETIME = 500; + + // Create the lease + Lease4 lease(0xffffffff, HWADDR, sizeof(HWADDR), + CLIENTID, sizeof(CLIENTID), VALID_LIFETIME, 0, 0, current_time, + SUBNET_ID); + + // Use copy constructor to copy the lease. + Lease4 copied_lease(lease); + + // Both leases should be now equal. When doing this check we assume that + // the equality operator works correctly. + EXPECT_TRUE(lease == copied_lease); + // Client IDs are equal, but they should be in two distinct pointers. + EXPECT_FALSE(lease.client_id_ == copied_lease.client_id_); +} + +// This test verfies that the assignment operator copies all Lease4 fields +// correctly. +TEST(Lease4, operatorAssign) { + + // Random values for the tests + const uint8_t HWADDR[] = {0x08, 0x00, 0x2b, 0x02, 0x3f, 0x4e}; + std::vector hwaddr(HWADDR, HWADDR + sizeof(HWADDR)); + + const uint8_t CLIENTID[] = {0x17, 0x34, 0xe2, 0xff, 0x09, 0x92, 0x54}; + std::vector clientid_vec(CLIENTID, CLIENTID + sizeof(CLIENTID)); + ClientId clientid(clientid_vec); + + // ...and a time + const time_t current_time = time(NULL); + + // Other random constants. + const uint32_t SUBNET_ID = 42; + const uint32_t VALID_LIFETIME = 500; + + // Create the lease + Lease4 lease(0xffffffff, HWADDR, sizeof(HWADDR), + CLIENTID, sizeof(CLIENTID), VALID_LIFETIME, 0, 0, current_time, + SUBNET_ID); + + // Use assignment operator to assign the lease. + Lease4 copied_lease = lease; + + // Both leases should be now equal. When doing this check we assume that + // the equality operator works correctly. + EXPECT_TRUE(lease == copied_lease); + // Client IDs are equal, but they should be in two distinct pointers. + EXPECT_FALSE(lease.client_id_ == copied_lease.client_id_); +} + /// @brief Lease4 Equality Test /// /// Checks that the operator==() correctly compares two leases for equality. /// As operator!=() is also defined for this class, every check on operator==() /// is followed by the reverse check on operator!=(). -TEST(Lease4, OperatorEquals) { +TEST(Lease4, operatorEquals) { // Random values for the tests const uint32_t ADDRESS = 0x01020304;