Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[le12] systemd: fix non-working service aliases #9620

Merged
merged 1 commit into from
Dec 28, 2024
+329 −0
Merged

[le12] systemd: fix non-working service aliases #9620

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 136 additions & 0 deletions packages/sysutils/systemd/patches/systemd-0400-use-RET_GATHER-more.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
From b2751cf0394d36c24590b5f7b33e9f864b57ba0d Mon Sep 17 00:00:00 2001
From: Mike Yuan <[email protected]>
Date: Thu, 29 Feb 2024 18:53:26 +0800
Subject: [PATCH] shared/install: use RET_GATHER more

---
src/shared/install.c | 54 ++++++++++++++++++--------------------------
1 file changed, 22 insertions(+), 32 deletions(-)

diff --git a/src/shared/install.c b/src/shared/install.c
index 32f504c32763a..42b725c076dda 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -1892,7 +1892,7 @@ static int install_info_symlink_alias(
InstallChange **changes,
size_t *n_changes) {

- int r = 0, q;
+ int r, ret = 0;

assert(info);
assert(lp);
@@ -1900,20 +1900,17 @@ static int install_info_symlink_alias(

STRV_FOREACH(s, info->aliases) {
_cleanup_free_ char *alias_path = NULL, *dst = NULL, *dst_updated = NULL;
- bool broken;

- q = install_name_printf(scope, info, *s, &dst);
- if (q < 0) {
- install_changes_add(changes, n_changes, q, *s, NULL);
- r = r < 0 ? r : q;
+ r = install_name_printf(scope, info, *s, &dst);
+ if (r < 0) {
+ RET_GATHER(ret, install_changes_add(changes, n_changes, r, *s, NULL));
continue;
}

- q = unit_file_verify_alias(info, dst, &dst_updated, changes, n_changes);
- if (q == -ELOOP)
- continue;
- if (q < 0) {
- r = r < 0 ? r : q;
+ r = unit_file_verify_alias(info, dst, &dst_updated, changes, n_changes);
+ if (r < 0) {
+ if (r != -ELOOP)
+ RET_GATHER(ret, r);
continue;
}

@@ -1921,18 +1918,18 @@ static int install_info_symlink_alias(
if (!alias_path)
return -ENOMEM;

- q = chase(alias_path, lp->root_dir, CHASE_NONEXISTENT, NULL, NULL);
- if (q < 0 && q != -ENOENT) {
- r = r < 0 ? r : q;
+ bool broken;
+ r = chase(alias_path, lp->root_dir, CHASE_NONEXISTENT, /* ret_path = */ NULL, /* ret_fd = */ NULL);
+ if (r < 0 && r != -ENOENT) {
+ RET_GATHER(ret, r);
continue;
}
- broken = q == 0; /* symlink target does not exist? */
+ broken = r == 0; /* symlink target does not exist? */

- q = create_symlink(lp, info->path, alias_path, force || broken, changes, n_changes);
- r = r < 0 ? r : q;
+ RET_GATHER(ret, create_symlink(lp, info->path, alias_path, force || broken, changes, n_changes));
}

- return r;
+ return ret;
}

static int install_info_symlink_wants(
@@ -1995,10 +1992,7 @@ static int install_info_symlink_wants(

q = install_name_printf(scope, info, *s, &dst);
if (q < 0) {
- install_changes_add(changes, n_changes, q, *s, NULL);
- if (r >= 0)
- r = q;
-
+ RET_GATHER(r, install_changes_add(changes, n_changes, q, *s, NULL));
continue;
}

@@ -2010,15 +2004,13 @@ static int install_info_symlink_wants(
* 'systemctl enable [email protected]' should fail, the user should specify an
* instance like in 'systemctl enable [email protected]'.
*/
- if (file_flags & UNIT_FILE_IGNORE_AUXILIARY_FAILURE)
+ if (FLAGS_SET(file_flags, UNIT_FILE_IGNORE_AUXILIARY_FAILURE))
continue;

if (unit_name_is_valid(dst, UNIT_NAME_ANY))
- q = install_changes_add(changes, n_changes, -EIDRM, dst, n);
+ RET_GATHER(r, install_changes_add(changes, n_changes, -EIDRM, dst, n));
else
- q = install_changes_add(changes, n_changes, -EUCLEAN, dst, NULL);
- if (r >= 0)
- r = q;
+ RET_GATHER(r, install_changes_add(changes, n_changes, -EUCLEAN, dst, NULL));

continue;
}
@@ -2027,7 +2019,7 @@ static int install_info_symlink_wants(
if (!path)
return -ENOMEM;

- q = create_symlink(lp, info->path, path, true, changes, n_changes);
+ q = create_symlink(lp, info->path, path, /* force = */ true, changes, n_changes);
if ((q < 0 && r >= 0) || r == 0)
r = q;

@@ -2418,7 +2410,7 @@ int unit_file_link(
_cleanup_strv_free_ char **todo = NULL;
const char *config_path;
size_t n_todo = 0;
- int r, q;
+ int r;

assert(scope >= 0);
assert(scope < _RUNTIME_SCOPE_MAX);
@@ -2487,9 +2479,7 @@ int unit_file_link(
if (!new_path)
return -ENOMEM;

- q = create_symlink(&lp, *i, new_path, flags & UNIT_FILE_FORCE, changes, n_changes);
- if (q < 0 && r >= 0)
- r = q;
+ RET_GATHER(r, create_symlink(&lp, *i, new_path, flags & UNIT_FILE_FORCE, changes, n_changes));
}

return r;
193 changes: 193 additions & 0 deletions packages/sysutils/systemd/patches/systemd-0500-install-alias-for-units-outside.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
From 9db7710a3e4ae7900b22c50b21479c941b3f33ed Mon Sep 17 00:00:00 2001
From: Nick Rosbrook <[email protected]>
Date: Fri, 15 Mar 2024 15:14:05 -0400
Subject: [PATCH] shared/install: correctly install alias for units outside
search path
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Currently, if a unit file is enabled from outside of the search path,
and that unit has an alias, then the symlink ends up pointing outside of
the search path too. For example:

$ cat /tmp/a.service
[Service]
ExecStart=sleep infinity

[Install]
Alias=b.service
WantedBy=multi-user.target

$ systemctl enable /tmp/a.service
Created symlink /etc/systemd/system/a.service → /tmp/a.service.
Created symlink /etc/systemd/system/b.service → /tmp/a.service.
Created symlink /etc/systemd/system/multi-user.target.wants/a.service → /tmp/a.service.

This then means the alias is treated as a separate unit:

$ systemctl start a.service
$ sudo systemctl status a
● a.service
Loaded: loaded (/etc/systemd/system/a.service; enabled; preset: enabled)
Active: active (running) since Fri 2024-03-15 15:17:49 EDT; 9s ago
Main PID: 769593 (sleep)
Tasks: 1 (limit: 18898)
Memory: 220.0K
CPU: 5ms
CGroup: /system.slice/a.service
└─769593 sleep infinity

Mar 15 15:17:49 six systemd[1]: Started a.service.
$ sudo systemctl status b
○ b.service
Loaded: loaded (/etc/systemd/system/b.service; alias)
Active: inactive (dead)

To fix this, make sure the alias uses a target that is inside the search
path. Since the unit file itself is outside of the search path, a
symlink inside the search path will have been created already. Hence,
just point the alias symlink to that recently created symlink.
---
src/shared/install.c | 16 ++++++++++++++--
src/test/test-install-root.c | 25 +++++++++++++++++++------
test/test-systemctl-enable.sh | 4 ++--
3 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/src/shared/install.c b/src/shared/install.c
index 81f898f28a477..12bf083a2e60d 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -1941,7 +1941,7 @@ static int install_info_symlink_alias(
assert(config_path);

STRV_FOREACH(s, info->aliases) {
- _cleanup_free_ char *alias_path = NULL, *dst = NULL, *dst_updated = NULL;
+ _cleanup_free_ char *alias_path = NULL, *alias_target = NULL, *dst = NULL, *dst_updated = NULL;

r = install_name_printf(scope, info, *s, &dst);
if (r < 0) {
@@ -1960,6 +1960,18 @@ static int install_info_symlink_alias(
if (!alias_path)
return -ENOMEM;

+ r = in_search_path(lp, info->path);
+ if (r < 0)
+ return r;
+ if (r == 0) {
+ /* The unit path itself is outside of the search path. To
+ * correctly apply the alias, we need the alias symlink to
+ * point to the symlink that was created in the search path. */
+ alias_target = path_join(config_path, info->name);
+ if (!alias_target)
+ return -ENOMEM;
+ }
+
bool broken;
r = chase(alias_path, lp->root_dir, CHASE_NONEXISTENT, /* ret_path = */ NULL, /* ret_fd = */ NULL);
if (r < 0 && r != -ENOENT) {
@@ -1968,7 +1980,7 @@ static int install_info_symlink_alias(
}
broken = r == 0; /* symlink target does not exist? */

- RET_GATHER(ret, create_symlink(lp, info->path, alias_path, force || broken, changes, n_changes));
+ RET_GATHER(ret, create_symlink(lp, alias_target ?: info->path, alias_path, force || broken, changes, n_changes));
}

return ret;
diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c
index efd75b2a6798b..c55445079c6c6 100644
--- a/src/test/test-install-root.c
+++ b/src/test/test-install-root.c
@@ -200,7 +200,7 @@ TEST(basic_mask_and_enable) {
}

TEST(linked_units) {
- const char *p, *q;
+ const char *p, *q, *s;
UnitFileState state;
InstallChange *changes = NULL;
size_t n_changes = 0, i;
@@ -224,6 +224,7 @@ TEST(linked_units) {
p = strjoina(root, "/opt/linked.service");
assert_se(write_string_file(p,
"[Install]\n"
+ "Alias=linked-alias.service\n"
"WantedBy=multi-user.target\n", WRITE_STRING_FILE_CREATE) >= 0);

p = strjoina(root, "/opt/linked2.service");
@@ -275,31 +276,41 @@ TEST(linked_units) {

/* Now, let's not just link it, but also enable it */
assert_se(unit_file_enable(RUNTIME_SCOPE_SYSTEM, 0, root, STRV_MAKE("/opt/linked.service"), &changes, &n_changes) >= 0);
- assert_se(n_changes == 2);
+ assert_se(n_changes == 3);
p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/linked.service");
q = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/linked.service");
+ s = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/linked-alias.service");
for (i = 0 ; i < n_changes; i++) {
assert_se(changes[i].type == INSTALL_CHANGE_SYMLINK);
- assert_se(streq(changes[i].source, "/opt/linked.service"));
+
+ if (s && streq(changes[i].path, s))
+ /* The alias symlink should point within the search path. */
+ assert_se(streq(changes[i].source, SYSTEM_CONFIG_UNIT_DIR"/linked.service"));
+ else
+ assert_se(streq(changes[i].source, "/opt/linked.service"));

if (p && streq(changes[i].path, p))
p = NULL;
else if (q && streq(changes[i].path, q))
q = NULL;
+ else if (s && streq(changes[i].path, s))
+ s = NULL;
else
assert_not_reached();
}
- assert_se(!p && !q);
+ assert_se(!p && !q && !s);
install_changes_free(changes, n_changes);
changes = NULL; n_changes = 0;

assert_se(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "linked.service", &state) >= 0 && state == UNIT_FILE_ENABLED);
+ assert_se(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "linked-alias.service", &state) >= 0 && state == UNIT_FILE_ALIAS);

/* And let's unlink it again */
assert_se(unit_file_disable(RUNTIME_SCOPE_SYSTEM, 0, root, STRV_MAKE("linked.service"), &changes, &n_changes) >= 0);
- assert_se(n_changes == 2);
+ assert_se(n_changes == 3);
p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/multi-user.target.wants/linked.service");
q = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/linked.service");
+ s = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/linked-alias.service");
for (i = 0; i < n_changes; i++) {
assert_se(changes[i].type == INSTALL_CHANGE_UNLINK);

@@ -307,10 +318,12 @@ TEST(linked_units) {
p = NULL;
else if (q && streq(changes[i].path, q))
q = NULL;
+ else if (s && streq(changes[i].path, s))
+ s = NULL;
else
assert_not_reached();
}
- assert_se(!p && !q);
+ assert_se(!p && !q && !s);
install_changes_free(changes, n_changes);
changes = NULL; n_changes = 0;

diff --git a/test/test-systemctl-enable.sh b/test/test-systemctl-enable.sh
index 8427f6849bfd9..5615c900f4861 100644
--- a/test/test-systemctl-enable.sh
+++ b/test/test-systemctl-enable.sh
@@ -534,8 +534,8 @@ test ! -h "$root/etc/systemd/system/link5alias2.service"

"$systemctl" --root="$root" enable '/link5copy.service'
islink "$root/etc/systemd/system/link5copy.service" '/link5copy.service'
-islink "$root/etc/systemd/system/link5alias.service" '/link5copy.service'
-islink "$root/etc/systemd/system/link5alias2.service" '/link5copy.service'
+islink "$root/etc/systemd/system/link5alias.service" '/etc/systemd/system/link5copy.service'
+islink "$root/etc/systemd/system/link5alias2.service" '/etc/systemd/system/link5copy.service'

"$systemctl" --root="$root" disable 'link5copy.service'
test ! -h "$root/etc/systemd/system/link5copy.service"