]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#2868] Addressed comments
authorFrancis Dupont <fdupont@isc.org>
Wed, 24 May 2023 22:09:23 +0000 (00:09 +0200)
committerFrancis Dupont <fdupont@isc.org>
Thu, 25 May 2023 21:03:53 +0000 (23:03 +0200)
src/lib/dhcpsrv/alloc_engine.h
src/lib/dhcpsrv/dhcpsrv_messages.cc
src/lib/dhcpsrv/dhcpsrv_messages.h
src/lib/dhcpsrv/dhcpsrv_messages.mes
src/lib/dhcpsrv/lease_mgr.h
src/lib/dhcpsrv/memfile_lease_mgr.cc
src/lib/dhcpsrv/mysql_lease_mgr.cc
src/lib/dhcpsrv/mysql_lease_mgr.h
src/lib/dhcpsrv/pgsql_lease_mgr.cc
src/lib/dhcpsrv/pgsql_lease_mgr.h

index 88bb9044db7db1844d334367966981894e13e8b0..d7d4fd7d7c2bf3c76a5164ca0f48abfe6d872311 100644 (file)
@@ -163,7 +163,7 @@ public:
 
     /// @brief Resource compare class.
     ///
-    /// Needed for using sets of Resource objets.
+    /// Needed for using sets of Resource objects.
     struct ResourceCompare {
         /// @brief Compare operator
         ///
index 3b47fcf4d4c7a6637d230e84cd0823449c467841..8044580035e02325e8fcd518fb355a774d8eb8d3 100644 (file)
@@ -1,4 +1,4 @@
-// File created from ../../../src/lib/dhcpsrv/dhcpsrv_messages.mes
+// File created from dhcpsrv_messages.mes
 
 #include <cstddef>
 #include <log/message_types.h>
index 0786c21e957d9fe67de974f1e0d414d98a7d27a0..4a5c9c615a13ec44421e31a9de007babb78e68e6 100644 (file)
@@ -1,4 +1,4 @@
-// File created from ../../../src/lib/dhcpsrv/dhcpsrv_messages.mes
+// File created from dhcpsrv_messages.mes
 
 #ifndef DHCPSRV_MESSAGES_H
 #define DHCPSRV_MESSAGES_H
index 324db1f75c80d4b25ca77a7ca3f9427d1f3949b6..75de2ecaa7a92fda97d51f05dba6fd9c8d025a56 100644 (file)
@@ -463,7 +463,7 @@ Extended info tables build was finished. Some statistics are displayed, the
 updated in database is returned to the command interface.
 
 % DHCPSRV_MEMFILE_BUILD_EXTENDED_INFO_TABLES6_ERROR building extended info tables got an exception on the lease for %1: %2
-A debug message issued when the server is building extended info tables and
+An error message issued when the server is building extended info tables and
 receives an exception processing a lease.
 
 % DHCPSRV_MEMFILE_COMMIT committing to memory file database
@@ -985,7 +985,7 @@ The server upgraded binary addresses. The number of pages and the
 final count of updated leases are displayed.
 
 % DHCPSRV_MYSQL_UPGRADE_BINARY_ADDRESS6_ERROR upgrading binary address for IPv6 lease at %1 failed with %2
-A debug message issued when the server failed to upgrade a binary address.
+An error message issued when the server failed to upgrade a binary address.
 The address of the lease and the error message are displayed.
 
 % DHCPSRV_MYSQL_UPGRADE_BINARY_ADDRESS6_PAGE upgrading IPv6 lease binary addresses at page %1 starting at %2 (updated %3)
@@ -1260,7 +1260,7 @@ The server upgraded binary addresses. The number of pages and the
 final count of updated leases are displayed.
 
 % DHCPSRV_PGSQL_UPGRADE_BINARY_ADDRESS6_ERROR upgrading binary address for IPv6 lease at %1 failed with %2
-A debug message issued when the server failed to upgrade a binary address.
+An error message issued when the server failed to upgrade a binary address.
 The address of the lease and the error message are displayed.
 
 % DHCPSRV_PGSQL_UPGRADE_BINARY_ADDRESS6_PAGE upgrading IPv6 lease binary addresses at page %1 starting at %2 (updated %3)
index f83511cbda4442cd7b677f681bbb679a2c8a7a6c..50277f0996087b2e09fa163ff23f291ad268dd81 100644 (file)
@@ -986,7 +986,7 @@ public:
     /// @brief Upgrade binary address (v6).
     ///
     /// On SQL backends for all leases with null binary address set this
-    /// new column. Memfile uses IOAddress objets so does not need it.
+    /// new column. Memfile uses IOAddress objects so does not need it.
     /// This function implements the new BLQ hook command named
     /// "binary-address6-upgrade".
     ///
index e56a116f2f2db6addaadbc361dce8f7e95b163be..d86b7025c9570c79ee3d6d760bd2b1f801ae2415 100644 (file)
@@ -2511,6 +2511,14 @@ Memfile_LeaseMgr::getLeases4ByRelayId(const OptionBuffer& relay_id,
                                       const LeasePageSize& page_size,
                                       const time_t& qry_start_time /* = 0 */,
                                       const time_t& qry_end_time /* = 0 */) {
+    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
+              DHCPSRV_MEMFILE_GET_RELAYID4)
+        .arg(page_size.page_size_)
+        .arg(lower_bound_address.toText())
+        .arg(idToText(relay_id))
+        .arg(qry_start_time)
+        .arg(qry_end_time);
+
     // Expecting IPv4 address.
     if (!lower_bound_address.isV4()) {
         isc_throw(InvalidAddressFamily, "expected IPv4 address while "
@@ -2529,14 +2537,6 @@ Memfile_LeaseMgr::getLeases4ByRelayId(const OptionBuffer& relay_id,
         isc_throw(BadValue, "start time must be before end time");
     }
 
-    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
-              DHCPSRV_MEMFILE_GET_RELAYID4)
-        .arg(page_size.page_size_)
-        .arg(lower_bound_address.toText())
-        .arg(idToText(relay_id))
-        .arg(qry_start_time)
-        .arg(qry_end_time);
-
     if (MultiThreadingMgr::instance().getMode()) {
         std::lock_guard<std::mutex> lock(*mutex_);
         return (getLeases4ByRelayIdInternal(relay_id,
@@ -2597,6 +2597,14 @@ Memfile_LeaseMgr::getLeases4ByRemoteId(const OptionBuffer& remote_id,
                                        const LeasePageSize& page_size,
                                        const time_t& qry_start_time /* = 0 */,
                                        const time_t& qry_end_time /* = 0 */) {
+    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
+              DHCPSRV_MEMFILE_GET_REMOTEID4)
+        .arg(page_size.page_size_)
+        .arg(lower_bound_address.toText())
+        .arg(idToText(remote_id))
+        .arg(qry_start_time)
+        .arg(qry_end_time);
+
     // Expecting IPv4 address.
     if (!lower_bound_address.isV4()) {
         isc_throw(InvalidAddressFamily, "expected IPv4 address while "
@@ -2615,14 +2623,6 @@ Memfile_LeaseMgr::getLeases4ByRemoteId(const OptionBuffer& remote_id,
         isc_throw(BadValue, "start time must be before end time");
     }
 
-    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
-              DHCPSRV_MEMFILE_GET_REMOTEID4)
-        .arg(page_size.page_size_)
-        .arg(lower_bound_address.toText())
-        .arg(idToText(remote_id))
-        .arg(qry_start_time)
-        .arg(qry_end_time);
-
     if (MultiThreadingMgr::instance().getMode()) {
         std::lock_guard<std::mutex> lock(*mutex_);
         return (getLeases4ByRemoteIdInternal(remote_id,
@@ -2683,6 +2683,14 @@ Memfile_LeaseMgr::getLeases6ByRelayId(const DUID& relay_id,
                                       uint8_t link_len,
                                       const IOAddress& lower_bound_address,
                                       const LeasePageSize& page_size) {
+    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
+              DHCPSRV_MEMFILE_GET_RELAYID6)
+        .arg(page_size.page_size_)
+        .arg(lower_bound_address.toText())
+        .arg(relay_id.toText())
+        .arg(link_addr.toText())
+        .arg(static_cast<unsigned>(link_len));
+
     // Expecting IPv6 valid prefix and address.
     if (!link_addr.isV6()) {
         isc_throw(InvalidAddressFamily, "expected IPv6 address while "
@@ -2699,14 +2707,6 @@ Memfile_LeaseMgr::getLeases6ByRelayId(const DUID& relay_id,
                   << lower_bound_address);
     }
 
-    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
-              DHCPSRV_MEMFILE_GET_RELAYID6)
-        .arg(page_size.page_size_)
-        .arg(lower_bound_address.toText())
-        .arg(relay_id.toText())
-        .arg(link_addr.toText())
-        .arg(static_cast<unsigned>(link_len));
-
     if (MultiThreadingMgr::instance().getMode()) {
         std::lock_guard<std::mutex> lock(*mutex_);
         return (getLeases6ByRelayIdInternal(relay_id,
@@ -2793,6 +2793,14 @@ Memfile_LeaseMgr::getLeases6ByRemoteId(const OptionBuffer& remote_id,
                                        uint8_t link_len,
                                        const IOAddress& lower_bound_address,
                                        const LeasePageSize& page_size) {
+    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
+              DHCPSRV_MEMFILE_GET_REMOTEID6)
+        .arg(page_size.page_size_)
+        .arg(lower_bound_address.toText())
+        .arg(idToText(remote_id))
+        .arg(link_addr.toText())
+        .arg(static_cast<unsigned>(link_len));
+
     // Expecting IPv6 valid prefix and address.
     if (!link_addr.isV6()) {
         isc_throw(InvalidAddressFamily, "expected IPv6 address while "
@@ -2809,14 +2817,6 @@ Memfile_LeaseMgr::getLeases6ByRemoteId(const OptionBuffer& remote_id,
                   << lower_bound_address);
     }
 
-    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
-              DHCPSRV_MEMFILE_GET_REMOTEID6)
-        .arg(page_size.page_size_)
-        .arg(lower_bound_address.toText())
-        .arg(idToText(remote_id))
-        .arg(link_addr.toText())
-        .arg(static_cast<unsigned>(link_len));
-
     if (MultiThreadingMgr::instance().getMode()) {
         std::lock_guard<std::mutex> lock(*mutex_);
         return (getLeases6ByRemoteIdInternal(remote_id,
@@ -2898,6 +2898,13 @@ Memfile_LeaseMgr::getLeases6ByLink(const IOAddress& link_addr,
                                    uint8_t link_len,
                                    const IOAddress& lower_bound_address,
                                    const LeasePageSize& page_size) {
+    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
+              DHCPSRV_MEMFILE_GET_LINKADDR6)
+        .arg(page_size.page_size_)
+        .arg(lower_bound_address.toText())
+        .arg(link_addr.toText())
+        .arg(static_cast<unsigned>(link_len));
+
     // Expecting IPv6 valid prefix and address.
     if (!link_addr.isV6()) {
         isc_throw(InvalidAddressFamily, "expected IPv6 address while "
@@ -2914,13 +2921,6 @@ Memfile_LeaseMgr::getLeases6ByLink(const IOAddress& link_addr,
                   << lower_bound_address);
     }
 
-    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
-              DHCPSRV_MEMFILE_GET_LINKADDR6)
-        .arg(page_size.page_size_)
-        .arg(lower_bound_address.toText())
-        .arg(link_addr.toText())
-        .arg(static_cast<unsigned>(link_len));
-
     if (MultiThreadingMgr::instance().getMode()) {
         std::lock_guard<std::mutex> lock(*mutex_);
         return (getLeases6ByLinkInternal(link_addr,
@@ -3083,7 +3083,7 @@ Memfile_LeaseMgr::buildExtendedInfoTables6Internal(bool update, bool current) {
                 ++processed;
             }
         } catch (const std::exception& ex) {
-            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE,
+            LOG_ERROR(dhcpsrv_logger,
                       DHCPSRV_MEMFILE_BUILD_EXTENDED_INFO_TABLES6_ERROR)
                 .arg(lease->addr_.toText())
                 .arg(ex.what());
index 7e2fda1f28c9068b9889681e33bbad72c1d079bd..1f6c56159a51760bd37ea2a603534135f7d3b396 100644 (file)
@@ -3717,6 +3717,14 @@ MySqlLeaseMgr::getLeases4ByRelayId(const OptionBuffer& relay_id,
                                    const LeasePageSize& page_size,
                                    const time_t& qry_start_time /* = 0 */,
                                    const time_t& qry_end_time /* = 0 */) {
+    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
+              DHCPSRV_MYSQL_GET_RELAYID4)
+        .arg(page_size.page_size_)
+        .arg(lower_bound_address.toText())
+        .arg(idToText(relay_id))
+        .arg(qry_start_time)
+        .arg(qry_end_time);
+
     // Expecting IPv4 address.
     if (!lower_bound_address.isV4()) {
         isc_throw(InvalidAddressFamily, "expected IPv4 address while "
@@ -3737,14 +3745,6 @@ MySqlLeaseMgr::getLeases4ByRelayId(const OptionBuffer& relay_id,
         isc_throw(BadValue, "start time must be before end time");
     }
 
-    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
-              DHCPSRV_MYSQL_GET_RELAYID4)
-        .arg(page_size.page_size_)
-        .arg(lower_bound_address.toText())
-        .arg(idToText(relay_id))
-        .arg(qry_start_time)
-        .arg(qry_end_time);
-
     // Prepare WHERE clause
     size_t bindings = 3;
     if (have_qst) {
@@ -3829,6 +3829,14 @@ MySqlLeaseMgr::getLeases4ByRemoteId(const OptionBuffer& remote_id,
                                     const LeasePageSize& page_size,
                                     const time_t& qry_start_time /* = 0 */,
                                     const time_t& qry_end_time /* = 0 */) {
+    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
+              DHCPSRV_MYSQL_GET_REMOTEID4)
+        .arg(page_size.page_size_)
+        .arg(lower_bound_address.toText())
+        .arg(idToText(remote_id))
+        .arg(qry_start_time)
+        .arg(qry_end_time);
+
     // Expecting IPv4 address.
     if (!lower_bound_address.isV4()) {
         isc_throw(InvalidAddressFamily, "expected IPv4 address while "
@@ -3849,14 +3857,6 @@ MySqlLeaseMgr::getLeases4ByRemoteId(const OptionBuffer& remote_id,
         isc_throw(BadValue, "start time must be before end time");
     }
 
-    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
-              DHCPSRV_MYSQL_GET_REMOTEID4)
-        .arg(page_size.page_size_)
-        .arg(lower_bound_address.toText())
-        .arg(idToText(remote_id))
-        .arg(qry_start_time)
-        .arg(qry_end_time);
-
     // Prepare WHERE clause
     size_t bindings = 3;
     if (have_qst) {
@@ -4047,6 +4047,13 @@ MySqlLeaseMgr::getLeases6ByLink(const IOAddress& link_addr,
                                 uint8_t link_len,
                                 const IOAddress& lower_bound_address,
                                 const LeasePageSize& page_size) {
+    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
+              DHCPSRV_MYSQL_GET_LINKADDR6)
+        .arg(page_size.page_size_)
+        .arg(lower_bound_address.toText())
+        .arg(link_addr.toText())
+        .arg(static_cast<unsigned>(link_len));
+
     // Expecting IPv6 valid prefix and address.
     if (!link_addr.isV6()) {
         isc_throw(InvalidAddressFamily, "expected IPv6 address while "
@@ -4063,13 +4070,6 @@ MySqlLeaseMgr::getLeases6ByLink(const IOAddress& link_addr,
                   << lower_bound_address);
     }
 
-    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
-              DHCPSRV_MYSQL_GET_LINKADDR6)
-        .arg(page_size.page_size_)
-        .arg(lower_bound_address.toText())
-        .arg(link_addr.toText())
-        .arg(static_cast<unsigned>(link_len));
-
     Lease6Collection result;
     const IOAddress& first_addr = firstAddrInPrefix(link_addr, link_len);
     const IOAddress& last_addr = lastAddrInPrefix(link_addr, link_len);
@@ -4181,6 +4181,8 @@ MySqlLeaseMgr::upgradeBinaryAddress6(const LeasePageSize& page_size) {
         start_addr = leases.back()->addr_;
         for (auto lease : leases) {
             try {
+                // Update to the same lease will fill the new column i.e.
+                // refresh does the job...
                 updateLease6(lease);
                 ++updated;
             } catch (const NoSuchLease&) {
@@ -4189,7 +4191,7 @@ MySqlLeaseMgr::upgradeBinaryAddress6(const LeasePageSize& page_size) {
                 continue;
             } catch (const std::exception& ex) {
                 // Something when wrong, for instance extract failed.
-                LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE,
+                LOG_ERROR(dhcpsrv_logger,
                           DHCPSRV_MYSQL_UPGRADE_BINARY_ADDRESS6_ERROR)
                     .arg(lease->addr_.toText())
                     .arg(ex.what());
index a3621bd371966398dbdb957a28857a4d3aca9b2c..58d50a45ddb7d621ac4ae6599655fa99f1faa916 100644 (file)
@@ -1139,7 +1139,7 @@ private:
     /// @brief Upgrade binary address (v6).
     ///
     /// On SQL backends for all leases with null binary address set this
-    /// new column. Memfile uses IOAddress objets so does not need it.
+    /// new column. Memfile uses IOAddress objects so does not need it.
     /// This function implements the new BLQ hook command named
     /// "binary-address6-upgrade".
     ///
index 0d46c0b873cc89e16edf541f7c439331df731bc0..06584b2d994b7d0a83678a2743af649fb7369403 100644 (file)
@@ -3178,6 +3178,13 @@ PgSqlLeaseMgr::getLeases6ByLink(const IOAddress& link_addr,
                                 uint8_t link_len,
                                 const IOAddress& lower_bound_address,
                                 const LeasePageSize& page_size) {
+    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
+              DHCPSRV_PGSQL_GET_LINKADDR6)
+        .arg(page_size.page_size_)
+        .arg(lower_bound_address.toText())
+        .arg(link_addr.toText())
+        .arg(static_cast<unsigned>(link_len));
+
     // Expecting IPv6 valid prefix and address.
     if (!link_addr.isV6()) {
         isc_throw(InvalidAddressFamily, "expected IPv6 address while "
@@ -3194,13 +3201,6 @@ PgSqlLeaseMgr::getLeases6ByLink(const IOAddress& link_addr,
                   << lower_bound_address);
     }
 
-    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
-              DHCPSRV_PGSQL_GET_LINKADDR6)
-        .arg(page_size.page_size_)
-        .arg(lower_bound_address.toText())
-        .arg(link_addr.toText())
-        .arg(static_cast<unsigned>(link_len));
-
     Lease6Collection result;
     const IOAddress& first_addr = firstAddrInPrefix(link_addr, link_len);
     const IOAddress& last_addr = lastAddrInPrefix(link_addr, link_len);
@@ -3296,6 +3296,8 @@ PgSqlLeaseMgr::upgradeBinaryAddress6(const LeasePageSize& page_size) {
         start_addr = leases.back()->addr_;
         for (auto lease : leases) {
             try {
+                // Update to the same lease will fill the new column i.e.
+                // refresh does the job...
                 updateLease6(lease);
                 ++updated;
             } catch (const NoSuchLease&) {
@@ -3304,7 +3306,7 @@ PgSqlLeaseMgr::upgradeBinaryAddress6(const LeasePageSize& page_size) {
                 continue;
             } catch (const std::exception& ex) {
                 // Something when wrong, for instance extract failed.
-                LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE,
+                LOG_ERROR(dhcpsrv_logger,
                           DHCPSRV_PGSQL_UPGRADE_BINARY_ADDRESS6_ERROR)
                     .arg(lease->addr_.toText())
                     .arg(ex.what());
index fe5f363d0da22d53cfb2176ff2cade2cb9d70243..95fda12d2ca12b8246737b62432fd16a67ee282e 100644 (file)
@@ -1093,7 +1093,7 @@ private:
     /// @brief Upgrade binary address (v6).
     ///
     /// On SQL backends for all leases with null binary address set this
-    /// new column. Memfile uses IOAddress objets so does not need it.
+    /// new column. Memfile uses IOAddress objects so does not need it.
     /// This function implements the new BLQ hook command named
     /// "binary-address6-upgrade".
     ///