From 38f901791f3c4b1cbd04b71323bbef2fdab65f83 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 14 May 2023 17:48:25 +0800 Subject: [PATCH 1/2] unit-file: support UpheldBy= in [Install] settings (adding Upholds= deps from .upholds/) Closes #26896 --- man/systemctl.xml | 9 +++---- man/systemd.unit.xml | 39 +++++++++++++++------------ src/basic/unit-file.c | 5 ++-- src/core/load-dropin.c | 6 ++++- src/core/load-fragment-gperf.gperf.in | 1 + src/shared/install.c | 21 +++++++++++---- src/shared/install.h | 1 + src/systemctl/systemctl-enable.c | 6 ++--- 8 files changed, 55 insertions(+), 33 deletions(-) diff --git a/man/systemctl.xml b/man/systemctl.xml index 009aabacb9d..0e786fd966c 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -367,10 +367,9 @@ Jan 12 10:46:45 example.com bluetoothd[8900]: gatt-time-server: Input/output err Shows units required and wanted by the specified units. This recursively lists units following the - Requires=, - Requisite=, - ConsistsOf=, - Wants=, BindsTo= + Requires=, Requisite=, + Wants=, ConsistsOf=, + BindsTo=, and Upholds= dependencies. If no units are specified, default.target is implied. @@ -1791,7 +1790,7 @@ Jan 12 10:46:45 example.com bluetoothd[8900]: gatt-time-server: Input/output err Show reverse dependencies between units with list-dependencies, i.e. follow dependencies of type WantedBy=, - RequiredBy=, + RequiredBy=, UpheldBy=, PartOf=, BoundBy=, instead of Wants= and similar. diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index c618e403f71..e28bfe845e6 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -749,8 +749,7 @@ When Upholds=b.service is used on a.service, this dependency will show as UpheldBy=a.service in the property listing of - b.service. The UpheldBy= dependency cannot be specified - directly. + b.service. @@ -1848,6 +1847,12 @@ [Unit] section [Install] section + + Upholds= + UpheldBy= + [Unit] section + [Install] section + PartOf= ConsistsOf= @@ -1895,10 +1900,10 @@ - Note: WantedBy= and RequiredBy= are - used in the [Install] section to create symlinks in .wants/ - and .requires/ directories. They cannot be used directly as a - unit configuration setting. + Note: WantedBy=, RequiredBy=, and UpheldBy= + are used in the [Install] section to create symlinks in .wants/, + .requires/, and .upholds/ directories. They cannot be used + directly as a unit configuration setting. Note: ConsistsOf=, BoundBy=, RequisiteOf=, ConflictedBy= are created @@ -1947,23 +1952,23 @@ WantedBy= RequiredBy= + UpheldBy= This option may be used more than once, or a space-separated list of unit names may - be given. A symbolic link is created in the .wants/ or - .requires/ directory of each of the listed units when this unit is installed by - systemctl enable. This has the effect of a dependency of type - Wants= or Requires= being added from the listed unit to the - current unit. The primary result is that the current unit will be started when the listed unit is - started, see the description of Wants= and Requires= in the - [Unit] section for details. + be given. A symbolic link is created in the .wants/, .requires/, + or .upholds/ directory of each of the listed units when this unit is installed + by systemctl enable. This has the effect of a dependency of type + Wants=, Requires=, or Upholds= being added + from the listed unit to the current unit. See the description of the mentioned dependency types + in the [Unit] section for details. In case of template units listing non template units, the listing unit must have DefaultInstance= set, or systemctl enable must be called with an instance name. The instance (default or specified) will be added to the - .wants/ or .requires/ list of the listed unit. For example, - WantedBy=getty.target in a service getty@.service will result - in systemctl enable getty@tty2.service creating a - getty.target.wants/getty@tty2.service link to + .wants/, .requires/, or .upholds/ + list of the listed unit. For example, WantedBy=getty.target in a service + getty@.service will result in systemctl enable getty@tty2.service + creating a getty.target.wants/getty@tty2.service link to getty@.service. This also applies to listing specific instances of templated units: this specific instance will gain the dependency. A template unit may also list a template unit, in which case a generic dependency will be added where each instance of the listing unit will diff --git a/src/basic/unit-file.c b/src/basic/unit-file.c index 41422579d64..54f2137a366 100644 --- a/src/basic/unit-file.c +++ b/src/basic/unit-file.c @@ -250,9 +250,10 @@ bool lookup_paths_timestamp_hash_same(const LookupPaths *lp, uint64_t timestamp_ static int directory_name_is_valid(const char *name) { - /* Accept a directory whose name is a valid unit file name ending in .wants/, .requires/ or .d/ */ + /* Accept a directory whose name is a valid unit file name ending in .wants/, .requires/, + * .upholds/ or .d/ */ - FOREACH_STRING(suffix, ".wants", ".requires", ".d") { + FOREACH_STRING(suffix, ".wants", ".requires", ".upholds", ".d") { _cleanup_free_ char *chopped = NULL; const char *e; diff --git a/src/core/load-dropin.c b/src/core/load-dropin.c index 53d2a3daa12..fd45744261d 100644 --- a/src/core/load-dropin.c +++ b/src/core/load-dropin.c @@ -88,7 +88,7 @@ int unit_load_dropin(Unit *u) { assert(u); - /* Load dependencies from .wants and .requires directories */ + /* Load dependencies from .wants, .requires and .upholds directories */ r = process_deps(u, UNIT_WANTS, ".wants"); if (r < 0) return r; @@ -97,6 +97,10 @@ int unit_load_dropin(Unit *u) { if (r < 0) return r; + r = process_deps(u, UNIT_UPHOLDS, ".upholds"); + if (r < 0) + return r; + /* Load .conf dropins */ r = unit_find_dropin_paths(u, &l); if (r <= 0) diff --git a/src/core/load-fragment-gperf.gperf.in b/src/core/load-fragment-gperf.gperf.in index 110bccb7ad4..2a3c2c2cb8e 100644 --- a/src/core/load-fragment-gperf.gperf.in +++ b/src/core/load-fragment-gperf.gperf.in @@ -580,5 +580,6 @@ Scope.OOMPolicy, config_parse_oom_policy, Install.Alias, NULL, 0, 0 Install.WantedBy, NULL, 0, 0 Install.RequiredBy, NULL, 0, 0 +Install.UpheldBy, NULL, 0, 0 Install.Also, NULL, 0, 0 Install.DefaultInstance, NULL, 0, 0 diff --git a/src/shared/install.c b/src/shared/install.c index 152e517ebc1..7903de17b1a 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -73,7 +73,8 @@ static bool install_info_has_rules(const InstallInfo *i) { return !strv_isempty(i->aliases) || !strv_isempty(i->wanted_by) || - !strv_isempty(i->required_by); + !strv_isempty(i->required_by) || + !strv_isempty(i->upheld_by); } static bool install_info_has_also(const InstallInfo *i) { @@ -942,7 +943,7 @@ static int find_symlinks( continue; suffix = strrchr(de->d_name, '.'); - if (!STRPTR_IN_SET(suffix, ".wants", ".requires")) + if (!STRPTR_IN_SET(suffix, ".wants", ".requires", ".upholds")) continue; path = path_join(config_path, de->d_name); @@ -967,7 +968,8 @@ static int find_symlinks( log_debug_errno(r, "Failed to look up symlinks in \"%s\": %m", path); } - /* We didn't find any suitable symlinks in .wants or .requires directories, let's look for linked unit files in this directory. */ + /* We didn't find any suitable symlinks in .wants, .requires or .upholds directories, + * let's look for linked unit files in this directory. */ rewinddir(config_dir); return find_symlinks_in_directory(config_dir, config_path, root_dir, i, /* ignore_destination= */ false, @@ -1081,6 +1083,7 @@ static void install_info_clear(InstallInfo *i) { i->aliases = strv_free(i->aliases); i->wanted_by = strv_free(i->wanted_by); i->required_by = strv_free(i->required_by); + i->upheld_by = strv_free(i->upheld_by); i->also = strv_free(i->also); i->default_instance = mfree(i->default_instance); i->symlink_target = mfree(i->symlink_target); @@ -1337,6 +1340,7 @@ static int unit_file_load( { "Install", "Alias", config_parse_alias, 0, &info->aliases }, { "Install", "WantedBy", config_parse_strv, 0, &info->wanted_by }, { "Install", "RequiredBy", config_parse_strv, 0, &info->required_by }, + { "Install", "UpheldBy", config_parse_strv, 0, &info->upheld_by }, { "Install", "DefaultInstance", config_parse_default_instance, 0, info }, { "Install", "Also", config_parse_also, 0, ctx }, {} @@ -1440,7 +1444,8 @@ static int unit_file_load( return (int) strv_length(info->aliases) + (int) strv_length(info->wanted_by) + - (int) strv_length(info->required_by); + (int) strv_length(info->required_by) + + (int) strv_length(info->upheld_by); } static int unit_file_load_or_readlink( @@ -2113,6 +2118,10 @@ static int install_info_apply( if (r == 0) r = q; + q = install_info_symlink_wants(scope, file_flags, info, lp, config_path, info->upheld_by, ".upholds/", changes, n_changes); + if (r == 0) + r = q; + return r; } @@ -2733,8 +2742,10 @@ int unit_file_add_dependency( if (dep == UNIT_WANTS) l = &info->wanted_by; - else + else if (dep == UNIT_REQUIRES) l = &info->required_by; + else + l = &info->upheld_by; strv_free(*l); *l = strv_new(target_info->name); diff --git a/src/shared/install.h b/src/shared/install.h index 30b07a725fe..bc0c6db828d 100644 --- a/src/shared/install.h +++ b/src/shared/install.h @@ -92,6 +92,7 @@ struct InstallInfo { char **aliases; char **wanted_by; char **required_by; + char **upheld_by; char **also; char *default_instance; diff --git a/src/systemctl/systemctl-enable.c b/src/systemctl/systemctl-enable.c index 6d3709705e0..940f54607ff 100644 --- a/src/systemctl/systemctl-enable.c +++ b/src/systemctl/systemctl-enable.c @@ -248,9 +248,9 @@ int verb_enable(int argc, char *argv[], void *userdata) { } if (carries_install_info == 0 && !ignore_carries_install_info) - log_notice("The unit files have no installation config (WantedBy=, RequiredBy=, Also=,\n" - "Alias= settings in the [Install] section, and DefaultInstance= for template\n" - "units). This means they are not meant to be enabled or disabled using systemctl.\n" + log_notice("The unit files have no installation config (WantedBy=, RequiredBy=, UpheldBy=,\n" + "Also=, or Alias= settings in the [Install] section, and DefaultInstance= for\n" + "template units). This means they are not meant to be enabled or disabled using systemctl.\n" " \n" /* trick: the space is needed so that the line does not get stripped from output */ "Possible reasons for having this kind of units are:\n" "%1$s A unit may be statically enabled by being symlinked from another unit's\n" From b5b1351317db64de1f2c944ec153208ba8174079 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 15 May 2023 14:41:29 +0800 Subject: [PATCH 2/2] test: add tests for UpheldBy= in [Install] section --- src/test/test-install-root.c | 12 ++++++++++-- test/fuzz/fuzz-unit-file/directives-all.service | 2 ++ .../testsuite-23-upheldby-install.service | 9 +++++++++ test/units/testsuite-23.Upholds.sh | 5 ++++- 4 files changed, 25 insertions(+), 3 deletions(-) create mode 100644 test/testsuite-23.units/testsuite-23-upheldby-install.service diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c index 55b8894ecc1..80166b17c68 100644 --- a/src/test/test-install-root.c +++ b/src/test/test-install-root.c @@ -1135,6 +1135,9 @@ TEST(verify_alias) { verify_one(&plain_service, "alias.socket", -EXDEV, NULL); verify_one(&plain_service, "alias@.service", -EXDEV, NULL); verify_one(&plain_service, "alias@inst.service", -EXDEV, NULL); + + /* Setting WantedBy= and RequiredBy= through Alias= is supported for the sake of backwards + * compatibility. */ verify_one(&plain_service, "foo.target.wants/plain.service", 0, NULL); verify_one(&plain_service, "foo.target.wants/plain.socket", -EXDEV, NULL); verify_one(&plain_service, "foo.target.wants/plain@.service", -EXDEV, NULL); @@ -1143,9 +1146,14 @@ TEST(verify_alias) { verify_one(&plain_service, "foo.target.requires/plain.socket", -EXDEV, NULL); verify_one(&plain_service, "foo.target.requires/plain@.service", -EXDEV, NULL); verify_one(&plain_service, "foo.target.requires/service", -EXDEV, NULL); - verify_one(&plain_service, "foo.target.conf/plain.service", -EXDEV, NULL); - verify_one(&plain_service, "foo.service/plain.service", -EXDEV, NULL); /* missing dir suffix */ verify_one(&plain_service, "asdf.requires/plain.service", -EXDEV, NULL); /* invalid unit name component */ + /* The newly-added UpheldBy= (.upholds/) and other suffixes should be rejected */ + verify_one(&plain_service, "foo.target.upholds/plain.service", -EXDEV, NULL); + verify_one(&plain_service, "foo.target.upholds/plain.socket", -EXDEV, NULL); + verify_one(&plain_service, "foo.target.upholds/plain@.service", -EXDEV, NULL); + verify_one(&plain_service, "foo.target.upholds/service", -EXDEV, NULL); + verify_one(&plain_service, "foo.service/plain.service", -EXDEV, NULL); /* missing dir suffix */ + verify_one(&plain_service, "foo.target.conf/plain.service", -EXDEV, NULL); verify_one(&bare_template, "alias.service", -EXDEV, NULL); verify_one(&bare_template, "alias.socket", -EXDEV, NULL); diff --git a/test/fuzz/fuzz-unit-file/directives-all.service b/test/fuzz/fuzz-unit-file/directives-all.service index 8450d024e60..818fb28dbf6 100644 --- a/test/fuzz/fuzz-unit-file/directives-all.service +++ b/test/fuzz/fuzz-unit-file/directives-all.service @@ -269,6 +269,8 @@ Type= USBFunctionDescriptors= USBFunctionStrings= Unit= +UpheldBy= +Upholds= User= WakeSystem= WantedBy= diff --git a/test/testsuite-23.units/testsuite-23-upheldby-install.service b/test/testsuite-23.units/testsuite-23-upheldby-install.service new file mode 100644 index 00000000000..a4562077db4 --- /dev/null +++ b/test/testsuite-23.units/testsuite-23-upheldby-install.service @@ -0,0 +1,9 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Unit] +Description=Unit that sets UpheldBy= through [Install] + +[Service] +ExecStart=/bin/sleep infinity + +[Install] +UpheldBy=testsuite-23-retry-uphold.service diff --git a/test/units/testsuite-23.Upholds.sh b/test/units/testsuite-23.Upholds.sh index 21db258e1d2..bce4d7205b9 100755 --- a/test/units/testsuite-23.Upholds.sh +++ b/test/units/testsuite-23.Upholds.sh @@ -30,6 +30,8 @@ done systemctl stop testsuite-23-uphold.service +systemctl enable testsuite-23-upheldby-install.service + # Idea is this: # 1. we start testsuite-23-retry-uphold.service # 2. which through Uphold= starts testsuite-23-retry-upheld.service @@ -42,12 +44,13 @@ systemctl stop testsuite-23-uphold.service rm -f /tmp/testsuite-23-retry-fail systemctl start testsuite-23-retry-uphold.service +systemctl is-active testsuite-23-upheldby-install.service while ! systemctl is-failed testsuite-23-retry-fail.service ; do sleep .5 done -systemctl is-active testsuite-23-retry-upheld.service && { echo 'unexpected success'; exit 1; } +(! systemctl is-active testsuite-23-retry-upheld.service) touch /tmp/testsuite-23-retry-fail