systemctl: when removing enablement or mask symlinks, cover both /run and /etc

'systemctl disable --runtime' would disable a unit, but only if it was enabled
with '--runtime', and silently do nothing if the unit was enabled persistently.
And similarly 'systemctl disable' would do nothing if the unit was enabled in
/run. This just doesn't seem useful.

This pathch changes enable/disable and mask/unmask to be asymmetrical. enable
and mask create symlinks in /etc or /run, depending on whether --runtime was
specified. disable and unmask remove symlinks from both locations. --runtime
cannot be specified for the disable and unmask verbs.

The advantage is that 'disable' now means that the unit is disabled, period.
And similarly for 'unmask', all masks are removed.

Similarly for preset and preset-all, they now cannot be called with --runtime,
and are asymmetrical: when they enable a unit, symlinks are created in /etc.
When they disable a unit, all symlinks are nuked.

$ systemctl --root=/ enable bluetooth
Created symlink /etc/systemd/system/dbus-org.bluez.service → /usr/lib/systemd/system/bluetooth.service.
Created symlink /etc/systemd/system/bluetooth.target.wants/bluetooth.service → /usr/lib/systemd/system/bluetooth.service.
$ systemctl --root=/ --runtime enable bluetooth
Created symlink /run/systemd/system/dbus-org.bluez.service → /usr/lib/systemd/system/bluetooth.service.
Created symlink /run/systemd/system/bluetooth.target.wants/bluetooth.service → /usr/lib/systemd/system/bluetooth.service.
$ systemctl --root=/ disable bluetooth
Removed /run/systemd/system/bluetooth.target.wants/bluetooth.service.
Removed /run/systemd/system/dbus-org.bluez.service.
Removed /etc/systemd/system/bluetooth.target.wants/bluetooth.service.
Removed /etc/systemd/system/dbus-org.bluez.service.
$ systemctl --root=/ disable --runtime bluetooth
--runtime cannot be used with disable

$ systemctl --root=/ mask --runtime bluetooth
Created symlink /run/systemd/system/bluetooth.service → /dev/null.
$ systemctl --root=/ mask bluetooth
Created symlink /etc/systemd/system/bluetooth.service → /dev/null.
$ systemctl --root=/ unmask bluetooth
Removed /run/systemd/system/bluetooth.service.
Removed /etc/systemd/system/bluetooth.service.
$ systemctl --root=/ unmask --runtime bluetooth
--runtime cannot be used with unmask

$ systemctl --root=/ --runtime enable bluetooth
Created symlink /run/systemd/system/dbus-org.bluez.service → /usr/lib/systemd/system/bluetooth.service.
Created symlink /run/systemd/system/bluetooth.target.wants/bluetooth.service → /usr/lib/systemd/system/bluetooth.service.
$ systemctl --root=/ enable bluetooth
Created symlink /etc/systemd/system/dbus-org.bluez.service → /usr/lib/systemd/system/bluetooth.service.
Created symlink /etc/systemd/system/bluetooth.target.wants/bluetooth.service → /usr/lib/systemd/system/bluetooth.service.
$ systemctl --root=/ preset bluetooth
Removed /run/systemd/system/bluetooth.target.wants/bluetooth.service.
Removed /run/systemd/system/dbus-org.bluez.service.
Removed /etc/systemd/system/bluetooth.target.wants/bluetooth.service.
Removed /etc/systemd/system/dbus-org.bluez.service.
$ systemctl --root=/ preset --runtime bluetooth
--runtime cannot be used with preset

$ systemctl preset-all --runtime
--runtime cannot be used with preset-all

3 files changed, 91 insertions, 98 deletions

diff --git a/man/systemctl.xml b/man/systemctl.xml
index b5b9bec30e..7a2777664e 100644
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -570,19 +570,19 @@
         <term><option>--runtime</option></term>
 
         <listitem>
-          <para>When used with <command>enable</command>,
-          <command>disable</command>, <command>edit</command>,
-          (and related commands), make changes only temporarily, so
-          that they are lost on the next reboot. This will have the
-          effect that changes are not made in subdirectories of
-          <filename>/etc</filename> but in <filename>/run</filename>,
-          with identical immediate effects, however, since the latter
+          <para>When used with <command>set-property</command>, make changes only
+          temporarily, so that they are lost on the next reboot.</para>
+
+          <para>Similarily, when used with <command>enable</command>, <command>mask</command>,
+          <command>edit</command> and related commands, make temporary changes, which are lost on
+          the next reboot. Changes are not made in subdirectories of <filename>/etc</filename>, but
+          in <filename>/run</filename>. The immediate effect is identical, however since the latter
           is lost on reboot, the changes are lost too.</para>
 
