From 76ec966f0e33685f8331603805c0349adceea836 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sat, 16 Apr 2016 18:41:34 -0400 Subject: tree-wide: use ERFKILL instead of ESHUTDOWN for "unit masked" If the error code ever leaks (we print the strerror error instead of providing our own), the message for ESHUTDOWN is "Cannot send after transport endpoint shutdown", which can be misleading. In particular it suggest that some mishandling of the dbus connection occured. Let's change that to ERFKILL which has the advantage that a) it sounds implausible as actual error, b) has the connotation of disabling something manually. --- src/core/dbus-manager.c | 2 +- src/core/transaction.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'src/core') diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 2392ed6c16..0b12aae2ef 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -1610,7 +1610,7 @@ fail: static int install_error(sd_bus_error *error, int c) { assert(c < 0); - if (c == -ESHUTDOWN) + if (c == -ERFKILL) return sd_bus_error_setf(error, BUS_ERROR_UNIT_MASKED, "Unit file is masked."); if (c == -EADDRNOTAVAIL) return sd_bus_error_setf(error, BUS_ERROR_UNIT_GENERATED, "Unit file is transient or generated."); diff --git a/src/core/transaction.c b/src/core/transaction.c index dad387749c..d5370b2a14 100644 --- a/src/core/transaction.c +++ b/src/core/transaction.c @@ -939,7 +939,7 @@ int transaction_add_job_and_dependencies( if (r < 0) { /* unit masked, job type not applicable and unit not found are not considered as errors. */ log_unit_full(dep, - IN_SET(r, -ESHUTDOWN, -EBADR, -ENOENT) ? LOG_DEBUG : LOG_WARNING, + IN_SET(r, -ERFKILL, -EBADR, -ENOENT) ? LOG_DEBUG : LOG_WARNING, r, "Cannot add dependency job, ignoring: %s", bus_error_message(e, r)); sd_bus_error_free(e); -- cgit v1.2.3-54-g00ecf From 9a0a413a195a21888cf926be5595d0efc1eef0fe Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sat, 16 Apr 2016 19:31:53 -0400 Subject: systemctl/core: ignore masked units in preset-all With any masked unit that would that would be enabled by presets, we'd get: test@rawhide $ sudo systemctl preset-all Failed to execute operation: Unit file is masked. test@rawhide $ sudo systemctl --root=/ preset-all Operation failed: Cannot send after transport endpoint shutdown Simply ignore those units: test@rawhide $ sudo systemctl preset-all Unit xxx.service is masked, ignoring. --- src/core/dbus-manager.c | 12 +++++++----- src/shared/bus-util.c | 10 ++++++++-- src/shared/install.c | 4 ++++ src/shared/install.h | 5 +++++ src/systemctl/systemctl.c | 16 ++++++++++++---- 5 files changed, 36 insertions(+), 11 deletions(-) (limited to 'src/core') diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 0b12aae2ef..9a913a6c91 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -1565,11 +1565,13 @@ static int reply_unit_file_changes_and_free( unsigned i; int r; - if (n_changes > 0) { - r = bus_foreach_bus(m, NULL, send_unit_files_changed, NULL); - if (r < 0) - log_debug_errno(r, "Failed to send UnitFilesChanged signal: %m"); - } + for (i = 0; i < n_changes; i++) + if (unit_file_change_is_modification(changes[i].type)) { + r = bus_foreach_bus(m, NULL, send_unit_files_changed, NULL); + if (r < 0) + log_debug_errno(r, "Failed to send UnitFilesChanged signal: %m"); + break; + } r = sd_bus_message_new_method_return(message, &reply); if (r < 0) diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index 2b86b1fcd6..677970b7f0 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -2203,11 +2203,17 @@ int bus_deserialize_and_dump_unit_file_changes(sd_bus_message *m, bool quiet, Un if (!quiet) { if (streq(type, "symlink")) log_info("Created symlink from %s to %s.", path, source); - else + else if (streq(type, "unlink")) log_info("Removed symlink %s.", path); + else if (streq(type, "masked")) + log_info("Unit %s is masked, ignoring.", path); + else + log_notice("Manager reported unknown change type \"%s\" for %s.", type, path); } - r = unit_file_changes_add(changes, n_changes, streq(type, "symlink") ? UNIT_FILE_SYMLINK : UNIT_FILE_UNLINK, path, source); + r = unit_file_changes_add(changes, n_changes, + unit_file_change_type_from_string(type), + path, source); if (r < 0) return r; } diff --git a/src/shared/install.c b/src/shared/install.c index 35d83dd16c..74de8141f1 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -2539,6 +2539,9 @@ int unit_file_preset_all( continue; r = preset_prepare_one(scope, &plus, &minus, &paths, mode, de->d_name); + if (r == -ERFKILL) + r = unit_file_changes_add(changes, n_changes, + UNIT_FILE_IS_MASKED, de->d_name, NULL); if (r < 0) return r; } @@ -2652,6 +2655,7 @@ DEFINE_STRING_TABLE_LOOKUP(unit_file_state, UnitFileState); static const char* const unit_file_change_type_table[_UNIT_FILE_CHANGE_TYPE_MAX] = { [UNIT_FILE_SYMLINK] = "symlink", [UNIT_FILE_UNLINK] = "unlink", + [UNIT_FILE_IS_MASKED] = "masked", }; DEFINE_STRING_TABLE_LOOKUP(unit_file_change_type, UnitFileChangeType); diff --git a/src/shared/install.h b/src/shared/install.h index 6f8b45d4f6..6b760def9b 100644 --- a/src/shared/install.h +++ b/src/shared/install.h @@ -72,10 +72,15 @@ enum UnitFilePresetMode { enum UnitFileChangeType { UNIT_FILE_SYMLINK, UNIT_FILE_UNLINK, + UNIT_FILE_IS_MASKED, _UNIT_FILE_CHANGE_TYPE_MAX, _UNIT_FILE_CHANGE_TYPE_INVALID = -1 }; +static inline bool unit_file_change_is_modification(UnitFileChangeType type) { + return IN_SET(type, UNIT_FILE_SYMLINK, UNIT_FILE_UNLINK); +} + struct UnitFileChange { UnitFileChangeType type; char *path; diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 4942dbb6ad..0c64cbab80 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -1989,12 +1989,20 @@ static void dump_unit_file_changes(const UnitFileChange *changes, unsigned n_cha assert(changes || n_changes == 0); - for (i = 0; i < n_changes; i++) { - if (changes[i].type == UNIT_FILE_SYMLINK) + for (i = 0; i < n_changes; i++) + switch(changes[i].type) { + case UNIT_FILE_SYMLINK: log_info("Created symlink %s, pointing to %s.", changes[i].path, changes[i].source); - else + break; + case UNIT_FILE_UNLINK: log_info("Removed %s.", changes[i].path); - } + break; + case UNIT_FILE_IS_MASKED: + log_info("Unit %s is masked, ignoring.", changes[i].path); + break; + default: + assert_not_reached("bad change type"); + } } static int set_default(int argc, char *argv[], void *userdata) { -- cgit v1.2.3-54-g00ecf From 3ae5990c6ed54f5d4acec12c2e9e40d110c72230 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sat, 16 Apr 2016 22:52:06 -0400 Subject: tree-wide: introduce PATH_IN_SET macro --- src/basic/path-util.c | 8 ++++---- src/basic/path-util.h | 13 +++++++++++++ src/core/mount.c | 6 ++---- src/test/test-path-util.c | 6 ++++++ 4 files changed, 25 insertions(+), 8 deletions(-) (limited to 'src/core') diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 822c09bfba..044a12889d 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -569,10 +569,10 @@ static int binary_is_good(const char *binary) { if (r < 0) return r; - return !path_equal(d, "true") && - !path_equal(d, "/bin/true") && - !path_equal(d, "/usr/bin/true") && - !path_equal(d, "/dev/null"); + return !PATH_IN_SET(d, "true" + "/bin/true", + "/usr/bin/true", + "/dev/null"); } int fsck_exists(const char *fstype) { diff --git a/src/basic/path-util.h b/src/basic/path-util.h index 2c2f87a9f2..f43d477eb9 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -48,6 +48,19 @@ bool path_equal(const char *a, const char *b) _pure_; bool path_equal_or_files_same(const char *a, const char *b); char* path_join(const char *root, const char *path, const char *rest); +/* Note: the search terminates on the first NULL item. */ +#define PATH_IN_SET(p, ...) \ + ({ \ + char **s; \ + bool _found = false; \ + STRV_FOREACH(s, STRV_MAKE(__VA_ARGS__)) \ + if (path_equal(p, *s)) { \ + _found = true; \ + break; \ + } \ + _found; \ + }) + int path_strv_make_absolute_cwd(char **l); char** path_strv_resolve(char **l, const char *prefix); char** path_strv_resolve_uniq(char **l, const char *prefix); diff --git a/src/core/mount.c b/src/core/mount.c index 74ab54bfd0..632c5c824c 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -376,8 +376,7 @@ static int mount_add_quota_links(Mount *m) { static bool should_umount(Mount *m) { MountParameters *p; - if (path_equal(m->where, "/") || - path_equal(m->where, "/usr") || + if (PATH_IN_SET(m->where, "/", "/usr") || path_startswith(m->where, "/run/initramfs")) return false; @@ -408,8 +407,7 @@ static int mount_add_default_dependencies(Mount *m) { * Also, don't bother with anything mounted below virtual * file systems, it's also going to be virtual, and hence * not worth the effort. */ - if (path_equal(m->where, "/") || - path_equal(m->where, "/usr") || + if (PATH_IN_SET(m->where, "/", "/usr") || path_startswith(m->where, "/run/initramfs") || path_startswith(m->where, "/proc") || path_startswith(m->where, "/sys") || diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index d376dd56c5..7ce2695c5c 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -90,6 +90,12 @@ static void test_path(void) { assert_se(path_equal(path_kill_slashes(p2), "/aaa/./ccc")); assert_se(path_equal(path_kill_slashes(p3), "/./")); } + + assert_se(PATH_IN_SET("/bin", "/", "/bin", "/foo")); + assert_se(PATH_IN_SET("/bin", "/bin")); + assert_se(PATH_IN_SET("/bin", "/foo/bar", "/bin")); + assert_se(PATH_IN_SET("/", "/", "/", "/foo/bar")); + assert_se(!PATH_IN_SET("/", "/abc", "/def")); } static void test_find_binary(const char *self) { -- cgit v1.2.3-54-g00ecf