picnoir/Systemd
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

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

Browse source 
Having systemctl disable/unmask remove all symlinks in /etc and /run is
unintuitive and breaks existing use cases.
systemctl should behave symmetrically.
A "systemctl --runtime unmask" should undo a "systemctl --runtime mask"
action.
Say you have a service, which was masked by the admin in /etc.
If you temporarily want to mask the execution of the service (say in a
script), you'd create a runtime mask via "systemctl --runtime mask".
It is is now no longer possible to undo this temporary mask without
nuking the admin changes, unless you start rm'ing files manually.

While it is useful to be able to remove all enablement/mask symlinks in
one go, this should be done via a separate command line switch, like
"systemctl --all unmask".

This reverts commit 4910b35078.

Fixes: #9393
This commit is contained in:
Michael Biebl 2018-06-27 14:31:42 +02:00
parent 00e7b3c8e5
commit 1830ac51a4
3 changed files with 98 additions and 91 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

22
man/systemctl.xml
View file

@ -555,19 +555,19 @@
<term><option>--runtime</option></term> <term><option>--runtime</option></term>
<listitem> <listitem>
<para>When used with <command>set-property</command>, make changes only <para>When used with <command>enable</command>,
temporarily, so that they are lost on the next reboot.</para> <command>disable</command>, <command>edit</command>,
(and related commands), make changes only temporarily, so
<para>Similarily, when used with <command>enable</command>, <command>mask</command>, that they are lost on the next reboot. This will have the
<command>edit</command> and related commands, make temporary changes, which are lost on effect that changes are not made in subdirectories of
the next reboot. Changes are not made in subdirectories of <filename>/etc</filename>, but <filename>/etc</filename> but in <filename>/run</filename>,
in <filename>/run</filename>. The immediate effect is identical, however since the latter with identical immediate effects, however, since the latter
is lost on reboot, the changes are lost too.</para> is lost on reboot, the changes are lost too.</para>
<para>Note: this option cannot be used with <command>disable</command>, <para>Similarly, when used with
<command>unmask</command>, <command>preset</command>, or <command>preset-all</command>, <command>set-property</command>, make changes only
because those operations sometimes need to remove symlinks under <filename>/etc</filename> temporarily, so that they are lost on the next
to have the desired effect, which would cause a persistent change.</para> reboot.</para>
</listitem> </listitem>
</varlistentry> </varlistentry>

162
src/shared/install.c
View file

