| From 3cb3b0145ed8439eb604b43596e6456ed3292c46 Mon Sep 17 00:00:00 2001 |
| From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl> |
| Date: Sun, 21 Aug 2016 09:10:51 -0400 |
| Subject: [PATCH] shared/install: do not enable masked instances (#4005) |
| MIME-Version: 1.0 |
| Content-Type: text/plain; charset=UTF-8 |
| Content-Transfer-Encoding: 8bit |
| |
| When told to enable a template unit, and the DefaultInstance specified in that |
| unit was masked, we would do this. Such a unit cannot be started or loaded, so |
| reporting successful enabling is misleading and unexpected. |
| |
| $ systemctl mask getty@tty1 |
| Created symlink /etc/systemd/system/getty@tty1.service → /dev/null. |
| $ systemctl --root=/ enable getty@tty1 |
| (unchanged) |
| Failed to enable unit, unit /etc/systemd/system/getty@tty1.service is masked. |
| |
| $ systemctl --root=/ enable getty@ |
| (before) |
| Created symlink /etc/systemd/system/getty.target.wants/getty@tty1.service → /usr/lib/systemd/system/getty@.service. |
| (now) |
| Failed to enable unit, unit /etc/systemd/system/getty@tty1.service is masked. |
| |
| The same error is emitted for enable and preset. And an error is emmited, not a |
| warning, so the failure to enable DefaultInstance is treated the same as if the |
| instance was specified on the command line. I think that this makes most sense, |
| for most template units. |
| |
| Fixes #2513. |
| (cherry picked from commit 047d91f9c8cf1bcf5a30f428668babd619533944) |
| --- |
| src/shared/install.c | 35 ++++++++++++++++++++++++++++------- |
| 1 file changed, 28 insertions(+), 7 deletions(-) |
| |
| diff --git a/src/shared/install.c b/src/shared/install.c |
| index 9d9f4dff4f..cb2a2e7e0d 100644 |
| --- a/src/shared/install.c |
| +++ b/src/shared/install.c |
| @@ -912,8 +912,8 @@ static int install_info_may_process( |
| assert(i); |
| assert(paths); |
| |
| - /* Checks whether the loaded unit file is one we should process, or is masked, transient or generated and thus |
| - * not subject to enable/disable operations. */ |
| + /* Checks whether the loaded unit file is one we should process, or is masked, |
| + * transient or generated and thus not subject to enable/disable operations. */ |
| |
| if (i->type == UNIT_FILE_TYPE_MASKED) { |
| unit_file_changes_add(changes, n_changes, -ERFKILL, i->path, NULL); |
| @@ -1134,7 +1134,6 @@ static int unit_file_load( |
| struct stat st; |
| int r; |
| |
| - assert(c); |
| assert(info); |
| assert(path); |
| |
| @@ -1163,6 +1162,9 @@ static int unit_file_load( |
| return 0; |
| } |
| |
| + /* c is only needed if we actually load the file */ |
| + assert(c); |
| + |
| fd = open(path, O_RDONLY|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW); |
| if (fd < 0) |
| return -errno; |
| @@ -1275,7 +1277,6 @@ static int unit_file_search( |
| char **p; |
| int r; |
| |
| - assert(c); |
| assert(info); |
| assert(paths); |
| |
| @@ -1546,7 +1547,14 @@ static int install_info_symlink_wants( |
| assert(paths); |
| assert(config_path); |
| |
| + if (strv_isempty(list)) |
| + return 0; |
| + |
| if (unit_name_is_valid(i->name, UNIT_NAME_TEMPLATE)) { |
| + UnitFileInstallInfo instance = { |
| + .type = _UNIT_FILE_TYPE_INVALID, |
| + }; |
| + _cleanup_free_ char *path = NULL; |
| |
| /* Don't install any symlink if there's no default |
| * instance configured */ |
| @@ -1558,6 +1566,19 @@ static int install_info_symlink_wants( |
| if (r < 0) |
| return r; |
| |
| + instance.name = buf; |
| + r = unit_file_search(NULL, &instance, paths, SEARCH_FOLLOW_CONFIG_SYMLINKS); |
| + if (r < 0) |
| + return r; |
| + |
| + path = instance.path; |
| + instance.path = NULL; |
| + |
| + if (instance.type == UNIT_FILE_TYPE_MASKED) { |
| + unit_file_changes_add(changes, n_changes, -ERFKILL, path, NULL); |
| + return -ERFKILL; |
| + } |
| + |
| n = buf; |
| } else |
| n = i->name; |
| @@ -1687,12 +1708,12 @@ static int install_context_apply( |
| return r; |
| |
| /* We can attempt to process a masked unit when a different unit |
| - * that we were processing specifies it in DefaultInstance= or Also=. */ |
| + * that we were processing specifies it in Also=. */ |
| if (i->type == UNIT_FILE_TYPE_MASKED) { |
| unit_file_changes_add(changes, n_changes, UNIT_FILE_IS_MASKED, i->path, NULL); |
| if (r >= 0) |
| - /* Assume that some *could* have been enabled here, avoid |
| - * "empty [Install] section" warning. */ |
| + /* Assume that something *could* have been enabled here, |
| + * avoid "empty [Install] section" warning. */ |
| r += 1; |
| continue; |
| } |