]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#399,!215] Addressed review comments
authorThomas Markwalder <tmark@isc.org>
Tue, 5 Feb 2019 13:24:15 +0000 (08:24 -0500)
committerThomas Markwalder <tmark@isc.org>
Tue, 5 Feb 2019 13:24:15 +0000 (08:24 -0500)
src/bin/dhcp4/tests/config_backend_unittest.cc
src/lib/dhcpsrv/cfg_shared_networks.cc
src/lib/dhcpsrv/cfg_shared_networks.h
src/lib/dhcpsrv/cfg_subnets4.cc
src/lib/dhcpsrv/cfg_subnets4.h
src/lib/dhcpsrv/shared_network.cc
src/lib/dhcpsrv/tests/cfg_shared_networks4_unittest.cc
src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc

index a16a8cfc560a05a892ceef230ab93058d241f33b..75d574b4bc52d978c8a2e5423b4887db44379a9e 100644 (file)
@@ -441,8 +441,10 @@ TEST_F(Dhcp4CBTest, mergeSharedNetworks) {
     staged_network = networks->getByName("two");
     ASSERT_TRUE(staged_network);
 
-    // Subnet3, which is in db2 should not have been merged, since it is
-    // backend data is first found, first used.
+    // Subnet3, which is in db2 should not have been merged.
+    // We queried db1 first and the query returned data. In
+    // other words, we iterate over the backends, asking for
+    // data.  We use the first data, we find.
     staged_network = networks->getByName("three");
     ASSERT_FALSE(staged_network);
 }