@ -1932,6 +1932,7 @@ static int install_context_mark_for_removal(
InstallContext *c, InstallContext *c,
const LookupPaths *paths, const LookupPaths *paths,
Set **remove_symlinks_to, Set **remove_symlinks_to,
const char *config_path,
UnitFileChange **changes, UnitFileChange **changes,
size_t *n_changes) { size_t *n_changes) {
@ -1940,6 +1941,7 @@ static int install_context_mark_for_removal(
assert(c); assert(c);
assert(paths); assert(paths);
assert(config_path);
/* Marks all items for removal */ /* Marks all items for removal */
@ -2049,7 +2051,7 @@ int unit_file_unmask(
size_t n_todo = 0, n_allocated = 0; size_t n_todo = 0, n_allocated = 0;
const char *config_path; const char *config_path;
char **i; char **i;
bool dry_run = !!(flags & UNIT_FILE_DRY_RUN); bool dry_run;
int r, q; int r, q;
assert(scope >= 0); assert(scope >= 0);
@ -2059,71 +2061,73 @@ int unit_file_unmask(
if (r < 0) if (r < 0)
return r; 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) { STRV_FOREACH(i, files) {
_cleanup_free_ char *path = NULL;
if (!unit_name_is_valid(*i, UNIT_NAME_ANY)) if (!unit_name_is_valid(*i, UNIT_NAME_ANY))
return -EINVAL; return -EINVAL;
FOREACH_STRING(config_path, paths.runtime_config, paths.persistent_config) { path = path_make_absolute(*i, config_path);
_cleanup_free_ char *path = NULL; if (!path)
return -ENOMEM;
path = path_make_absolute(*i, config_path); r = null_or_empty_path(path);
if (!path) if (r == -ENOENT)
return -ENOMEM; continue;
if (r < 0)
return r;
if (r == 0)
continue;
r = null_or_empty_path(path); if (!GREEDY_REALLOC0(todo, n_allocated, n_todo + 2))
if (r == -ENOENT) return -ENOMEM;
continue;
if (r < 0)
return r;
if (r == 0)
continue;
if (!GREEDY_REALLOC0(todo, n_allocated, n_todo + 2)) todo[n_todo] = strdup(*i);
return -ENOMEM; if (!todo[n_todo])
return -ENOMEM;
todo[n_todo] = strdup(*i); n_todo++;
if (!todo[n_todo])
return -ENOMEM;
n_todo++;
}
} }
strv_uniq(todo); strv_uniq(todo);
r = 0; r = 0;
FOREACH_STRING(config_path, paths.runtime_config, paths.persistent_config) { STRV_FOREACH(i, todo) {
STRV_FOREACH(i, todo) { _cleanup_free_ char *path = NULL;
_cleanup_free_ char *path = NULL; const char *rp;
const char *rp;
path = path_make_absolute(*i, config_path); path = path_make_absolute(*i, config_path);
if (!path) if (!path)
return -ENOMEM; return -ENOMEM;
if (!dry_run && unlink(path) < 0) { if (!dry_run && unlink(path) < 0) {
if (errno != ENOENT) { if (errno != ENOENT) {
if (r >= 0) if (r >= 0)
r = -errno; r = -errno;
unit_file_changes_add(changes, n_changes, -errno, path, NULL); unit_file_changes_add(changes, n_changes, -errno, path, NULL);
}
continue;
} }
unit_file_changes_add(changes, n_changes, UNIT_FILE_UNLINK, path, NULL); continue;
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); unit_file_changes_add(changes, n_changes, UNIT_FILE_UNLINK, path, NULL);
if (r >= 0)
r = 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;
return r; return r;
} }
@ -2506,7 +2510,6 @@ int unit_file_disable(
_cleanup_(lookup_paths_free) LookupPaths paths = {}; _cleanup_(lookup_paths_free) LookupPaths paths = {};
_cleanup_(install_context_done) InstallContext c = {}; _cleanup_(install_context_done) InstallContext c = {};
_cleanup_set_free_free_ Set *remove_symlinks_to = NULL; _cleanup_set_free_free_ Set *remove_symlinks_to = NULL;
bool dry_run = !!(flags & UNIT_FILE_DRY_RUN);
const char *config_path; const char *config_path;
char **i; char **i;
int r; int r;
@ -2518,6 +2521,10 @@ int unit_file_disable(
if (r < 0) if (r < 0)
return r; return r;
config_path = (flags & UNIT_FILE_RUNTIME) ? paths.runtime_config : paths.persistent_config;
if (!config_path)
return -ENXIO;
STRV_FOREACH(i, files) { STRV_FOREACH(i, files) {
if (!unit_name_is_valid(*i, UNIT_NAME_ANY)) if (!unit_name_is_valid(*i, UNIT_NAME_ANY))
return -EINVAL; return -EINVAL;
@ -2527,17 +2534,11 @@ int unit_file_disable(
return r; 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) if (r < 0)
return r; return r;
FOREACH_STRING(config_path, paths.runtime_config, paths.persistent_config) { return remove_marked_symlinks(remove_symlinks_to, config_path, &paths, !!(flags & UNIT_FILE_DRY_RUN), changes, n_changes);
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( int unit_file_reenable(
@ -3022,45 +3023,45 @@ int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char
static int execute_preset( static int execute_preset(
UnitFileScope scope, UnitFileScope scope,
UnitFileFlags flags,
InstallContext *plus, InstallContext *plus,
InstallContext *minus, InstallContext *minus,
const LookupPaths *paths, const LookupPaths *paths,
const char *config_path,
char **files, char **files,
UnitFilePresetMode mode, UnitFilePresetMode mode,
bool force,
UnitFileChange **changes, UnitFileChange **changes,
size_t *n_changes) { size_t *n_changes) {
const char *config_path; int r;
bool force = !!(flags & UNIT_FILE_FORCE);
bool runtime = !!(flags & UNIT_FILE_RUNTIME);
int r = 0, q;
assert(plus); assert(plus);
assert(minus); assert(minus);
assert(paths); assert(paths);
assert(config_path);
if (mode != UNIT_FILE_PRESET_ENABLE_ONLY) { if (mode != UNIT_FILE_PRESET_ENABLE_ONLY) {
_cleanup_set_free_free_ Set *remove_symlinks_to = NULL; _cleanup_set_free_free_ Set *remove_symlinks_to = NULL;
q = install_context_mark_for_removal(scope, minus, paths, &remove_symlinks_to, changes, n_changes); r = install_context_mark_for_removal(scope, minus, paths, &remove_symlinks_to, config_path, changes, n_changes);
if (q < 0) if (r < 0)
return q; return r;
FOREACH_STRING(config_path, paths->runtime_config, paths->persistent_config) { r = remove_marked_symlinks(remove_symlinks_to, config_path, paths, false, changes, n_changes);
q = remove_marked_symlinks(remove_symlinks_to, config_path, paths, false, changes, n_changes); } else
if (r == 0) r = 0;
r = q;
}
}
if (mode != UNIT_FILE_PRESET_DISABLE_ONLY) { if (mode != UNIT_FILE_PRESET_DISABLE_ONLY) {
int q;
/* Returns number of symlinks that where supposed to be installed. */ /* Returns number of symlinks that where supposed to be installed. */
q = install_context_apply(scope, plus, paths, q = install_context_apply(scope, plus, paths, config_path, force, SEARCH_LOAD, changes, n_changes);
runtime ? paths->runtime_config : paths->persistent_config, if (r >= 0) {
force, SEARCH_LOAD, changes, n_changes); if (q < 0)
if (r == 0) r = q;
r = q; else
r += q;
}
} }
return r; return r;
@ -3132,6 +3133,7 @@ int unit_file_preset(
_cleanup_(install_context_done) InstallContext plus = {}, minus = {}; _cleanup_(install_context_done) InstallContext plus = {}, minus = {};
_cleanup_(lookup_paths_free) LookupPaths paths = {}; _cleanup_(lookup_paths_free) LookupPaths paths = {};
_cleanup_(presets_freep) Presets presets = {}; _cleanup_(presets_freep) Presets presets = {};
const char *config_path;
char **i; char **i;
int r; int r;
@ -3143,6 +3145,10 @@ int unit_file_preset(
if (r < 0) if (r < 0)
return r; 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); r = read_presets(scope, root_dir, &presets);
if (r < 0) if (r < 0)
return r; return r;
@ -3153,7 +3159,7 @@ int unit_file_preset(
return r; 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( int unit_file_preset_all(
@ -3167,6 +3173,7 @@ int unit_file_preset_all(
_cleanup_(install_context_done) InstallContext plus = {}, minus = {}; _cleanup_(install_context_done) InstallContext plus = {}, minus = {};
_cleanup_(lookup_paths_free) LookupPaths paths = {}; _cleanup_(lookup_paths_free) LookupPaths paths = {};
_cleanup_(presets_freep) Presets presets = {}; _cleanup_(presets_freep) Presets presets = {};
const char *config_path = NULL;
char **i; char **i;
int r; int r;
@ -3178,6 +3185,10 @@ int unit_file_preset_all(
if (r < 0) if (r < 0)
return r; 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); r = read_presets(scope, root_dir, &presets);
if (r < 0) if (r < 0)
return r; return r;
@ -3195,6 +3206,7 @@ int unit_file_preset_all(
} }
FOREACH_DIRENT(de, d, return -errno) { FOREACH_DIRENT(de, d, return -errno) {
if (!unit_name_is_valid(de->d_name, UNIT_NAME_ANY)) if (!unit_name_is_valid(de->d_name, UNIT_NAME_ANY))
continue; continue;
@ -3218,7 +3230,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) { static void unit_file_list_free_one(UnitFileList *f) {

5
src/systemctl/systemctl.c
View file

@ -7831,11 +7831,6 @@ static int systemctl_parse_argv(int argc, char *argv[]) {
return log_error_errno(SYNTHETIC_ERRNO(EINVAL), return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
"--wait may not be combined with --no-block."); "--wait may not be combined with --no-block.");
if (arg_runtime && STRPTR_IN_SET(argv[optind], "disable", "unmask", "preset", "preset-all"))
return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
"--runtime cannot be used with %s",
argv[optind]);
return 1; return 1;
} }