From: Francis Dupont Date: Wed, 24 May 2023 22:09:23 +0000 (+0200) Subject: [#2868] Addressed comments X-Git-Tag: Kea-2.3.8~32 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=003be31fd3d89ec4a5b8841d4c01ce6cbff9fbb3;p=thirdparty%2Fkea.git [#2868] Addressed comments --- diff --git a/src/lib/dhcpsrv/alloc_engine.h b/src/lib/dhcpsrv/alloc_engine.h index 88bb9044db..d7d4fd7d7c 100644 --- a/src/lib/dhcpsrv/alloc_engine.h +++ b/src/lib/dhcpsrv/alloc_engine.h @@ -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 /// diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.cc b/src/lib/dhcpsrv/dhcpsrv_messages.cc index 3b47fcf4d4..8044580035 100644 --- a/src/lib/dhcpsrv/dhcpsrv_messages.cc +++ b/src/lib/dhcpsrv/dhcpsrv_messages.cc @@ -1,4 +1,4 @@ -// File created from ../../../src/lib/dhcpsrv/dhcpsrv_messages.mes +// File created from dhcpsrv_messages.mes #include #include diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.h b/src/lib/dhcpsrv/dhcpsrv_messages.h index 0786c21e95..4a5c9c615a 100644 --- a/src/lib/dhcpsrv/dhcpsrv_messages.h +++ b/src/lib/dhcpsrv/dhcpsrv_messages.h @@ -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 diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.mes b/src/lib/dhcpsrv/dhcpsrv_messages.mes index 324db1f75c..75de2ecaa7 100644 --- a/src/lib/dhcpsrv/dhcpsrv_messages.mes +++ b/src/lib/dhcpsrv/dhcpsrv_messages.mes @@ -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) diff --git a/src/lib/dhcpsrv/lease_mgr.h b/src/lib/dhcpsrv/lease_mgr.h index f83511cbda..50277f0996 100644 --- a/src/lib/dhcpsrv/lease_mgr.h +++ b/src/lib/dhcpsrv/lease_mgr.h @@ -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". /// diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc index e56a116f2f..d86b7025c9 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.cc +++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc @@ -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 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 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(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(link_len)); - if (MultiThreadingMgr::instance().getMode()) { std::lock_guard 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(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(link_len)); - if (MultiThreadingMgr::instance().getMode()) { std::lock_guard 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(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(link_len)); - if (MultiThreadingMgr::instance().getMode()) { std::lock_guard 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()); diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index 7e2fda1f28..1f6c56159a 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -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(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(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()); diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.h b/src/lib/dhcpsrv/mysql_lease_mgr.h index a3621bd371..58d50a45dd 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.h +++ b/src/lib/dhcpsrv/mysql_lease_mgr.h @@ -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". /// diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc index 0d46c0b873..06584b2d99 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc @@ -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(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(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()); diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.h b/src/lib/dhcpsrv/pgsql_lease_mgr.h index fe5f363d0d..95fda12d2c 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.h +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.h @@ -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". ///