index c1558f7350a93ed0d5b9a12d8694708c180af6f1..3ab6e3626ebd018c4d0ab71522d16723161bf6e0 100644 (file)
@@ -24,8 +24,8 @@ CfgSharedNetworks4::merge(const CfgSharedNetworks4& other) {
     auto& index = networks_.get<SharedNetworkNameIndexTag>();
 
     // Iterate over the subnets to be merged. They will replace the existing
-    // subnets with the same id. All new subnets will be inserted into the
-    // configuration into which we're merging.
+    // subnets with the same id. All new subnets will be inserted into this
+    // configuration.
     auto other_networks = other.getAll();
     for (auto other_network = other_networks->begin();
          other_network != other_networks->end(); ++other_network) {
index c9094623f38c52bdcf43de5a1fae922b95c31701..833b8234b9ea1e882d6343b6726b74a8206866b9 100644 (file)
@@ -124,7 +124,8 @@ public:
     /// @return true if there is a network with a specified server identifier.
     bool hasNetworkWithServerId(const asiolink::IOAddress& server_id) const;
 
-    /// @brief Merges specified shared network configuration into this configuration.
+    /// @brief Merges specified shared network configuration into this
+    /// configuration.
     ///
     /// This method merges networks from the @c other configuration into this
     /// configuration. The general rule is that existing networks are replaced
@@ -132,14 +133,14 @@ public:
     ///
     /// For each network in @c other, do the following:
     ///
-    /// - Any associated subnets are removed.  Shared networks retreived from
-    /// config backends, do not carry their associated subnets (if any) with them.
-    /// Subnet assignments are maintained by subnet merges.
-    /// - If a shared network of the same name already exists in this
+    /// - Any associated subnets are removed.  Shared networks retrieved from
+    /// config backends, do not carry their associated subnets (if any) with
+    /// them. (Subnet assignments are maintained by subnet merges).
+    /// - If a shared network of the same name already exists in this
     /// configuration:
-    ///     - All of its associated subnets are movde to the "other" network
-    ///     - The existing network is removed from this configuration
-    /// - The "other" network is added
+    ///     - All of its associated subnets are moved to the "other" network.
+    ///     - The existing network is removed from this configuration.
+    /// - The "other" network is added to this configuration.
     ///
     /// @warning The merge operation may affect the @c other configuration.
     /// Therefore, the caller must not rely on the data held in the @c other
index bdcf2fe58adaac81f081c36f949a513ee3f33643..fa097d329c9a97f8691cbdcf8d67481c66e09566 100644 (file)
@@ -64,7 +64,8 @@ CfgSubnets4::merge(CfgSharedNetworks4Ptr networks,
     // subnets with the same id. All new subnets will be inserted into the
     // configuration into which we're merging.
     auto other_subnets = other.getAll();
-    for (auto other_subnet = other_subnets->begin(); other_subnet != other_subnets->end();
+    for (auto other_subnet = other_subnets->begin();
+         other_subnet != other_subnets->end();
          ++other_subnet) {
 
         // Check if there is a subnet with the same ID.
@@ -74,7 +75,7 @@ CfgSubnets4::merge(CfgSharedNetworks4Ptr networks,
             // Subnet found.
             auto existing_subnet = *subnet_it;
 
-            // Continue if the existing subnet and other subnet
+            // If the existing subnet and other subnet
             // are the same instance skip it.
             if (existing_subnet == *other_subnet) {
                 continue;
@@ -106,7 +107,7 @@ CfgSubnets4::merge(CfgSharedNetworks4Ptr networks,
             } else {
                 // This implies the shared-network collection we were given
                 // is out of sync with the subnets we were given.
-                isc_throw(InvalidOperation, "Cannot assign subnet ID of '"
+                isc_throw(InvalidOperation, "Cannot assign subnet ID of "
                           << (*other_subnet)->getID()
                           << " to shared network: " << network_name
                           << ", network does not exist");
index 762d8637f7033ce65106fe4d9b8a0689289b1d26..b6c1b80fb99a9d64943b947189707c76193c665c 100644 (file)
@@ -60,9 +60,9 @@ public:
     /// are the authority on their shared network assignments. It is also
     /// assumed that @ networks is the list of shared networks that should be
     /// used in making assignments.  The general concept is that the overarching
-    /// merge process will first merge shared networks and then pass that list of
-    /// networks into this method. Subnets from @c other are then merged into this
-    /// configuration as follows:
+    /// merge process will first merge shared networks and then pass that list
+    /// of networks into this method. Subnets from @c other are then merged
+    /// into this configuration as follows:
     ///
     /// For each subnet in @c other:
     ///
@@ -70,7 +70,7 @@ public:
     ///    -# If it belongs to a shared network, remove it from that network
     ///    -# Remove the subnet from this configuration and discard it
     ///
-    /// - Add the subnet from other to this configuration.
+    /// - Add the subnet from @c other to this configuration.
     /// - If that subnet is associated to shared network, find that network
     ///   in @ networks and add that subnet to it.
     ///
@@ -81,7 +81,7 @@ public:
     /// merged configuration.
     ///
     /// @param networks collection of shared networks that to which assignments
-    /// should be added.  In other words, the list of shared networks that belong
+    /// should be added. In other words, the list of shared networks that belong
     /// to the same SrvConfig instance we are merging into.
     /// @param other the subnet configuration to be merged into this
     /// configuration.
index 014442fa7b5fc2961f833bc34978834caab3fd17..3ae74b2f19a9ef58317cf4855aab1a92b09d9274 100644 (file)
@@ -1,4 +1,4 @@
-// Copyright (C) 2017-2018 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2017-2019 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -319,12 +319,14 @@ SharedNetwork6::add(const Subnet6Ptr& subnet) {
     Impl::add(subnets_, subnet);
     // Associate the subnet with this network.
     setSharedNetwork(subnet);
+    subnet->setSharedNetworkName(name_);
 }
 
 void
 SharedNetwork6::del(const SubnetID& subnet_id) {
     Subnet6Ptr subnet = Impl::del<Subnet6Ptr>(subnets_, subnet_id);
     clearSharedNetwork(subnet);
+    subnet->setSharedNetworkName("");
 }
 
 void
index 918f46e278924a982b2a671327ae0f6195ea02ad..5d704fba3b4ddde4b53e68bf5346e5095d022fd7 100644 (file)
@@ -193,7 +193,7 @@ TEST(CfgSharedNetworks4Test, mergeNetworks) {
     SharedNetwork4Ptr network2(new SharedNetwork4("network2"));
     network2->setValid(Triplet<uint32_t>(200));
 
-    // Create network3 with one subnets.
+    // Create network3 with one subnet.
     SharedNetwork4Ptr network3(new SharedNetwork4("network3"));
     network3->setValid(Triplet<uint32_t>(300));
     ASSERT_NO_THROW(network3->add(subnet3));
@@ -240,7 +240,7 @@ TEST(CfgSharedNetworks4Test, mergeNetworks) {
 
     // Should still have 3 networks.
 
-    // Network1 should have doubled its valid life time but still only have
+    // Network1 should have doubled its valid lifetime but still only have
     // the orignal two subnets.  Merge should discard assocations on CB
     // subnets and preserve the associations from existing config.
     ASSERT_EQ(3, cfg_to.getAll()->size());
@@ -251,7 +251,7 @@ TEST(CfgSharedNetworks4Test, mergeNetworks) {
     ASSERT_NO_FATAL_FAILURE(checkMergedNetwork(cfg_to, "network2", Triplet<uint32_t>(200),
                                                std::vector<SubnetID>()));
 
-    // Network1 should have doubled its valid life time and still subnet3.
+    // Network1 should have doubled its valid lifetime and still subnet3.
     ASSERT_NO_FATAL_FAILURE(checkMergedNetwork(cfg_to, "network3", Triplet<uint32_t>(600),
                                                std::vector<SubnetID>{SubnetID(3)}));
 }
index bfea1173f8cc85e59e842091d51b44772cdd3a05..c0984b26f97beeb82b2cfe1e50948cf0c5c4f817 100644 (file)
@@ -159,7 +159,7 @@ TEST(CfgSubnets4Test, mergeSubnets) {
     // Empty network pointer.
     SharedNetwork4Ptr no_network;
 
-    // Add Subnets1,2, and 4 to shared networks.
+    // Add Subnets 1, 2 and 4 to shared networks.
     ASSERT_NO_THROW(shared_network1->add(subnet1));
     ASSERT_NO_THROW(shared_network2->add(subnet2));
     ASSERT_NO_THROW(shared_network2->add(subnet4));