You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
165 lines
7.3 KiB
165 lines
7.3 KiB
From db20c5cec8adf865dd47672bc091092b8cea5e0e Mon Sep 17 00:00:00 2001 |
|
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl> |
|
Date: Thu, 10 Mar 2022 15:47:12 +0100 |
|
Subject: [PATCH] shared/install: return failure when enablement fails, but |
|
process as much as possible |
|
|
|
So far we'd issue a warning (before this series, just in the logs on the server |
|
side, and before this commit, on stderr on the caller's side), but return |
|
success. It seems that successfull return was introduced by mistake in |
|
aa0f357fd833feecbea6c3e9be80b643e433bced (my fault :( ), which was supposed to |
|
be a refactoring without a functional change. I think it's better to fail, |
|
because if enablement fails, the user will most likely want to diagnose the |
|
issue. |
|
|
|
Note that we still do partial enablement, as far as that is possible. So if |
|
e.g. we have [Install] Alias=foo.service foobar, we'll create the symlink |
|
'foo.service', but not 'foobar', since that's not a valid unit name. We'll |
|
print info about the action taken, and about 'foobar' being invalid, and return |
|
failure. |
|
|
|
(cherry picked from commit 0d11db59825a9deee0b56fdede0602ef1c37c5c5) |
|
|
|
Related: #2082131 |
|
--- |
|
src/shared/install.c | 10 +++++---- |
|
test/test-systemctl-enable.sh | 39 ++++++++++++++++++----------------- |
|
2 files changed, 26 insertions(+), 23 deletions(-) |
|
|
|
diff --git a/src/shared/install.c b/src/shared/install.c |
|
index 6da9ba6b0c..a541d32fb7 100644 |
|
--- a/src/shared/install.c |
|
+++ b/src/shared/install.c |
|
@@ -1802,20 +1802,22 @@ static int install_info_symlink_alias( |
|
q = install_name_printf(scope, info, *s, info->root, &dst); |
|
if (q < 0) { |
|
unit_file_changes_add(changes, n_changes, q, *s, NULL); |
|
- return q; |
|
+ r = r < 0 ? r : q; |
|
+ continue; |
|
} |
|
|
|
q = unit_file_verify_alias(info, dst, &dst_updated, changes, n_changes); |
|
- if (q < 0) |
|
+ if (q < 0) { |
|
+ r = r < 0 ? r : q; |
|
continue; |
|
+ } |
|
|
|
alias_path = path_make_absolute(dst_updated ?: dst, config_path); |
|
if (!alias_path) |
|
return -ENOMEM; |
|
|
|
q = create_symlink(lp, info->path, alias_path, force, changes, n_changes); |
|
- if (r == 0) |
|
- r = q; |
|
+ r = r < 0 ? r : q; |
|
} |
|
|
|
return r; |
|
diff --git a/test/test-systemctl-enable.sh b/test/test-systemctl-enable.sh |
|
index 8ac1342b91..32bc6e5ef7 100644 |
|
--- a/test/test-systemctl-enable.sh |
|
+++ b/test/test-systemctl-enable.sh |
|
@@ -56,19 +56,27 @@ test ! -e "$root/etc/systemd/system/default.target.wants/test1.service" |
|
test ! -e "$root/etc/systemd/system/special.target.requires/test1.service" |
|
|
|
: -------aliases---------------------------------------------- |
|
-"$systemctl" --root="$root" enable test1 |
|
-test -h "$root/etc/systemd/system/default.target.wants/test1.service" |
|
-test -h "$root/etc/systemd/system/special.target.requires/test1.service" |
|
- |
|
cat >>"$root/etc/systemd/system/test1.service" <<EOF |
|
Alias=test1-goodalias.service |
|
Alias=test1@badalias.service |
|
Alias=test1-badalias.target |
|
Alias=test1-badalias.socket |
|
+# we have a series of good, bad, and then good again |
|
+Alias=test1-goodalias2.service |
|
EOF |
|
|
|
+"$systemctl" --root="$root" enable test1 && { echo "Expected failure" >&2; exit 1; } |
|
+test -h "$root/etc/systemd/system/default.target.wants/test1.service" |
|
+test -h "$root/etc/systemd/system/special.target.requires/test1.service" |
|
+test ! -e "$root/etc/systemd/system/test1-goodalias.service" |
|
+test -h "$root/etc/systemd/system/test1-goodalias.service" |
|
+test ! -e "$root/etc/systemd/system/test1@badalias.service" |
|
+test ! -e "$root/etc/systemd/system/test1-badalias.target" |
|
+test ! -e "$root/etc/systemd/system/test1-badalias.socket" |
|
+test -h "$root/etc/systemd/system/test1-goodalias2.service" |
|
+ |
|
: -------aliases in reeanble---------------------------------- |
|
-"$systemctl" --root="$root" reenable test1 |
|
+"$systemctl" --root="$root" reenable test1 && { echo "Expected failure" >&2; exit 1; } |
|
test -h "$root/etc/systemd/system/default.target.wants/test1.service" |
|
test ! -e "$root/etc/systemd/system/test1-goodalias.service" |
|
test -h "$root/etc/systemd/system/test1-goodalias.service" |
|
@@ -328,7 +336,7 @@ Alias=link4alias.service |
|
Alias=link4alias2.service |
|
EOF |
|
|
|
-"$systemctl" --root="$root" enable 'link4.service' |
|
+"$systemctl" --root="$root" enable 'link4.service' && { echo "Expected failure" >&2; exit 1; } |
|
test ! -h "$root/etc/systemd/system/link4.service" # this is our file |
|
test ! -h "$root/etc/systemd/system/link4@.service" |
|
test ! -h "$root/etc/systemd/system/link4@inst.service" |
|
@@ -343,18 +351,20 @@ test ! -h "$root/etc/systemd/system/link4alias.service" |
|
test ! -h "$root/etc/systemd/system/link4alias2.service" |
|
|
|
: -------systemctl enable on path to unit file---------------- |
|
+cat >"$root/etc/systemd/system/link4.service" <<EOF |
|
+[Install] |
|
+Alias=link4alias.service |
|
+Alias=link4alias2.service |
|
+EOF |
|
+ |
|
# Apparently this works. I'm not sure what to think. |
|
"$systemctl" --root="$root" enable '/etc/systemd/system/link4.service' |
|
test ! -h "$root/etc/systemd/system/link4.service" # this is our file |
|
-test ! -h "$root/etc/systemd/system/link4@.service" |
|
-test ! -h "$root/etc/systemd/system/link4@inst.service" |
|
islink "$root/etc/systemd/system/link4alias.service" "/etc/systemd/system/link4.service" |
|
islink "$root/etc/systemd/system/link4alias2.service" "/etc/systemd/system/link4.service" |
|
|
|
"$systemctl" --root="$root" disable '/etc/systemd/system/link4.service' |
|
test ! -h "$root/etc/systemd/system/link4.service" |
|
-test ! -h "$root/etc/systemd/system/link4@.service" |
|
-test ! -h "$root/etc/systemd/system/link4@inst.service" |
|
test ! -h "$root/etc/systemd/system/link4alias.service" |
|
test ! -h "$root/etc/systemd/system/link4alias2.service" |
|
|
|
@@ -364,25 +374,18 @@ cat >"$root/etc/systemd/system/link5.service" <<EOF |
|
[Install] |
|
# FIXME: self-alias should be ignored |
|
# Alias=link5.service |
|
-Alias=link5@.service |
|
-Alias=link5@inst.service |
|
Alias=link5alias.service |
|
Alias=link5alias2.service |
|
EOF |
|
|
|
"$systemctl" --root="$root" enable 'link5.service' |
|
test ! -h "$root/etc/systemd/system/link5.service" # this is our file |
|
-test ! -h "$root/etc/systemd/system/link5@.service" |
|
-test ! -h "$root/etc/systemd/system/link5@inst.service" |
|
# FIXME/CLARIFYME: will systemd think that link5alias2, link5alias, link5 are all aliases? |
|
# https://github.com/systemd/systemd/issues/661#issuecomment-1057931149 |
|
islink "$root/etc/systemd/system/link5alias.service" "/etc/systemd/system/link5.service" |
|
islink "$root/etc/systemd/system/link5alias2.service" "/etc/systemd/system/link5.service" |
|
|
|
"$systemctl" --root="$root" disable 'link5.service' |
|
-test ! -h "$root/etc/systemd/system/link5.service" |
|
-test ! -h "$root/etc/systemd/system/link5@.service" |
|
-test ! -h "$root/etc/systemd/system/link5@inst.service" |
|
test ! -h "$root/etc/systemd/system/link5alias.service" |
|
test ! -h "$root/etc/systemd/system/link5alias2.service" |
|
|
|
@@ -528,8 +531,6 @@ check_alias % '%' && { echo "Expected failure because % is not legal in unit nam |
|
|
|
check_alias z 'z' && { echo "Expected failure because %z is not known" >&2; exit 1; } |
|
|
|
-# FIXME: if there's an invalid Alias=, we shouldn't preach about empty [Install] |
|
- |
|
# TODO: repeat the tests above for presets |
|
|
|
: -------SYSTEMD_OS_RELEASE relative to root------------------
|
|
|