-          <para>Similarly, when used with
-          <command>set-property</command>, make changes only
-          temporarily, so that they are lost on the next
-          reboot.</para>
+          <para>Note: this option cannot be used with <command>disable</command>,
+          <command>unmask</command>, <command>preset</command>, or <command>preset-all</command>,
+          because those operations sometimes need to remove symlinks under <filename>/etc</filename>
+          to have the desired effect, which would cause a persistent change.</para>
         </listitem>
       </varlistentry>
 
diff --git a/src/shared/install.c b/src/shared/install.c
index e246cd727c..4cf40df97d 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -1916,7 +1916,6 @@ static int install_context_mark_for_removal(
                 InstallContext *c,
                 const LookupPaths *paths,
                 Set **remove_symlinks_to,
-                const char *config_path,
                 UnitFileChange **changes,
                 size_t *n_changes) {
 
@@ -1925,7 +1924,6 @@ static int install_context_mark_for_removal(
 
         assert(c);
         assert(paths);
-        assert(config_path);
 
         /* Marks all items for removal */
 
@@ -2035,7 +2033,7 @@ int unit_file_unmask(
         size_t n_todo = 0, n_allocated = 0;
         const char *config_path;
         char **i;
-        bool dry_run;
+        bool dry_run = !!(flags & UNIT_FILE_DRY_RUN);
         int r, q;
 
         assert(scope >= 0);
@@ -2045,73 +2043,71 @@ int unit_file_unmask(
         if (r < 0)
                 return r;
 
-        config_path = (flags & UNIT_FILE_RUNTIME) ? paths.runtime_config : paths.persistent_config;
-        if (!config_path)
-                return -ENXIO;
-
-        dry_run = !!(flags & UNIT_FILE_DRY_RUN);
-
         STRV_FOREACH(i, files) {
-                _cleanup_free_ char *path = NULL;
-
                 if (!unit_name_is_valid(*i, UNIT_NAME_ANY))
                         return -EINVAL;
 
-                path = path_make_absolute(*i, config_path);
-                if (!path)
-                        return -ENOMEM;
+                FOREACH_STRING(config_path, paths.runtime_config, paths.persistent_config) {
+                        _cleanup_free_ char *path = NULL;
 
-                r = null_or_empty_path(path);
-                if (r == -ENOENT)
-                        continue;
-                if (r < 0)
-                        return r;
-                if (r == 0)
-                        continue;
+                        path = path_make_absolute(*i, config_path);
+                        if (!path)
+                                return -ENOMEM;
 
-                if (!GREEDY_REALLOC0(todo, n_allocated, n_todo + 2))
-                        return -ENOMEM;
+                        r = null_or_empty_path(path);
+                        if (r == -ENOENT)
+                                continue;
+                        if (r < 0)
+                                return r;
+                        if (r == 0)
+                                continue;
 
-                todo[n_todo] = strdup(*i);
-                if (!todo[n_todo])
-                        return -ENOMEM;
+                        if (!GREEDY_REALLOC0(todo, n_allocated, n_todo + 2))
+                                return -ENOMEM;
 
-                n_todo++;
+                        todo[n_todo] = strdup(*i);
+                        if (!todo[n_todo])
+                                return -ENOMEM;
+
+                        n_todo++;
+                }
         }
 
         strv_uniq(todo);
 
         r = 0;
-        STRV_FOREACH(i, todo) {
-                _cleanup_free_ char *path = NULL;
-                const char *rp;
+        FOREACH_STRING(config_path, paths.runtime_config, paths.persistent_config) {
+                STRV_FOREACH(i, todo) {
+                        _cleanup_free_ char *path = NULL;
+                        const char *rp;
 
-                path = path_make_absolute(*i, config_path);
-                if (!path)
-                        return -ENOMEM;
+                        path = path_make_absolute(*i, config_path);
+                        if (!path)
+                                return -ENOMEM;
 
-                if (!dry_run && unlink(path) < 0) {
-                        if (errno != ENOENT) {
-                                if (r >= 0)
-                                        r = -errno;
-                                unit_file_changes_add(changes, n_changes, -errno, path, NULL);
+                        if (!dry_run && unlink(path) < 0) {
+                                if (errno != ENOENT) {
+                                        if (r >= 0)
+                                                r = -errno;
+                                        unit_file_changes_add(changes, n_changes, -errno, path, NULL);
+                                }
+
+                                continue;
                         }
 
-                        continue;
-                }
+                        unit_file_changes_add(changes, n_changes, UNIT_FILE_UNLINK, path, NULL);
 
-                unit_file_changes_add(changes, n_changes, UNIT_FILE_UNLINK, path, NULL);
+                        rp = skip_root(&paths, path);
+                        q = mark_symlink_for_removal(&remove_symlinks_to, rp ?: path);
+                        if (q < 0)
+                                return q;
+                }
 
-                rp = skip_root(&paths, path);
-                q = mark_symlink_for_removal(&remove_symlinks_to, rp ?: path);
-                if (q < 0)
-                        return q;
+                q = remove_marked_symlinks(remove_symlinks_to, config_path, &paths, dry_run, changes, n_changes);
+                if (r >= 0)
+                        r = q;
         }
 
-        q = remove_marked_symlinks(remove_symlinks_to, config_path, &paths, dry_run, changes, n_changes);
-        if (r >= 0)
-                r = q;
-
         return r;
 }
 
@@ -2503,6 +2499,7 @@ int unit_file_disable(
         _cleanup_(lookup_paths_free) LookupPaths paths = {};
         _cleanup_(install_context_done) InstallContext c = {};
         _cleanup_set_free_free_ Set *remove_symlinks_to = NULL;
+        bool dry_run = !!(flags & UNIT_FILE_DRY_RUN);
         const char *config_path;
         char **i;
         int r;
@@ -2514,10 +2511,6 @@ int unit_file_disable(
         if (r < 0)
                 return r;
 
-        config_path = (flags & UNIT_FILE_RUNTIME) ? paths.runtime_config : paths.persistent_config;
-        if (!config_path)
-                return -ENXIO;
-
         STRV_FOREACH(i, files) {
                 if (!unit_name_is_valid(*i, UNIT_NAME_ANY))
                         return -EINVAL;
@@ -2527,11 +2520,17 @@ int unit_file_disable(
                         return r;
         }
 
-        r = install_context_mark_for_removal(scope, &c, &paths, &remove_symlinks_to, config_path, changes, n_changes);
+        r = install_context_mark_for_removal(scope, &c, &paths, &remove_symlinks_to, changes, n_changes);
         if (r < 0)
                 return r;
 
-        return remove_marked_symlinks(remove_symlinks_to, config_path, &paths, !!(flags & UNIT_FILE_DRY_RUN), changes, n_changes);
+        FOREACH_STRING(config_path, paths.runtime_config, paths.persistent_config) {
+                r = remove_marked_symlinks(remove_symlinks_to, config_path, &paths, dry_run, changes, n_changes);
+                if (r < 0)
+                        return r;
+        }
+
+        return 0;
 }
 
 int unit_file_reenable(
@@ -2915,45 +2914,45 @@ int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char
 
 static int execute_preset(
                 UnitFileScope scope,
+                UnitFileFlags flags,
                 InstallContext *plus,
                 InstallContext *minus,
                 const LookupPaths *paths,
-                const char *config_path,
                 char **files,
                 UnitFilePresetMode mode,
-                bool force,
                 UnitFileChange **changes,
                 size_t *n_changes) {
 
-        int r;
+        const char *config_path;
+        bool force = !!(flags & UNIT_FILE_FORCE);
+        bool runtime = !!(flags & UNIT_FILE_RUNTIME);
+        int r = 0, q;
 
         assert(plus);
         assert(minus);
         assert(paths);
-        assert(config_path);
 
         if (mode != UNIT_FILE_PRESET_ENABLE_ONLY) {
                 _cleanup_set_free_free_ Set *remove_symlinks_to = NULL;
 
-                r = install_context_mark_for_removal(scope, minus, paths, &remove_symlinks_to, config_path, changes, n_changes);
-                if (r < 0)
-                        return r;
+                q = install_context_mark_for_removal(scope, minus, paths, &remove_symlinks_to, changes, n_changes);
+                if (q < 0)
+                        return q;
 
-                r = remove_marked_symlinks(remove_symlinks_to, config_path, paths, false, changes, n_changes);
-        } else
-                r = 0;
+                FOREACH_STRING(config_path, paths->runtime_config, paths->persistent_config) {
+                        q = remove_marked_symlinks(remove_symlinks_to, config_path, paths, false, changes, n_changes);
+                        if (r == 0)
+                                r = q;
+                }
+        }
 
         if (mode != UNIT_FILE_PRESET_DISABLE_ONLY) {
-                int q;
-
                 /* Returns number of symlinks that where supposed to be installed. */
-                q = install_context_apply(scope, plus, paths, config_path, force, SEARCH_LOAD, changes, n_changes);
-                if (r >= 0) {
-                        if (q < 0)
-                                r = q;
-                        else
-                                r += q;
-                }
+                q = install_context_apply(scope, plus, paths,
+                                          runtime ? paths->runtime_config : paths->persistent_config,
+                                          force, SEARCH_LOAD, changes, n_changes);
+                if (r == 0)
+                        r = q;
         }
 
         return r;
@@ -3017,7 +3016,6 @@ int unit_file_preset(
         _cleanup_(install_context_done) InstallContext plus = {}, minus = {};
         _cleanup_(lookup_paths_free) LookupPaths paths = {};
         _cleanup_(presets_freep) Presets presets = {};
-        const char *config_path;
         char **i;
         int r;
 
@@ -3029,10 +3027,6 @@ int unit_file_preset(
         if (r < 0)
                 return r;
 
-        config_path = (flags & UNIT_FILE_RUNTIME) ? paths.runtime_config : paths.persistent_config;
-        if (!config_path)
-                return -ENXIO;
-
         r = read_presets(scope, root_dir, &presets);
         if (r < 0)
                 return r;
@@ -3043,7 +3037,7 @@ int unit_file_preset(
                         return r;
         }
 
-        return execute_preset(scope, &plus, &minus, &paths, config_path, files, mode, !!(flags & UNIT_FILE_FORCE), changes, n_changes);
+        return execute_preset(scope, flags, &plus, &minus, &paths, files, mode, changes, n_changes);
 }
 
 int unit_file_preset_all(
@@ -3057,7 +3051,6 @@ int unit_file_preset_all(
         _cleanup_(install_context_done) InstallContext plus = {}, minus = {};
         _cleanup_(lookup_paths_free) LookupPaths paths = {};
         _cleanup_(presets_freep) Presets presets = {};
-        const char *config_path = NULL;
         char **i;
         int r;
 
@@ -3069,10 +3062,6 @@ int unit_file_preset_all(
         if (r < 0)
                 return r;
 
-        config_path = (flags & UNIT_FILE_RUNTIME) ? paths.runtime_config : paths.persistent_config;
-        if (!config_path)
-                return -ENXIO;
-
         r = read_presets(scope, root_dir, &presets);
         if (r < 0)
                 return r;
@@ -3090,7 +3079,6 @@ int unit_file_preset_all(
                 }
 
                 FOREACH_DIRENT(de, d, return -errno) {
-
                         if (!unit_name_is_valid(de->d_name, UNIT_NAME_ANY))
                                 continue;
 
@@ -3114,7 +3102,7 @@ int unit_file_preset_all(
                 }
         }
 
-        return execute_preset(scope, &plus, &minus, &paths, config_path, NULL, mode, !!(flags & UNIT_FILE_FORCE), changes, n_changes);
+        return execute_preset(scope, flags, &plus, &minus, &paths, NULL, mode, changes, n_changes);
 }
 
 static void unit_file_list_free_one(UnitFileList *f) {
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index fbaa464991..11762c7c41 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -7685,6 +7685,11 @@ static int systemctl_parse_argv(int argc, char *argv[]) {
                 return -EINVAL;
         }
 
+        if (arg_runtime && STRPTR_IN_SET(argv[optind], "disable", "unmask", "preset", "preset-all")) {
+                log_error("--runtime cannot be used with %s", argv[optind]);
+                return -EINVAL;
+        }
+
         return 1;
 }