From: Zbigniew Jędrzejewski-Szmek Date: Thu, 21 Sep 2017 16:53:45 +0000 (+0200) Subject: install: only consider names in Alias= as "enabling" X-Git-Tag: v235~71^2~2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=5cd8ae31527384dcdbd1e9d2c7e7cd3a803222f8;p=thirdparty%2Fsystemd.git install: only consider names in Alias= as "enabling" When a unit has a symlink that makes an alias in the filesystem, but that name is not specified in [Install], it is confusing is the unit is shown as "enabled". Look only for names specified in Alias=. Fixes #6338. v2: - Fix indentation. - Fix checking for normal enablement, when the symlink name is the same as the unit name. This case wasn't handled properly in v1. v3: - Rework the patch to also handle templates properly: A template templ@.service with DefaultInstance=foo will be considered enabled only when templ@foo.service symlink is found. Symlinks with other instance names do not count, which matches the logic for aliases to normal units. Tests are updated. --- diff --git a/man/systemctl.xml b/man/systemctl.xml index 0778c38ca4b..cf9d85c98c3 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -1245,7 +1245,7 @@ Jan 12 10:46:45 example.com bluetoothd[8900]: gatt-time-server: Input/output err enabled - Enabled via .wants/, .requires/ or alias symlinks (permanently in /etc/systemd/system/, or transiently in /run/systemd/system/). + Enabled via .wants/, .requires/ or Alias= symlinks (permanently in /etc/systemd/system/, or transiently in /run/systemd/system/). 0 diff --git a/src/shared/install.c b/src/shared/install.c index d73727e403e..f6684452825 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -699,9 +699,34 @@ static int remove_marked_symlinks( return r; } +static bool is_symlink_with_known_name(const UnitFileInstallInfo *i, const char *name) { + int r; + + if (streq(name, i->name)) + return true; + + if (strv_contains(i->aliases, name)) + return true; + + /* Look for template symlink matching DefaultInstance */ + if (i->default_instance && unit_name_is_valid(i->name, UNIT_NAME_TEMPLATE)) { + _cleanup_free_ char *s = NULL; + + r = unit_name_replace_instance(i->name, i->default_instance, &s); + if (r < 0) { + if (r != -EINVAL) + return r; + + } else if (streq(name, s)) + return true; + } + + return false; +} + static int find_symlinks_fd( const char *root_dir, - const char *name, + UnitFileInstallInfo *i, int fd, const char *path, const char *config_path, @@ -711,7 +736,7 @@ static int find_symlinks_fd( struct dirent *de; int r = 0; - assert(name); + assert(i); assert(fd >= 0); assert(path); assert(config_path); @@ -748,7 +773,8 @@ static int find_symlinks_fd( } /* This will close nfd, regardless whether it succeeds or not */ - q = find_symlinks_fd(root_dir, name, nfd, p, config_path, same_name_link); + q = find_symlinks_fd(root_dir, i, nfd, + p, config_path, same_name_link); if (q > 0) return 1; if (r == 0) @@ -788,24 +814,24 @@ static int find_symlinks_fd( /* Check if the symlink itself matches what we * are looking for */ - if (path_is_absolute(name)) - found_path = path_equal(p, name); + if (path_is_absolute(i->name)) + found_path = path_equal(p, i->name); else - found_path = streq(de->d_name, name); + found_path = streq(de->d_name, i->name); /* Check if what the symlink points to * matches what we are looking for */ - if (path_is_absolute(name)) - found_dest = path_equal(dest, name); + if (path_is_absolute(i->name)) + found_dest = path_equal(dest, i->name); else - found_dest = streq(basename(dest), name); + found_dest = streq(basename(dest), i->name); if (found_path && found_dest) { _cleanup_free_ char *t = NULL; /* Filter out same name links in the main * config path */ - t = path_make_absolute(name, config_path); + t = path_make_absolute(i->name, config_path); if (!t) return -ENOMEM; @@ -814,8 +840,15 @@ static int find_symlinks_fd( if (b) *same_name_link = true; - else if (found_path || found_dest) - return 1; + else if (found_path || found_dest) { + /* Check if symlink name is in the set of names used by [Install] */ + q = is_symlink_with_known_name(i, de->d_name); + log_info("is_symlink_with_known_name(%s, %s) → %d", i->name, de->d_name, q); + if (q < 0) + return q; + if (q > 0) + return 1; + } } } @@ -824,13 +857,13 @@ static int find_symlinks_fd( static int find_symlinks( const char *root_dir, - const char *name, + UnitFileInstallInfo *i, const char *config_path, bool *same_name_link) { int fd; - assert(name); + assert(i); assert(config_path); assert(same_name_link); @@ -842,12 +875,13 @@ static int find_symlinks( } /* This takes possession of fd and closes it */ - return find_symlinks_fd(root_dir, name, fd, config_path, config_path, same_name_link); + return find_symlinks_fd(root_dir, i, fd, + config_path, config_path, same_name_link); } static int find_symlinks_in_scope( const LookupPaths *paths, - const char *name, + UnitFileInstallInfo *i, UnitFileState *state) { bool same_name_link_runtime = false, same_name_link_config = false; @@ -856,12 +890,12 @@ static int find_symlinks_in_scope( int r; assert(paths); - assert(name); + assert(i); STRV_FOREACH(p, paths->search_path) { bool same_name_link = false; - r = find_symlinks(paths->root_dir, name, *p, &same_name_link); + r = find_symlinks(paths->root_dir, i, *p, &same_name_link); if (r < 0) return r; if (r > 0) { @@ -910,7 +944,7 @@ static int find_symlinks_in_scope( * outside of runtime and configuration directory, then we consider it statically enabled. Note we do that only * for instance, not for regular names, as those are merely aliases, while instances explicitly instantiate * something, and hence are a much stronger concept. */ - if (enabled_at_all && unit_name_is_valid(name, UNIT_NAME_INSTANCE)) { + if (enabled_at_all && unit_name_is_valid(i->name, UNIT_NAME_INSTANCE)) { *state = UNIT_FILE_STATIC; return 1; } @@ -2603,7 +2637,10 @@ static int unit_file_lookup_state( break; } - r = find_symlinks_in_scope(paths, i->name, &state); + /* Check if any of the Alias= symlinks have been created. + * We ignore other aliases, and only check those that would + * be created by systemctl enable for this unit. */ + r = find_symlinks_in_scope(paths, i, &state); if (r < 0) return r; if (r == 0) { diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c index 575401cb91b..bb61d7fb197 100644 --- a/src/test/test-install-root.c +++ b/src/test/test-install-root.c @@ -381,6 +381,8 @@ static void test_template_enable(const char *root) { UnitFileState state; const char *p; + log_info("== %s ==", __func__); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@.service", &state) == -ENOENT); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@def.service", &state) == -ENOENT); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@foo.service", &state) == -ENOENT); @@ -402,6 +404,8 @@ static void test_template_enable(const char *root) { assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@def.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@foo.service", &state) >= 0 && state == UNIT_FILE_DISABLED); + log_info("== %s with template@.service enabled ==", __func__); + assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("template@.service"), &changes, &n_changes) >= 0); assert_se(n_changes == 1); assert_se(changes[0].type == UNIT_FILE_SYMLINK); @@ -432,6 +436,8 @@ static void test_template_enable(const char *root) { assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@def.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@foo.service", &state) >= 0 && state == UNIT_FILE_DISABLED); + log_info("== %s with template@foo.service enabled ==", __func__); + assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("template@foo.service"), &changes, &n_changes) >= 0); assert_se(changes[0].type == UNIT_FILE_SYMLINK); assert_se(streq(changes[0].source, "/usr/lib/systemd/system/template@.service")); @@ -440,7 +446,7 @@ static void test_template_enable(const char *root) { unit_file_changes_free(changes, n_changes); changes = NULL; n_changes = 0; - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@.service", &state) >= 0 && state == UNIT_FILE_ENABLED); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@def.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@foo.service", &state) >= 0 && state == UNIT_FILE_ENABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@foo.service", &state) >= 0 && state == UNIT_FILE_ENABLED); @@ -463,6 +469,8 @@ static void test_template_enable(const char *root) { assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@foo.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@quux.service", &state) >= 0 && state == UNIT_FILE_DISABLED); + log_info("== %s with template-symlink@quux.service enabled ==", __func__); + assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("template-symlink@quux.service"), &changes, &n_changes) >= 0); assert_se(changes[0].type == UNIT_FILE_SYMLINK); assert_se(streq(changes[0].source, "/usr/lib/systemd/system/template@.service")); @@ -471,11 +479,11 @@ static void test_template_enable(const char *root) { unit_file_changes_free(changes, n_changes); changes = NULL; n_changes = 0; - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@.service", &state) >= 0 && state == UNIT_FILE_ENABLED); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@def.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@foo.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@quux.service", &state) >= 0 && state == UNIT_FILE_ENABLED); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@.service", &state) >= 0 && state == UNIT_FILE_ENABLED); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@def.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@foo.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@quux.service", &state) >= 0 && state == UNIT_FILE_ENABLED);