From 17488e1ed175cfeb2e515a82d291b7f7b28e2a3b Mon Sep 17 00:00:00 2001 From: Michael Biebl Date: Wed, 27 Jun 2018 14:31:42 +0200 Subject: [PATCH] Revert "systemctl: when removing enablement or mask symlinks, cover both /run and /etc" We currently have packages in the archive which use "systemctl --runtime unmask" and are broken by this change. This is a intermediate step until it is clear whether upstream will revert this commit or whether we will have to update affected packages to deal with this changed behaviour. See #902287 and https://github.com/systemd/systemd/issues/9393 This reverts commit 4910b35078ad24dcbc63f372b2fee087640201d0. Gbp-Pq: Topic debian Gbp-Pq: Name Revert-systemctl-when-removing-enablement-or-mask-symlink.patch --- man/systemctl.xml | 22 +++--- src/shared/install.c | 162 ++++++++++++++++++++------------------ src/systemctl/systemctl.c | 5 -- 3 files changed, 98 insertions(+), 91 deletions(-) diff --git a/man/systemctl.xml b/man/systemctl.xml index d95d3726..abf3ea49 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -557,19 +557,19 @@ - When used with set-property, make changes only - temporarily, so that they are lost on the next reboot. - - Similarily, when used with enable, mask, - edit and related commands, make temporary changes, which are lost on - the next reboot. Changes are not made in subdirectories of /etc, but - in /run. The immediate effect is identical, however since the latter + When used with enable, + disable, edit, + (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 + /etc but in /run, + with identical immediate effects, however, since the latter is lost on reboot, the changes are lost too. - Note: this option cannot be used with disable, - unmask, preset, or preset-all, - because those operations sometimes need to remove symlinks under /etc - to have the desired effect, which would cause a persistent change. + Similarly, when used with + set-property, make changes only + temporarily, so that they are lost on the next + reboot. diff --git a/src/shared/install.c b/src/shared/install.c index 77ae8128..e53f09fa 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -1911,6 +1911,7 @@ 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) { @@ -1919,6 +1920,7 @@ static int install_context_mark_for_removal( assert(c); assert(paths); + assert(config_path); /* Marks all items for removal */ @@ -2028,7 +2030,7 @@ int unit_file_unmask( size_t n_todo = 0, n_allocated = 0; const char *config_path; char **i; - bool dry_run = !!(flags & UNIT_FILE_DRY_RUN); + bool dry_run; int r, q; assert(scope >= 0); @@ -2038,71 +2040,73 @@ 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; - FOREACH_STRING(config_path, paths.runtime_config, paths.persistent_config) { - _cleanup_free_ char *path = NULL; - - path = path_make_absolute(*i, config_path); - if (!path) - return -ENOMEM; + path = path_make_absolute(*i, config_path); + if (!path) + return -ENOMEM; - r = null_or_empty_path(path); - if (r == -ENOENT) - continue; - if (r < 0) - return r; - if (r == 0) - continue; + r = null_or_empty_path(path); + if (r == -ENOENT) + continue; + if (r < 0) + return r; + if (r == 0) + continue; - if (!GREEDY_REALLOC0(todo, n_allocated, n_todo + 2)) - return -ENOMEM; + if (!GREEDY_REALLOC0(todo, n_allocated, n_todo + 2)) + return -ENOMEM; - todo[n_todo] = strdup(*i); - if (!todo[n_todo]) - return -ENOMEM; + todo[n_todo] = strdup(*i); + if (!todo[n_todo]) + return -ENOMEM; - n_todo++; - } + n_todo++; } strv_uniq(todo); r = 0; - 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; + STRV_FOREACH(i, todo) { + _cleanup_free_ char *path = NULL; + const char *rp; - if (!dry_run && unlink(path) < 0) { - if (errno != ENOENT) { - if (r >= 0) - r = -errno; - unit_file_changes_add(changes, n_changes, -errno, path, NULL); - } + path = path_make_absolute(*i, config_path); + if (!path) + return -ENOMEM; - continue; + if (!dry_run && unlink(path) < 0) { + if (errno != ENOENT) { + if (r >= 0) + r = -errno; + unit_file_changes_add(changes, n_changes, -errno, 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; + continue; } - q = remove_marked_symlinks(remove_symlinks_to, config_path, &paths, dry_run, changes, n_changes); - if (r >= 0) - r = q; + 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; } + q = remove_marked_symlinks(remove_symlinks_to, config_path, &paths, dry_run, changes, n_changes); + if (r >= 0) + r = q; + return r; } @@ -2494,7 +2498,6 @@ 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; @@ -2506,6 +2509,10 @@ 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; @@ -2515,17 +2522,11 @@ int unit_file_disable( return r; } - r = install_context_mark_for_removal(scope, &c, &paths, &remove_symlinks_to, changes, n_changes); + r = install_context_mark_for_removal(scope, &c, &paths, &remove_symlinks_to, config_path, changes, n_changes); if (r < 0) return r; - 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; + return remove_marked_symlinks(remove_symlinks_to, config_path, &paths, !!(flags & UNIT_FILE_DRY_RUN), changes, n_changes); } int unit_file_reenable( @@ -2909,45 +2910,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) { - const char *config_path; - bool force = !!(flags & UNIT_FILE_FORCE); - bool runtime = !!(flags & UNIT_FILE_RUNTIME); - int r = 0, q; + int r; 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; - q = install_context_mark_for_removal(scope, minus, paths, &remove_symlinks_to, changes, n_changes); - if (q < 0) - return q; + r = install_context_mark_for_removal(scope, minus, paths, &remove_symlinks_to, config_path, changes, n_changes); + if (r < 0) + return r; - 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; - } - } + r = remove_marked_symlinks(remove_symlinks_to, config_path, paths, false, changes, n_changes); + } else + r = 0; 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, - runtime ? paths->runtime_config : paths->persistent_config, - force, SEARCH_LOAD, changes, n_changes); - if (r == 0) - r = q; + 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; + } } return r; @@ -3011,6 +3012,7 @@ 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; @@ -3022,6 +3024,10 @@ 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; @@ -3032,7 +3038,7 @@ int unit_file_preset( return r; } - return execute_preset(scope, flags, &plus, &minus, &paths, files, mode, changes, n_changes); + return execute_preset(scope, &plus, &minus, &paths, config_path, files, mode, !!(flags & UNIT_FILE_FORCE), changes, n_changes); } int unit_file_preset_all( @@ -3046,6 +3052,7 @@ 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; @@ -3057,6 +3064,10 @@ 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; @@ -3074,6 +3085,7 @@ int unit_file_preset_all( } FOREACH_DIRENT(de, d, return -errno) { + if (!unit_name_is_valid(de->d_name, UNIT_NAME_ANY)) continue; @@ -3097,7 +3109,7 @@ int unit_file_preset_all( } } - return execute_preset(scope, flags, &plus, &minus, &paths, NULL, mode, changes, n_changes); + return execute_preset(scope, &plus, &minus, &paths, config_path, NULL, mode, !!(flags & UNIT_FILE_FORCE), 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 f072ad0c..fc0baa8c 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -7683,11 +7683,6 @@ 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; } -- 2.30.2