From afaae43bb191dc187a366fc3595b6b4e34039acf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 21 Jul 2019 15:06:35 +0200 Subject: [PATCH 1/3] timedated: add back support for ntp-units.d/ We removed support for foreign services (and ntp-units.d/) in b72ddf0f4. Support for foreign services was added back in 5d280742, but through an environment variable. The problem with the env var approach is that it only works as a mechanism to select one item, and doesn't work nicely as a mechinism to create a list of items through drop-ins (because the env var can be easily overridden, but not extended). Having a list of "ntp providers" is important to be able to reliably disable all of them when that is requested. Another problem is that nobody ever bothered to care about our new "standard". ntp-units.d/ is a nice simple format that works and is already supported by chrony and ntpd and timedatex. If we were to introduce and ask people to follow a new standard, there should be some good reason for this. The idea with env vars has lower functionality, requires systemd-specific syntax. We should just re-adopt the format that we originally introduced and that seems to work for everyone, and more on to more interesting problems. --- man/systemd-timedated.service.xml | 43 +++++++++++------- man/timedatectl.xml | 31 +++++++------ src/shared/pretty-print.c | 5 +++ src/timedate/timedated.c | 72 +++++++++++++++++++++++++++---- 4 files changed, 111 insertions(+), 40 deletions(-) diff --git a/man/systemd-timedated.service.xml b/man/systemd-timedated.service.xml index 3626e8bc511..f981848cb20 100644 --- a/man/systemd-timedated.service.xml +++ b/man/systemd-timedated.service.xml @@ -31,7 +31,7 @@ systemd-timedated is a system service that may be used as a mechanism to change the system clock and - timezone, as well as to enable/disable NTP time synchronization. + timezone, as well as to enable/disable network time synchronization. systemd-timedated is automatically activated on request and terminates itself when it is unused. @@ -46,25 +46,36 @@ - Environment + List of network time synchronization services - - - $SYSTEMD_TIMEDATED_NTP_SERVICES + systemd-timesyncd will look for files with a .list extension + in ntp-units.d/ directories. Each file is parsed as a list of unit names, one per + line. Empty lines and lines with comments (#) are ignored. Files are read from + /usr/lib/systemd/ntp-units.d/ and the corresponding directories under + /etc/, /run/, /usr/local/lib/. Files in + /etc/ override files with the same name in /run/, + /usr/local/lib/, and /usr/lib/. Files in + /run/ override files with the same name under /usr/. Packages + should install their configuration files in /usr/lib/ (distribution packages) or + /usr/local/lib/ (local installs). - Colon-separated list of unit names of NTP client services. - If not set, then - systemd-timesyncd.service8 - is used. See the entries of NTP related commands of - timedatectl1 - for details about this. + + <filename>ntp-units.d/</filename> entry for <command>systemd-timesyncd</command> + # /usr/lib/systemd/ntp-units.d/80-systemd-timesync.list +systemd-timesyncd.service + + - Example: - SYSTEMD_TIMEDATED_NTP_SERVICES=ntpd.service:chronyd.service:systemd-timesyncd.service - - - + If the environment variable $SYSTEMD_TIMEDATED_NTP_SERVICES is set, + systemd-timesyncd will parse the contents of that variable as a colon-separated list + of unit names. When set, this variable overrides the file-based list described above. + + + An override that specifies that <command>chronyd</command> should be used if available + SYSTEMD_TIMEDATED_NTP_SERVICES=chronyd.service:systemd-timesyncd.service + + See Also diff --git a/man/timedatectl.xml b/man/timedatectl.xml index 262b9126e7c..f797e0cd67c 100644 --- a/man/timedatectl.xml +++ b/man/timedatectl.xml @@ -23,22 +23,25 @@ - timedatectl OPTIONS COMMAND + timedatectl + OPTIONS + COMMAND Description - timedatectl may be used to query and - change the system clock and its settings. + timedatectl may be used to query and change the system clock and its settings, + and enable or disable time synchronization services. Use systemd-firstboot1 to initialize the system time zone for mounted (but not booted) system images. - timedatectl may be used to show the current status of + timedatectl may be used to show the current status of time synchronization + services, for example systemd-timesyncd.service8. @@ -123,11 +126,8 @@ status - Show current settings of the system clock and RTC, - including whether network time synchronization through - systemd-timesyncd.service is active. Even if it is - inactive, a different service might still synchronize the clock. - If no command is specified, this is the implied default. + Show current settings of the system clock and RTC, including whether network time + synchronization is active. If no command is specified, this is the implied default. @@ -193,11 +193,11 @@ set-ntp [BOOL] - Takes a boolean argument. Controls whether network time synchronization is active - and enabled (if available). If the argument is true, this enables and starts the first existed - service listed in the environment variable $SYSTEMD_TIMEDATED_NTP_SERVICES - of systemd-timedated.service. If the argument is false, then this disables and - stops the all services listed in $SYSTEMD_TIMEDATED_NTP_SERVICES. + Takes a boolean argument. Controls whether network time synchronization is active and + enabled (if available). If the argument is true, this enables and starts the first existing network + synchronization service. If the argument is false, then this disables and stops the known network + synchronization services. The way that the list of services is built is described below. + @@ -250,8 +250,7 @@ Exit status - On success, 0 is returned, a non-zero failure - code otherwise. + On success, 0 is returned, a non-zero failure code otherwise. diff --git a/src/shared/pretty-print.c b/src/shared/pretty-print.c index c602e036eb5..96e74a758f0 100644 --- a/src/shared/pretty-print.c +++ b/src/shared/pretty-print.c @@ -247,6 +247,11 @@ static int guess_type(const char **name, bool *is_usr, bool *is_collection, cons if (path_equal(n, "kernel/install.d")) ext = ".install"; + if (path_equal(n, "systemd/ntp-units.d")) { + coll = true; + ext = ".list"; + } + if (PATH_IN_SET(n, "systemd/system-preset", "systemd/user-preset")) { coll = true; ext = ".preset"; diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c index 57bbefc0c10..074ba3c0792 100644 --- a/src/timedate/timedated.c +++ b/src/timedate/timedated.c @@ -15,7 +15,9 @@ #include "bus-error.h" #include "bus-util.h" #include "clock-util.h" +#include "conf-files.h" #include "def.h" +#include "fd-util.h" #include "fileio-label.h" #include "fileio.h" #include "fs-util.h" @@ -36,6 +38,8 @@ #define NULL_ADJTIME_UTC "0.0 0 0\n0\nUTC\n" #define NULL_ADJTIME_LOCAL "0.0 0 0\n0\nLOCAL\n" +#define UNIT_LIST_DIRS (const char* const*) CONF_PATHS_STRV("systemd/ntp-units.d") + typedef struct UnitStatusInfo { char *name; char *load_state; @@ -117,20 +121,17 @@ static int context_add_ntp_service(Context *c, const char *s) { return 0; } -static int context_parse_ntp_services(Context *c) { +static int context_parse_ntp_services_from_environment(Context *c) { const char *env, *p; int r; assert(c); env = getenv("SYSTEMD_TIMEDATED_NTP_SERVICES"); - if (!env) { - r = context_add_ntp_service(c, "systemd-timesyncd.service"); - if (r < 0) - log_warning_errno(r, "Failed to add NTP service \"systemd-timesyncd.service\", ignoring: %m"); - + if (!env) return 0; - } + + log_debug("Using list of ntp services from environment variable $SYSTEMD_TIMEDATED_NTP_SERVICES."); for (p = env;;) { _cleanup_free_ char *word = NULL; @@ -150,7 +151,62 @@ static int context_parse_ntp_services(Context *c) { log_warning_errno(r, "Failed to add NTP service \"%s\", ignoring: %m", word); } - return 0; + return 1; +} + +static int context_parse_ntp_services_from_disk(Context *c) { + _cleanup_strv_free_ char **files = NULL; + char **f; + int r; + + r = conf_files_list_strv(&files, ".list", NULL, CONF_FILES_FILTER_MASKED, UNIT_LIST_DIRS); + if (r < 0) + return log_error_errno(r, "Failed to enumerate .list files: %m"); + + STRV_FOREACH(f, files) { + _cleanup_fclose_ FILE *file = NULL; + + log_debug("Reading file '%s'", *f); + + r = fopen_unlocked(*f, "re", &file); + if (r < 0) { + log_error_errno(r, "Failed to open %s, ignoring: %m", *f); + continue; + } + + for (;;) { + _cleanup_free_ char *line = NULL; + const char *word; + + r = read_line(file, LINE_MAX, &line); + if (r < 0) { + log_error_errno(r, "Failed to read %s, ignoring: %m", *f); + continue; + } + if (r == 0) + break; + + word = strstrip(line); + if (isempty(word) || startswith("#", word)) + continue; + + r = context_add_ntp_service(c, word); + if (r < 0) + log_warning_errno(r, "Failed to add NTP service \"%s\", ignoring: %m", word); + } + } + + return 1; +} + +static int context_parse_ntp_services(Context *c) { + int r; + + r = context_parse_ntp_services_from_environment(c); + if (r != 0) + return r; + + return context_parse_ntp_services_from_disk(c); } static int context_ntp_service_is_active(Context *c) { From e5ea741c62f9747417eefd1d197ef6c8eea87047 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 22 Jul 2019 14:47:51 +0200 Subject: [PATCH 2/3] timesyncd: add ntp-units.d/ entry for timesync Priority is 80. At least in Fedora, chrony uses 50, and ntpd 60. timesyncd has lower priority, because if people install those other packages, it's most likely on purpose. timesyncd is always installed and provides less functionality. --- meson.build | 1 + src/timesync/80-systemd-timesync.list | 1 + src/timesync/meson.build | 2 ++ 3 files changed, 4 insertions(+) create mode 100644 src/timesync/80-systemd-timesync.list diff --git a/meson.build b/meson.build index 04a830cdfaa..09abd9e2ec6 100644 --- a/meson.build +++ b/meson.build @@ -157,6 +157,7 @@ systemdstatedir = join_paths(localstatedir, 'lib/systemd') catalogstatedir = join_paths(systemdstatedir, 'catalog') randomseeddir = join_paths(localstatedir, 'lib/systemd') profiledir = join_paths(rootlibexecdir, 'portable', 'profile') +ntpservicelistdir = join_paths(rootprefixdir, 'lib/systemd/ntp-units.d') docdir = get_option('docdir') if docdir == '' diff --git a/src/timesync/80-systemd-timesync.list b/src/timesync/80-systemd-timesync.list new file mode 100644 index 00000000000..d5959ade899 --- /dev/null +++ b/src/timesync/80-systemd-timesync.list @@ -0,0 +1 @@ +systemd-timesyncd.service diff --git a/src/timesync/meson.build b/src/timesync/meson.build index b79ef08277a..e5c118c8db7 100644 --- a/src/timesync/meson.build +++ b/src/timesync/meson.build @@ -32,6 +32,8 @@ if conf.get('ENABLE_TIMESYNCD') == 1 install_dir : dbuspolicydir) install_data('org.freedesktop.timesync1.service', install_dir : dbussystemservicedir) + install_data('80-systemd-timesync.list', + install_dir : ntpservicelistdir) endif ############################################################ From 971a7a1526a6c1d5cb439a8a41dc65ccd4e3a66f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 22 Jul 2019 15:58:08 +0200 Subject: [PATCH 3/3] timesyncd: add Conflicts for ntpd and chronyd Users might end up with more than one of those service enabled, through admin mistake, or broken installation scriptlets, or whatever. On my machine, I had both chronyd and timesyncd happilly running at the same time. If more than one is enabled, it's better to have just one running. Adding Conflicts will make the issue more visible in logs too. --- units/systemd-timesyncd.service.in | 1 + 1 file changed, 1 insertion(+) diff --git a/units/systemd-timesyncd.service.in b/units/systemd-timesyncd.service.in index 2d8d14f6de0..2136a85b351 100644 --- a/units/systemd-timesyncd.service.in +++ b/units/systemd-timesyncd.service.in @@ -15,6 +15,7 @@ ConditionVirtualization=!container DefaultDependencies=no After=systemd-remount-fs.service systemd-sysusers.service Before=time-set.target sysinit.target shutdown.target +Conflicts=chronyd.service ntpd.service Conflicts=shutdown.target Wants=time-set.target time-sync.target