From: Frantisek Sumsal Date: Tue, 7 Nov 2023 12:16:05 +0000 (+0100) Subject: timesync: fix PropertiesChanges signals for NTP properties X-Git-Tag: v255-rc2~90^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F29915%2Fhead;p=thirdparty%2Fsystemd.git timesync: fix PropertiesChanges signals for NTP properties As in their current form they didn't work at all: systemd-timesyncd[190115]: Assertion 's' failed at src/libsystemd/sd-event/sd-event.c:3058, function sd_event_source_set_enabled(). Ignoring. systemd-timesyncd[190115]: Failed to reenable system ntp server change event source! systemd-timesyncd[190115]: Failed to enable ntp server defer event, ignoring: Invalid argument This was also pointed out in the post-merge review [0]. Let's address this together with the rest of the comments, and add some tests to make sure everything works as it should. Resolves: #28770 Follow-up to: 8f1c446 [0] https://github.com/systemd/systemd/commit/8f1c4469793f2f0281fdfbc20ba4085e20cdd16f#r124147466 --- diff --git a/src/timesync/timesyncd-manager.c b/src/timesync/timesyncd-manager.c index 21825f1165c..82fe668bf9c 100644 --- a/src/timesync/timesyncd-manager.c +++ b/src/timesync/timesyncd-manager.c @@ -1227,8 +1227,8 @@ static int manager_save_time_and_rearm(Manager *m, usec_t t) { static const char* ntp_server_property_name[_SERVER_TYPE_MAX] = { [SERVER_SYSTEM] = "SystemNTPServers", [SERVER_FALLBACK] = "FallbackNTPServers", - [SERVER_LINK] = "LinkNTPServers", - [SERVER_RUNTIME] = "RuntimeNTPServers", + [SERVER_LINK] = "LinkNTPServers", + [SERVER_RUNTIME] = "RuntimeNTPServers", }; static int ntp_server_emit_changed_strv(Manager *manager, char **properties) { diff --git a/src/timesync/timesyncd-server.c b/src/timesync/timesyncd-server.c index fc9bca8c5a8..0f68203e031 100644 --- a/src/timesync/timesyncd-server.c +++ b/src/timesync/timesyncd-server.c @@ -66,16 +66,12 @@ static int enable_ntp_server_defer_event(Manager *m, ServerType type) { assert(m); assert((type >= 0) && (type < _SERVER_TYPE_MAX)); - r = sd_event_source_set_enabled(m->deferred_ntp_server_event_source, SD_EVENT_ONESHOT); - if (r < 0) - return log_debug_errno(r, "Failed to reenable system ntp server change event source!"); + m->ntp_server_change_mask |= 1U << type; r = bus_manager_emit_ntp_server_changed(m); if (r < 0) return r; - m->ntp_server_change_mask |= 1U << type; - return 1; } diff --git a/test/units/testsuite-45.sh b/test/units/testsuite-45.sh index d6a5f1da42a..d93e33f22c0 100755 --- a/test/units/testsuite-45.sh +++ b/test/units/testsuite-45.sh @@ -281,6 +281,117 @@ EOF assert_rc 3 systemctl is-active --quiet systemd-timesyncd systemctl stop busctl-monitor.service + rm -rf /run/systemd/system/systemd-timesyncd.service.d/ + systemctl daemon-reload +} + +assert_timesyncd_signal() { + local timestamp="${1:?}" + local property="${2:?}" + local value="${3:?}" + local args=(-q --since="$timestamp" -p info _SYSTEMD_UNIT="busctl-monitor.service") + + journalctl --sync + + for _ in {0..9}; do + if journalctl "${args[@]}" --grep .; then + [[ "$(journalctl "${args[@]}" -o cat | tr -d '\n' | jq -r ".payload.data[1].$property.data | join(\" \")")" == "$value" ]]; + return 0 + fi + + sleep .5 + done + + return 1 +} + +assert_networkd_ntp() { + local interface="${1:?}" + local value="${2:?}" + # Go through the array of NTP servers and for each entry do: + # - if the entry is an IPv4 address, join the Address array into a dot separated string + # - if the entry is a server address, select it unchanged + # These steps produce an array of strings, that is then joined into a space-separated string + # Note: this doesn't support IPv6 addresses, since converting them to a string is a bit more + # involved than a simple join(), but let's leave that to another time + local expr='[.NTP[] | (select(.Family == 2).Address | join(".")), select(has("Server")).Server] | join(" ")' + + [[ "$(networkctl status "$interface" --json=short | jq -r "$expr")" == "$value" ]] +} + +testcase_timesyncd() { + if systemd-detect-virt -cq; then + echo "This test case requires a VM, skipping..." + return 0 + fi + + if ! command -v networkctl >/dev/null; then + echo "This test requires systemd-networkd, skipping..." + return 0 + fi + + # Create a dummy interface managed by networkd, so we can configure link NTP servers + mkdir -p /run/systemd/network/ + cat >/etc/systemd/network/ntp99.netdev </etc/systemd/network/ntp99.network <