From e1714f0250a9128a89f5610bd53064cc7c7ec960 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 28 Jul 2019 10:13:21 +0200 Subject: [PATCH 01/12] shared/exit-status: turn status level into a bitmask, add "test" The "test" doesn't really test much automatically, but it is still useful to look at the mappings. --- src/core/execute.c | 10 +- src/core/main.c | 19 ++- src/shared/exit-status.c | 303 +++++++++++------------------------- src/shared/exit-status.h | 31 ++-- src/systemctl/systemctl.c | 5 +- src/test/meson.build | 4 + src/test/test-exit-status.c | 26 ++++ 7 files changed, 160 insertions(+), 238 deletions(-) create mode 100644 src/test/test-exit-status.c diff --git a/src/core/execute.c b/src/core/execute.c index 911c369042..5b55557f4e 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -3878,15 +3878,19 @@ int exec_spawn(Unit *unit, unit->manager->user_lookup_fds[1], &exit_status); - if (r < 0) + if (r < 0) { + const char *status = + exit_status_to_string(exit_status, + EXIT_STATUS_GLIBC | EXIT_STATUS_SYSTEMD); + log_struct_errno(LOG_ERR, r, "MESSAGE_ID=" SD_MESSAGE_SPAWN_FAILED_STR, LOG_UNIT_ID(unit), LOG_UNIT_INVOCATION_ID(unit), LOG_UNIT_MESSAGE(unit, "Failed at step %s spawning %s: %m", - exit_status_to_string(exit_status, EXIT_STATUS_SYSTEMD), - command->path), + status, command->path), "EXECUTABLE=%s", command->path); + } _exit(exit_status); } diff --git a/src/core/main.c b/src/core/main.c index 0674e00ab0..0698f893fd 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -221,16 +221,19 @@ _noreturn_ static void crash(int sig) { r = wait_for_terminate(pid, &status); if (r < 0) log_emergency_errno(r, "Caught <%s>, waitpid() failed: %m", signal_to_string(sig)); - else if (status.si_code != CLD_DUMPED) + else if (status.si_code != CLD_DUMPED) { + const char *s = status.si_code == CLD_EXITED + ? exit_status_to_string(status.si_status, EXIT_STATUS_GLIBC) + : signal_to_string(status.si_status); + log_emergency("Caught <%s>, core dump failed (child "PID_FMT", code=%s, status=%i/%s).", signal_to_string(sig), - pid, sigchld_code_to_string(status.si_code), - status.si_status, - strna(status.si_code == CLD_EXITED - ? exit_status_to_string(status.si_status, EXIT_STATUS_MINIMAL) - : signal_to_string(status.si_status))); - else - log_emergency("Caught <%s>, dumped core as pid "PID_FMT".", signal_to_string(sig), pid); + pid, + sigchld_code_to_string(status.si_code), + status.si_status, strna(s)); + } else + log_emergency("Caught <%s>, dumped core as pid "PID_FMT".", + signal_to_string(sig), pid); } } diff --git a/src/shared/exit-status.c b/src/shared/exit-status.c index 58ebc3ca4d..51f2380027 100644 --- a/src/shared/exit-status.c +++ b/src/shared/exit-status.c @@ -8,8 +8,7 @@ #include "macro.h" #include "set.h" -const char* exit_status_to_string(int status, ExitStatusLevel level) { - +const ExitStatusMapping exit_status_mappings[256] = { /* Exit status ranges: * * 0…1 │ ISO C, EXIT_SUCCESS + EXIT_FAILURE @@ -25,224 +24,100 @@ const char* exit_status_to_string(int status, ExitStatusLevel level) { * │ signal or such, and we follow that logic here.) */ - switch (status) { /* We always cover the ISO C ones */ + [EXIT_SUCCESS] = { "SUCCESS", EXIT_STATUS_GLIBC }, + [EXIT_FAILURE] = { "FAILURE", EXIT_STATUS_GLIBC }, - case EXIT_SUCCESS: - return "SUCCESS"; + [EXIT_CHDIR] = { "CHDIR", EXIT_STATUS_SYSTEMD }, + [EXIT_NICE] = { "NICE", EXIT_STATUS_SYSTEMD }, + [EXIT_FDS] = { "FDS", EXIT_STATUS_SYSTEMD }, + [EXIT_EXEC] = { "EXEC", EXIT_STATUS_SYSTEMD }, + [EXIT_MEMORY] = { "MEMORY", EXIT_STATUS_SYSTEMD }, + [EXIT_LIMITS] = { "LIMITS", EXIT_STATUS_SYSTEMD }, + [EXIT_OOM_ADJUST] = { "OOM_ADJUST", EXIT_STATUS_SYSTEMD }, + [EXIT_SIGNAL_MASK] = { "SIGNAL_MASK", EXIT_STATUS_SYSTEMD }, + [EXIT_STDIN] = { "STDIN", EXIT_STATUS_SYSTEMD }, + [EXIT_STDOUT] = { "STDOUT", EXIT_STATUS_SYSTEMD }, + [EXIT_CHROOT] = { "CHROOT", EXIT_STATUS_SYSTEMD }, + [EXIT_IOPRIO] = { "IOPRIO", EXIT_STATUS_SYSTEMD }, + [EXIT_TIMERSLACK] = { "TIMERSLACK", EXIT_STATUS_SYSTEMD }, + [EXIT_SECUREBITS] = { "SECUREBITS", EXIT_STATUS_SYSTEMD }, + [EXIT_SETSCHEDULER] = { "SETSCHEDULER", EXIT_STATUS_SYSTEMD }, + [EXIT_CPUAFFINITY] = { "CPUAFFINITY", EXIT_STATUS_SYSTEMD }, + [EXIT_GROUP] = { "GROUP", EXIT_STATUS_SYSTEMD }, + [EXIT_USER] = { "USER", EXIT_STATUS_SYSTEMD }, + [EXIT_CAPABILITIES] = { "CAPABILITIES", EXIT_STATUS_SYSTEMD }, + [EXIT_CGROUP] = { "CGROUP", EXIT_STATUS_SYSTEMD }, + [EXIT_SETSID] = { "SETSID", EXIT_STATUS_SYSTEMD }, + [EXIT_CONFIRM] = { "CONFIRM", EXIT_STATUS_SYSTEMD }, + [EXIT_STDERR] = { "STDERR", EXIT_STATUS_SYSTEMD }, + [EXIT_PAM] = { "PAM", EXIT_STATUS_SYSTEMD }, + [EXIT_NETWORK] = { "NETWORK", EXIT_STATUS_SYSTEMD }, + [EXIT_NAMESPACE] = { "NAMESPACE", EXIT_STATUS_SYSTEMD }, + [EXIT_NO_NEW_PRIVILEGES] = { "NO_NEW_PRIVILEGES", EXIT_STATUS_SYSTEMD }, + [EXIT_SECCOMP] = { "SECCOMP", EXIT_STATUS_SYSTEMD }, + [EXIT_SELINUX_CONTEXT] = { "SELINUX_CONTEXT", EXIT_STATUS_SYSTEMD }, + [EXIT_PERSONALITY] = { "PERSONALITY", EXIT_STATUS_SYSTEMD }, + [EXIT_APPARMOR_PROFILE] = { "APPARMOR", EXIT_STATUS_SYSTEMD }, + [EXIT_ADDRESS_FAMILIES] = { "ADDRESS_FAMILIES", EXIT_STATUS_SYSTEMD }, + [EXIT_RUNTIME_DIRECTORY] = { "RUNTIME_DIRECTORY", EXIT_STATUS_SYSTEMD }, + [EXIT_CHOWN] = { "CHOWN", EXIT_STATUS_SYSTEMD }, + [EXIT_SMACK_PROCESS_LABEL] = { "SMACK_PROCESS_LABEL", EXIT_STATUS_SYSTEMD }, + [EXIT_KEYRING] = { "KEYRING", EXIT_STATUS_SYSTEMD }, + [EXIT_STATE_DIRECTORY] = { "STATE_DIRECTORY", EXIT_STATUS_SYSTEMD }, + [EXIT_CACHE_DIRECTORY] = { "CACHE_DIRECTORY", EXIT_STATUS_SYSTEMD }, + [EXIT_LOGS_DIRECTORY] = { "LOGS_DIRECTORY", EXIT_STATUS_SYSTEMD }, + [EXIT_CONFIGURATION_DIRECTORY] = { "CONFIGURATION_DIRECTORY", EXIT_STATUS_SYSTEMD }, + [EXIT_NUMA_POLICY] = { "NUMA_POLICY", EXIT_STATUS_SYSTEMD }, + [EXIT_EXCEPTION] = { "EXCEPTION", EXIT_STATUS_SYSTEMD }, - case EXIT_FAILURE: - return "FAILURE"; + [EXIT_INVALIDARGUMENT] = { "INVALIDARGUMENT", EXIT_STATUS_LSB }, + [EXIT_NOTIMPLEMENTED] = { "NOTIMPLEMENTED", EXIT_STATUS_LSB }, + [EXIT_NOPERMISSION] = { "NOPERMISSION", EXIT_STATUS_LSB }, + [EXIT_NOTINSTALLED] = { "NOTINSTALLED", EXIT_STATUS_LSB }, + [EXIT_NOTCONFIGURED] = { "NOTCONFIGURED", EXIT_STATUS_LSB }, + [EXIT_NOTRUNNING] = { "NOTRUNNING", EXIT_STATUS_LSB }, + + [EX_USAGE] = { "USAGE", EXIT_STATUS_BSD }, + [EX_DATAERR] = { "DATAERR", EXIT_STATUS_BSD }, + [EX_NOINPUT] = { "NOINPUT", EXIT_STATUS_BSD }, + [EX_NOUSER] = { "NOUSER", EXIT_STATUS_BSD }, + [EX_NOHOST] = { "NOHOST", EXIT_STATUS_BSD }, + [EX_UNAVAILABLE] = { "UNAVAILABLE", EXIT_STATUS_BSD }, + [EX_SOFTWARE] = { "SOFTWARE", EXIT_STATUS_BSD }, + [EX_OSERR] = { "OSERR", EXIT_STATUS_BSD }, + [EX_OSFILE] = { "OSFILE", EXIT_STATUS_BSD }, + [EX_CANTCREAT] = { "CANTCREAT", EXIT_STATUS_BSD }, + [EX_IOERR] = { "IOERR", EXIT_STATUS_BSD }, + [EX_TEMPFAIL] = { "TEMPFAIL", EXIT_STATUS_BSD }, + [EX_PROTOCOL] = { "PROTOCOL", EXIT_STATUS_BSD }, + [EX_NOPERM] = { "NOPERM", EXIT_STATUS_BSD }, + [EX_CONFIG] = { "CONFIG", EXIT_STATUS_BSD }, +}; + +const char* exit_status_to_string(int code, ExitStatusClass class) { + if (code < 0 || (size_t) code >= ELEMENTSOF(exit_status_mappings)) + return NULL; + return FLAGS_SET(exit_status_mappings[code].class, class) ? exit_status_mappings[code].name : NULL; +} + +const char* exit_status_class(int code) { + if (code < 0 || (size_t) code >= ELEMENTSOF(exit_status_mappings)) + return NULL; + + switch (exit_status_mappings[code].class) { + case EXIT_STATUS_GLIBC: + return "glibc"; + case EXIT_STATUS_SYSTEMD: + return "systemd"; + case EXIT_STATUS_LSB: + return "LSB"; + case EXIT_STATUS_BSD: + return "BSD"; + default: return NULL; } - - if (IN_SET(level, EXIT_STATUS_SYSTEMD, EXIT_STATUS_LSB, EXIT_STATUS_FULL)) { - switch (status) { /* Optionally we cover our own ones */ - - case EXIT_CHDIR: - return "CHDIR"; - - case EXIT_NICE: - return "NICE"; - - case EXIT_FDS: - return "FDS"; - - case EXIT_EXEC: - return "EXEC"; - - case EXIT_MEMORY: - return "MEMORY"; - - case EXIT_LIMITS: - return "LIMITS"; - - case EXIT_OOM_ADJUST: - return "OOM_ADJUST"; - - case EXIT_SIGNAL_MASK: - return "SIGNAL_MASK"; - - case EXIT_STDIN: - return "STDIN"; - - case EXIT_STDOUT: - return "STDOUT"; - - case EXIT_CHROOT: - return "CHROOT"; - - case EXIT_IOPRIO: - return "IOPRIO"; - - case EXIT_TIMERSLACK: - return "TIMERSLACK"; - - case EXIT_SECUREBITS: - return "SECUREBITS"; - - case EXIT_SETSCHEDULER: - return "SETSCHEDULER"; - - case EXIT_CPUAFFINITY: - return "CPUAFFINITY"; - - case EXIT_GROUP: - return "GROUP"; - - case EXIT_USER: - return "USER"; - - case EXIT_CAPABILITIES: - return "CAPABILITIES"; - - case EXIT_CGROUP: - return "CGROUP"; - - case EXIT_SETSID: - return "SETSID"; - - case EXIT_CONFIRM: - return "CONFIRM"; - - case EXIT_STDERR: - return "STDERR"; - - case EXIT_PAM: - return "PAM"; - - case EXIT_NETWORK: - return "NETWORK"; - - case EXIT_NAMESPACE: - return "NAMESPACE"; - - case EXIT_NO_NEW_PRIVILEGES: - return "NO_NEW_PRIVILEGES"; - - case EXIT_SECCOMP: - return "SECCOMP"; - - case EXIT_SELINUX_CONTEXT: - return "SELINUX_CONTEXT"; - - case EXIT_PERSONALITY: - return "PERSONALITY"; - - case EXIT_APPARMOR_PROFILE: - return "APPARMOR"; - - case EXIT_ADDRESS_FAMILIES: - return "ADDRESS_FAMILIES"; - - case EXIT_RUNTIME_DIRECTORY: - return "RUNTIME_DIRECTORY"; - - case EXIT_CHOWN: - return "CHOWN"; - - case EXIT_SMACK_PROCESS_LABEL: - return "SMACK_PROCESS_LABEL"; - - case EXIT_KEYRING: - return "KEYRING"; - - case EXIT_STATE_DIRECTORY: - return "STATE_DIRECTORY"; - - case EXIT_CACHE_DIRECTORY: - return "CACHE_DIRECTORY"; - - case EXIT_LOGS_DIRECTORY: - return "LOGS_DIRECTORY"; - - case EXIT_CONFIGURATION_DIRECTORY: - return "CONFIGURATION_DIRECTORY"; - - case EXIT_NUMA_POLICY: - return "NUMA_POLICY"; - - case EXIT_EXCEPTION: - return "EXCEPTION"; - } - } - - if (IN_SET(level, EXIT_STATUS_LSB, EXIT_STATUS_FULL)) { - switch (status) { /* Optionally we support LSB ones */ - - case EXIT_INVALIDARGUMENT: - return "INVALIDARGUMENT"; - - case EXIT_NOTIMPLEMENTED: - return "NOTIMPLEMENTED"; - - case EXIT_NOPERMISSION: - return "NOPERMISSION"; - - case EXIT_NOTINSTALLED: - return "NOTINSTALLED"; - - case EXIT_NOTCONFIGURED: - return "NOTCONFIGURED"; - - case EXIT_NOTRUNNING: - return "NOTRUNNING"; - } - } - - if (level == EXIT_STATUS_FULL) { - switch (status) { /* Optionally, we support BSD exit statusses */ - - case EX_USAGE: - return "USAGE"; - - case EX_DATAERR: - return "DATAERR"; - - case EX_NOINPUT: - return "NOINPUT"; - - case EX_NOUSER: - return "NOUSER"; - - case EX_NOHOST: - return "NOHOST"; - - case EX_UNAVAILABLE: - return "UNAVAILABLE"; - - case EX_SOFTWARE: - return "SOFTWARE"; - - case EX_OSERR: - return "OSERR"; - - case EX_OSFILE: - return "OSFILE"; - - case EX_CANTCREAT: - return "CANTCREAT"; - - case EX_IOERR: - return "IOERR"; - - case EX_TEMPFAIL: - return "TEMPFAIL"; - - case EX_PROTOCOL: - return "PROTOCOL"; - - case EX_NOPERM: - return "NOPERM"; - - case EX_CONFIG: - return "CONFIG"; - } - } - - return NULL; } bool is_clean_exit(int code, int status, ExitClean clean, ExitStatusSet *success_status) { - if (code == CLD_EXITED) return status == 0 || (success_status && diff --git a/src/shared/exit-status.h b/src/shared/exit-status.h index 5637e6aa04..425f841523 100644 --- a/src/shared/exit-status.h +++ b/src/shared/exit-status.h @@ -7,8 +7,8 @@ #include "macro.h" #include "set.h" -/* This defines pretty names for the LSB 'start' verb exit codes. Note that they shouldn't be confused with the LSB - * 'status' verb exit codes which are defined very differently. For details see: +/* This defines pretty names for the LSB 'start' verb exit codes. Note that they shouldn't be confused with + * the LSB 'status' verb exit codes which are defined very differently. For details see: * * https://refspecs.linuxbase.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/iniscrptact.html */ @@ -25,8 +25,8 @@ enum { /* BSD's sysexits.h defines a couple EX_xyz exit codes in the range 64 … 78 */ - /* The LSB suggests that error codes >= 200 are "reserved". We use them here under the assumption that they - * hence are unused by init scripts. */ + /* The LSB suggests that error codes >= 200 are "reserved". We use them here under the assumption + * that they hence are unused by init scripts. */ EXIT_CHDIR = 200, EXIT_NICE, EXIT_FDS, @@ -74,19 +74,28 @@ enum { EXIT_EXCEPTION = 255, /* Whenever we want to propagate an abnormal/signal exit, in line with bash */ }; -typedef enum ExitStatusLevel { - EXIT_STATUS_MINIMAL, /* only cover libc EXIT_STATUS/EXIT_FAILURE */ - EXIT_STATUS_SYSTEMD, /* cover libc and systemd's own exit codes */ - EXIT_STATUS_LSB, /* cover libc, systemd's own and LSB exit codes */ - EXIT_STATUS_FULL, /* cover libc, systemd's own, LSB and BSD (EX_xyz) exit codes */ -} ExitStatusLevel; +typedef enum ExitStatusClass { + EXIT_STATUS_GLIBC = 1 << 0, /* libc EXIT_STATUS/EXIT_FAILURE */ + EXIT_STATUS_SYSTEMD = 1 << 1, /* systemd's own exit codes */ + EXIT_STATUS_LSB = 1 << 2, /* LSB exit codes */ + EXIT_STATUS_BSD = 1 << 3, /* BSD (EX_xyz) exit codes */ + EXIT_STATUS_FULL = EXIT_STATUS_GLIBC | EXIT_STATUS_SYSTEMD | EXIT_STATUS_LSB | EXIT_STATUS_BSD, +} ExitStatusClass; typedef struct ExitStatusSet { Set *status; Set *signal; } ExitStatusSet; -const char* exit_status_to_string(int status, ExitStatusLevel level) _const_; +const char* exit_status_to_string(int code, ExitStatusClass class) _const_; +const char* exit_status_class(int code) _const_; + +typedef struct ExitStatusMapping { + const char *name; + ExitStatusClass class; +} ExitStatusMapping; + +extern const ExitStatusMapping exit_status_mappings[256]; typedef enum ExitClean { EXIT_CLEAN_DAEMON, diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 95a8524594..b9a7d488bd 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -4380,7 +4380,7 @@ static void print_status_info( printf("status=%i", p->status); - c = exit_status_to_string(p->status, EXIT_STATUS_SYSTEMD); + c = exit_status_to_string(p->status, EXIT_STATUS_GLIBC | EXIT_STATUS_SYSTEMD); if (c) printf("/%s", c); @@ -4421,7 +4421,8 @@ static void print_status_info( printf("status=%i", i->exit_status); - c = exit_status_to_string(i->exit_status, EXIT_STATUS_SYSTEMD); + c = exit_status_to_string(i->exit_status, + EXIT_STATUS_GLIBC | EXIT_STATUS_SYSTEMD); if (c) printf("/%s", c); diff --git a/src/test/meson.build b/src/test/meson.build index 0595cfe37a..5625e682cf 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -305,6 +305,10 @@ tests += [ [], []], + [['src/test/test-exit-status.c'], + [], + []], + [['src/test/test-specifier.c'], [], []], diff --git a/src/test/test-exit-status.c b/src/test/test-exit-status.c new file mode 100644 index 0000000000..bab0596580 --- /dev/null +++ b/src/test/test-exit-status.c @@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ + +#include "exit-status.h" +#include "tests.h" + +static void test_exit_status_to_string(void) { + log_info("/* %s */", __func__); + + for (int i = -1; i <= 256; i++) { + const char *s, *class; + + s = exit_status_to_string(i, EXIT_STATUS_FULL); + class = exit_status_class(i); + log_info("%d: %s%s%s%s", + i, s ?: "-", + class ? " (" : "", class ?: "", class ? ")" : ""); + } +} + +int main(int argc, char *argv[]) { + test_setup_logging(LOG_DEBUG); + + test_exit_status_to_string(); + + return 0; +} From f0d67dcddd4bcbe0a221a4ff4248114e5cf57dd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 28 Jul 2019 10:19:53 +0200 Subject: [PATCH 02/12] shared/exit-status: add exit_status_from_string() --- src/shared/exit-status.c | 17 +++++++++++++++++ src/shared/exit-status.h | 1 + src/test/test-exit-status.c | 15 +++++++++++++++ 3 files changed, 33 insertions(+) diff --git a/src/shared/exit-status.c b/src/shared/exit-status.c index 51f2380027..12880f805e 100644 --- a/src/shared/exit-status.c +++ b/src/shared/exit-status.c @@ -6,7 +6,9 @@ #include "exit-status.h" #include "macro.h" +#include "parse-util.h" #include "set.h" +#include "string-util.h" const ExitStatusMapping exit_status_mappings[256] = { /* Exit status ranges: @@ -117,6 +119,21 @@ const char* exit_status_class(int code) { } } +int exit_status_from_string(const char *s) { + uint8_t val; + int r; + + for (size_t i = 0; i < ELEMENTSOF(exit_status_mappings); i++) + if (streq_ptr(s, exit_status_mappings[i].name)) + return i; + + r = safe_atou8(s, &val); + if (r < 0) + return r; + + return val; +} + bool is_clean_exit(int code, int status, ExitClean clean, ExitStatusSet *success_status) { if (code == CLD_EXITED) return status == 0 || diff --git a/src/shared/exit-status.h b/src/shared/exit-status.h index 425f841523..24eba79f56 100644 --- a/src/shared/exit-status.h +++ b/src/shared/exit-status.h @@ -89,6 +89,7 @@ typedef struct ExitStatusSet { const char* exit_status_to_string(int code, ExitStatusClass class) _const_; const char* exit_status_class(int code) _const_; +int exit_status_from_string(const char *s) _pure_; typedef struct ExitStatusMapping { const char *name; diff --git a/src/test/test-exit-status.c b/src/test/test-exit-status.c index bab0596580..3bcebd06a4 100644 --- a/src/test/test-exit-status.c +++ b/src/test/test-exit-status.c @@ -14,13 +14,28 @@ static void test_exit_status_to_string(void) { log_info("%d: %s%s%s%s", i, s ?: "-", class ? " (" : "", class ?: "", class ? ")" : ""); + + if (s) + assert_se(exit_status_from_string(s) == i); } } +static void test_exit_status_from_string(void) { + log_info("/* %s */", __func__); + + assert_se(exit_status_from_string("11") == 11); + assert_se(exit_status_from_string("-1") == -ERANGE); + assert_se(exit_status_from_string("256") == -ERANGE); + assert_se(exit_status_from_string("foo") == -EINVAL); + assert_se(exit_status_from_string("SUCCESS") == 0); + assert_se(exit_status_from_string("FAILURE") == 1); +} + int main(int argc, char *argv[]) { test_setup_logging(LOG_DEBUG); test_exit_status_to_string(); + test_exit_status_from_string(); return 0; } From 8594c8a552c02fb6fa2bf569e68aa73b739e8da6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 28 Jul 2019 11:04:12 +0200 Subject: [PATCH 03/12] shared/bitmap: constify various operators which don't modify bitmap --- src/shared/bitmap.c | 11 +++++------ src/shared/bitmap.h | 8 ++++---- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/shared/bitmap.c b/src/shared/bitmap.c index a956a42cab..de28b1055a 100644 --- a/src/shared/bitmap.c +++ b/src/shared/bitmap.c @@ -117,7 +117,7 @@ void bitmap_unset(Bitmap *b, unsigned n) { b->bitmaps[offset] &= ~bitmask; } -bool bitmap_isset(Bitmap *b, unsigned n) { +bool bitmap_isset(const Bitmap *b, unsigned n) { uint64_t bitmask; unsigned offset; @@ -134,7 +134,7 @@ bool bitmap_isset(Bitmap *b, unsigned n) { return !!(b->bitmaps[offset] & bitmask); } -bool bitmap_isclear(Bitmap *b) { +bool bitmap_isclear(const Bitmap *b) { unsigned i; if (!b) @@ -148,7 +148,6 @@ bool bitmap_isclear(Bitmap *b) { } void bitmap_clear(Bitmap *b) { - if (!b) return; @@ -157,7 +156,7 @@ void bitmap_clear(Bitmap *b) { b->bitmaps_allocated = 0; } -bool bitmap_iterate(Bitmap *b, Iterator *i, unsigned *n) { +bool bitmap_iterate(const Bitmap *b, Iterator *i, unsigned *n) { uint64_t bitmask; unsigned offset, rem; @@ -192,9 +191,9 @@ bool bitmap_iterate(Bitmap *b, Iterator *i, unsigned *n) { return false; } -bool bitmap_equal(Bitmap *a, Bitmap *b) { +bool bitmap_equal(const Bitmap *a, const Bitmap *b) { size_t common_n_bitmaps; - Bitmap *c; + const Bitmap *c; unsigned i; if (a == b) diff --git a/src/shared/bitmap.h b/src/shared/bitmap.h index 843d27d24d..611a3e0e9d 100644 --- a/src/shared/bitmap.h +++ b/src/shared/bitmap.h @@ -15,13 +15,13 @@ void bitmap_free(Bitmap *b); int bitmap_set(Bitmap *b, unsigned n); void bitmap_unset(Bitmap *b, unsigned n); -bool bitmap_isset(Bitmap *b, unsigned n); -bool bitmap_isclear(Bitmap *b); +bool bitmap_isset(const Bitmap *b, unsigned n); +bool bitmap_isclear(const Bitmap *b); void bitmap_clear(Bitmap *b); -bool bitmap_iterate(Bitmap *b, Iterator *i, unsigned *n); +bool bitmap_iterate(const Bitmap *b, Iterator *i, unsigned *n); -bool bitmap_equal(Bitmap *a, Bitmap *b); +bool bitmap_equal(const Bitmap *a, const Bitmap *b); #define BITMAP_FOREACH(n, b, i) \ for ((i).idx = 0; bitmap_iterate((b), &(i), (unsigned*)&(n)); ) From 23d5dd168724fe60c7b00d78f49563a6be05627d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 28 Jul 2019 11:14:46 +0200 Subject: [PATCH 04/12] shared/exit-status: use Bitmap instead of Sets I opted to embed the Bitmap structure directly in the ExitStatusSet. This means that memory usage is a bit higher for units which don't define this setting: Service changes: /* size: 2720, cachelines: 43, members: 73 */ /* sum members: 2680, holes: 9, sum holes: 39 */ /* sum bitfield members: 7 bits, bit holes: 1, sum bit holes: 1 bits */ /* last cacheline: 32 bytes */ /* size: 2816, cachelines: 44, members: 73 */ /* sum members: 2776, holes: 9, sum holes: 39 */ /* sum bitfield members: 7 bits, bit holes: 1, sum bit holes: 1 bits */ But this way the code is simpler and we do less pointer chasing. --- src/core/dbus-service.c | 32 ++++++------------- src/core/load-fragment.c | 12 +++---- src/shared/bitmap.c | 6 ---- src/shared/bitmap.h | 6 +++- src/shared/exit-status.c | 27 +++++++--------- src/shared/exit-status.h | 12 +++---- .../tty-ask-password-agent.c | 1 + 7 files changed, 38 insertions(+), 58 deletions(-) diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c index 07f02deed6..0b873fb486 100644 --- a/src/core/dbus-service.c +++ b/src/core/dbus-service.c @@ -39,9 +39,9 @@ static int property_get_exit_status_set( void *userdata, sd_bus_error *error) { - ExitStatusSet *status_set = userdata; + const ExitStatusSet *status_set = userdata; + unsigned n; Iterator i; - void *id; int r; assert(bus); @@ -56,13 +56,10 @@ static int property_get_exit_status_set( if (r < 0) return r; - SET_FOREACH(id, status_set->status, i) { - int32_t val = PTR_TO_INT(id); + BITMAP_FOREACH(n, &status_set->status, i) { + assert(n < 256); - if (val < 0 || val > 255) - continue; - - r = sd_bus_message_append_basic(reply, 'i', &val); + r = sd_bus_message_append_basic(reply, 'i', &n); if (r < 0) return r; } @@ -75,15 +72,14 @@ static int property_get_exit_status_set( if (r < 0) return r; - SET_FOREACH(id, status_set->signal, i) { - int32_t val = PTR_TO_INT(id); + BITMAP_FOREACH(n, &status_set->signal, i) { const char *str; - str = signal_to_string((int) val); + str = signal_to_string(n); if (!str) continue; - r = sd_bus_message_append_basic(reply, 'i', &val); + r = sd_bus_message_append_basic(reply, 'i', &n); if (r < 0) return r; } @@ -196,11 +192,7 @@ static int bus_set_transient_exit_status( return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid status code in %s: %"PRIi32, name, status[i]); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { - r = set_ensure_allocated(&status_set->status, NULL); - if (r < 0) - return r; - - r = set_put(status_set->status, INT_TO_PTR((int) status[i])); + r = bitmap_set(&status_set->status, status[i]); if (r < 0) return r; @@ -216,11 +208,7 @@ static int bus_set_transient_exit_status( return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid signal in %s: %"PRIi32, name, signal[i]); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { - r = set_ensure_allocated(&status_set->signal, NULL); - if (r < 0) - return r; - - r = set_put(status_set->signal, INT_TO_PTR((int) signal[i])); + r = bitmap_set(&status_set->signal, signal[i]); if (r < 0) return r; diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 3288b0b838..ecea4f526a 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -3937,7 +3937,7 @@ int config_parse_set_status( FOREACH_WORD(word, l, rvalue, state) { _cleanup_free_ char *temp; int val; - Set **set; + Bitmap *bitmap; temp = strndup(word, l); if (!temp) @@ -3951,20 +3951,16 @@ int config_parse_set_status( log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to parse value, ignoring: %s", word); continue; } - set = &status_set->signal; + bitmap = &status_set->signal; } else { if (val < 0 || val > 255) { log_syntax(unit, LOG_ERR, filename, line, 0, "Value %d is outside range 0-255, ignoring", val); continue; } - set = &status_set->status; + bitmap = &status_set->status; } - r = set_ensure_allocated(set, NULL); - if (r < 0) - return log_oom(); - - r = set_put(*set, INT_TO_PTR(val)); + r = bitmap_set(bitmap, val); if (r < 0) return log_oom(); } diff --git a/src/shared/bitmap.c b/src/shared/bitmap.c index de28b1055a..2eba72dd59 100644 --- a/src/shared/bitmap.c +++ b/src/shared/bitmap.c @@ -12,12 +12,6 @@ #include "macro.h" #include "memory-util.h" -struct Bitmap { - uint64_t *bitmaps; - size_t n_bitmaps; - size_t bitmaps_allocated; -}; - /* Bitmaps are only meant to store relatively small numbers * (corresponding to, say, an enum), so it is ok to limit * the max entry. 64k should be plenty. */ diff --git a/src/shared/bitmap.h b/src/shared/bitmap.h index 611a3e0e9d..f65a050584 100644 --- a/src/shared/bitmap.h +++ b/src/shared/bitmap.h @@ -6,7 +6,11 @@ #include "hashmap.h" #include "macro.h" -typedef struct Bitmap Bitmap; +typedef struct Bitmap { + uint64_t *bitmaps; + size_t n_bitmaps; + size_t bitmaps_allocated; +} Bitmap; Bitmap *bitmap_new(void); Bitmap *bitmap_copy(Bitmap *b); diff --git a/src/shared/exit-status.c b/src/shared/exit-status.c index 12880f805e..80ac4868cb 100644 --- a/src/shared/exit-status.c +++ b/src/shared/exit-status.c @@ -134,18 +134,19 @@ int exit_status_from_string(const char *s) { return val; } -bool is_clean_exit(int code, int status, ExitClean clean, ExitStatusSet *success_status) { +bool is_clean_exit(int code, int status, ExitClean clean, const ExitStatusSet *success_status) { if (code == CLD_EXITED) return status == 0 || (success_status && - set_contains(success_status->status, INT_TO_PTR(status))); + bitmap_isset(&success_status->status, status)); - /* If a daemon does not implement handlers for some of the signals that's not considered an unclean shutdown */ + /* If a daemon does not implement handlers for some of the signals, we do not consider this an + unclean shutdown */ if (code == CLD_KILLED) return (clean == EXIT_CLEAN_DAEMON && IN_SET(status, SIGHUP, SIGINT, SIGTERM, SIGPIPE)) || (success_status && - set_contains(success_status->signal, INT_TO_PTR(status))); + bitmap_isset(&success_status->signal, status)); return false; } @@ -153,26 +154,22 @@ bool is_clean_exit(int code, int status, ExitClean clean, ExitStatusSet *success void exit_status_set_free(ExitStatusSet *x) { assert(x); - x->status = set_free(x->status); - x->signal = set_free(x->signal); + bitmap_clear(&x->status); + bitmap_clear(&x->signal); } -bool exit_status_set_is_empty(ExitStatusSet *x) { +bool exit_status_set_is_empty(const ExitStatusSet *x) { if (!x) return true; - return set_isempty(x->status) && set_isempty(x->signal); + return bitmap_isclear(&x->status) && bitmap_isclear(&x->signal); } -bool exit_status_set_test(ExitStatusSet *x, int code, int status) { - - if (exit_status_set_is_empty(x)) - return false; - - if (code == CLD_EXITED && set_contains(x->status, INT_TO_PTR(status))) +bool exit_status_set_test(const ExitStatusSet *x, int code, int status) { + if (code == CLD_EXITED && bitmap_isset(&x->status, status)) return true; - if (IN_SET(code, CLD_KILLED, CLD_DUMPED) && set_contains(x->signal, INT_TO_PTR(status))) + if (IN_SET(code, CLD_KILLED, CLD_DUMPED) && bitmap_isset(&x->signal, status)) return true; return false; diff --git a/src/shared/exit-status.h b/src/shared/exit-status.h index 24eba79f56..d6da8c19b9 100644 --- a/src/shared/exit-status.h +++ b/src/shared/exit-status.h @@ -3,9 +3,9 @@ #include +#include "bitmap.h" #include "hashmap.h" #include "macro.h" -#include "set.h" /* This defines pretty names for the LSB 'start' verb exit codes. Note that they shouldn't be confused with * the LSB 'status' verb exit codes which are defined very differently. For details see: @@ -83,8 +83,8 @@ typedef enum ExitStatusClass { } ExitStatusClass; typedef struct ExitStatusSet { - Set *status; - Set *signal; + Bitmap status; + Bitmap signal; } ExitStatusSet; const char* exit_status_to_string(int code, ExitStatusClass class) _const_; @@ -103,8 +103,8 @@ typedef enum ExitClean { EXIT_CLEAN_COMMAND, } ExitClean; -bool is_clean_exit(int code, int status, ExitClean clean, ExitStatusSet *success_status); +bool is_clean_exit(int code, int status, ExitClean clean, const ExitStatusSet *success_status); void exit_status_set_free(ExitStatusSet *x); -bool exit_status_set_is_empty(ExitStatusSet *x); -bool exit_status_set_test(ExitStatusSet *x, int code, int status); +bool exit_status_set_is_empty(const ExitStatusSet *x); +bool exit_status_set_test(const ExitStatusSet *x, int code, int status); diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c index e17140ea0c..5f5245e48a 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -39,6 +39,7 @@ #include "plymouth-util.h" #include "pretty-print.h" #include "process-util.h" +#include "set.h" #include "signal-util.h" #include "socket-util.h" #include "string-util.h" From 32131a3aaad8f2102698fbd66a308d1d3e500da6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 28 Jul 2019 11:43:53 +0200 Subject: [PATCH 05/12] bus-util: convert bus_log_{parse,create}_error into defines With SYSTEMD_LOG_LOCATION=1, it is much more useful to see the location where the call to bus_log_{parse,create}_error() was made, rather then the one-line body of the helper function. Also, it's our internal code, so having a one-line non-inline function doesn't make much sense anyway. --- src/shared/bus-util.c | 8 -------- src/shared/bus-util.h | 7 +++++-- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index 8e301250bc..6af115e7aa 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -1480,14 +1480,6 @@ int bus_property_get_ulong( } #endif -int bus_log_parse_error(int r) { - return log_error_errno(r, "Failed to parse bus message: %m"); -} - -int bus_log_create_error(int r) { - return log_error_errno(r, "Failed to create bus message: %m"); -} - /** * bus_path_encode_unique() - encode unique object path * @b: bus connection or NULL diff --git a/src/shared/bus-util.h b/src/shared/bus-util.h index 3216b0c37a..1e2f04cc5d 100644 --- a/src/shared/bus-util.h +++ b/src/shared/bus-util.h @@ -114,8 +114,11 @@ assert_cc(sizeof(pid_t) == sizeof(uint32_t)); assert_cc(sizeof(mode_t) == sizeof(uint32_t)); #define bus_property_get_mode ((sd_bus_property_get_t) NULL) -int bus_log_parse_error(int r); -int bus_log_create_error(int r); +#define bus_log_parse_error(r) \ + log_error_errno(r, "Failed to parse bus message: %m") + +#define bus_log_create_error(r) \ + log_error_errno(r, "Failed to create bus message: %m") #define BUS_DEFINE_PROPERTY_GET_GLOBAL(function, bus_type, val) \ int function(sd_bus *bus, \ From 62b21e2e89b68f0a1fb209e6677c61fdb4c32b34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 28 Jul 2019 11:55:49 +0200 Subject: [PATCH 06/12] shared/bus-util: fix dbus serialization of {RestartPrevent,RestartForce,Success}ExitStatus We were passing 1/4th of the size in bytes as argument. So depending on the size of the array, either we'd only transfer a subset of values, or we'd get an alignment error. --- src/core/dbus-service.c | 16 ++++++++-------- src/shared/bus-unit-util.c | 14 +++++++------- src/systemctl/systemctl.c | 16 ++++++++-------- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c index 0b873fb486..fbda8d8a4c 100644 --- a/src/core/dbus-service.c +++ b/src/core/dbus-service.c @@ -159,18 +159,18 @@ static int bus_set_transient_exit_status( sd_bus_error *error) { const int32_t *status, *signal; - size_t sz_status, sz_signal, i; + size_t n_status, n_signal, i; int r; r = sd_bus_message_enter_container(message, 'r', "aiai"); if (r < 0) return r; - r = sd_bus_message_read_array(message, 'i', (const void **) &status, &sz_status); + r = sd_bus_message_read_array(message, 'i', (const void **) &status, &n_status); if (r < 0) return r; - r = sd_bus_message_read_array(message, 'i', (const void **) &signal, &sz_signal); + r = sd_bus_message_read_array(message, 'i', (const void **) &signal, &n_signal); if (r < 0) return r; @@ -178,16 +178,16 @@ static int bus_set_transient_exit_status( if (r < 0) return r; - sz_status /= sizeof(int32_t); - sz_signal /= sizeof(int32_t); + n_status /= sizeof(int32_t); + n_signal /= sizeof(int32_t); - if (sz_status == 0 && sz_signal == 0 && !UNIT_WRITE_FLAGS_NOOP(flags)) { + if (n_status == 0 && n_signal == 0 && !UNIT_WRITE_FLAGS_NOOP(flags)) { exit_status_set_free(status_set); unit_write_settingf(u, flags, name, "%s=", name); return 1; } - for (i = 0; i < sz_status; i++) { + for (i = 0; i < n_status; i++) { if (status[i] < 0 || status[i] > 255) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid status code in %s: %"PRIi32, name, status[i]); @@ -200,7 +200,7 @@ static int bus_set_transient_exit_status( } } - for (i = 0; i < sz_signal; i++) { + for (i = 0; i < n_signal; i++) { const char *str; str = signal_to_string((int) signal[i]); diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 322204dd22..99511d338a 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -1439,7 +1439,7 @@ static int bus_append_service_property(sd_bus_message *m, const char *field, con if (STR_IN_SET(field, "RestartPreventExitStatus", "RestartForceExitStatus", "SuccessExitStatus")) { _cleanup_free_ int *status = NULL, *signal = NULL; - size_t sz_status = 0, sz_signal = 0; + size_t n_status = 0, n_signal = 0; const char *p; for (p = eq;;) { @@ -1460,17 +1460,17 @@ static int bus_append_service_property(sd_bus_message *m, const char *field, con if (val < 0) return log_error_errno(r, "Invalid status or signal %s in %s: %m", word, field); - signal = reallocarray(signal, sz_signal + 1, sizeof(int)); + signal = reallocarray(signal, n_signal + 1, sizeof(int)); if (!signal) return log_oom(); - signal[sz_signal++] = val; + signal[n_signal++] = val; } else { - status = reallocarray(status, sz_status + 1, sizeof(int)); + status = reallocarray(status, n_status + 1, sizeof(int)); if (!status) return log_oom(); - status[sz_status++] = val; + status[n_status++] = val; } } @@ -1490,11 +1490,11 @@ static int bus_append_service_property(sd_bus_message *m, const char *field, con if (r < 0) return bus_log_create_error(r); - r = sd_bus_message_append_array(m, 'i', status, sz_status); + r = sd_bus_message_append_array(m, 'i', status, n_status * sizeof(int)); if (r < 0) return bus_log_create_error(r); - r = sd_bus_message_append_array(m, 'i', signal, sz_signal); + r = sd_bus_message_append_array(m, 'i', signal, n_signal * sizeof(int)); if (r < 0) return bus_log_create_error(r); diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index b9a7d488bd..9e8095c8b7 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -4911,17 +4911,17 @@ static int print_property(const char *name, const char *expected_value, sd_bus_m } else if (endswith(name, "ExitStatus") && streq(contents, "aiai")) { const int32_t *status, *signal; - size_t sz_status, sz_signal, i; + size_t n_status, n_signal, i; r = sd_bus_message_enter_container(m, 'r', "aiai"); if (r < 0) return bus_log_parse_error(r); - r = sd_bus_message_read_array(m, 'i', (const void **) &status, &sz_status); + r = sd_bus_message_read_array(m, 'i', (const void **) &status, &n_status); if (r < 0) return bus_log_parse_error(r); - r = sd_bus_message_read_array(m, 'i', (const void **) &signal, &sz_signal); + r = sd_bus_message_read_array(m, 'i', (const void **) &signal, &n_signal); if (r < 0) return bus_log_parse_error(r); @@ -4929,10 +4929,10 @@ static int print_property(const char *name, const char *expected_value, sd_bus_m if (r < 0) return bus_log_parse_error(r); - sz_status /= sizeof(int32_t); - sz_signal /= sizeof(int32_t); + n_status /= sizeof(int32_t); + n_signal /= sizeof(int32_t); - if (all || sz_status > 0 || sz_signal > 0) { + if (all || n_status > 0 || n_signal > 0) { bool first = true; if (!value) { @@ -4940,7 +4940,7 @@ static int print_property(const char *name, const char *expected_value, sd_bus_m fputc('=', stdout); } - for (i = 0; i < sz_status; i++) { + for (i = 0; i < n_status; i++) { if (status[i] < 0 || status[i] > 255) continue; @@ -4952,7 +4952,7 @@ static int print_property(const char *name, const char *expected_value, sd_bus_m printf("%"PRIi32, status[i]); } - for (i = 0; i < sz_signal; i++) { + for (i = 0; i < n_signal; i++) { const char *str; str = signal_to_string((int) signal[i]); From 2e2ed88062fcd4fbe138a5198a979ccdea4fb11c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 28 Jul 2019 12:37:31 +0200 Subject: [PATCH 07/12] pid1,systemctl: allow symbolic exit code names --- TODO | 3 --- man/systemd.service.xml | 37 +++++++++++++++++++++---------------- src/core/load-fragment.c | 28 ++++++++++++++-------------- src/shared/bus-unit-util.c | 32 +++++++++++++++++++------------- 4 files changed, 54 insertions(+), 46 deletions(-) diff --git a/TODO b/TODO index af41aa57ac..ae52d9fc3b 100644 --- a/TODO +++ b/TODO @@ -220,9 +220,6 @@ Features: * add --vacuum-xyz options to coredumpctl, matching those journalctl already has. -* SuccessExitStatus= and friends should probably also accept symbolic exit - codes names, i.e. error codes from the list maintained in exit-codes.[ch] - * introduce Ephemeral= unit file switch, that creates an ephemeral copy of all files and directories that are left writable for a unit, and which are removed after the unit goes down again. A bit like --ephemeral for diff --git a/man/systemd.service.xml b/man/systemd.service.xml index 90c1257f37..06116df1b0 100644 --- a/man/systemd.service.xml +++ b/man/systemd.service.xml @@ -852,27 +852,32 @@ SuccessExitStatus= - Takes a list of exit status definitions that, - when returned by the main service process, will be considered - successful termination, in addition to the normal successful - exit code 0 and the signals SIGHUP, - SIGINT, SIGTERM, and - SIGPIPE. Exit status definitions can - either be numeric exit codes or termination signal names, - separated by spaces. For example: - - SuccessExitStatus=1 2 8 SIGKILL - - ensures that exit codes 1, 2, 8 and - the termination signal SIGKILL are - considered clean service terminations. - + Takes a list of exit status definitions that, when returned by the main service + process, will be considered successful termination, in addition to the normal successful exit code 0 + and the signals SIGHUP, SIGINT, + SIGTERM, and SIGPIPE. Exit status definitions can be + numeric exit codes, termination code names, or termination signal names, separated by spaces. See the + Process Exit Codes section in + systemd.exec5 for + a list of termination codes names (for this setting only the part without the + EXIT_ or EX_ prefix should be used). See + signal7 for + a list of signal names. This option may appear more than once, in which case the list of successful exit statuses is merged. If the empty string is assigned to this option, the list is reset, all prior assignments of this option will have no - effect. + effect. + + + A service with with the the <varname>SuccessExitStatus=</varname> setting + + SuccessExitStatus=TEMPFAIL 250 SIGUSR1 + + Exit codes 75 (TEMPFAIL), 250, and the termination signal + SIGKILL are considered clean service terminations. + diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index ecea4f526a..8664500e1d 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -3936,33 +3936,33 @@ int config_parse_set_status( FOREACH_WORD(word, l, rvalue, state) { _cleanup_free_ char *temp; - int val; Bitmap *bitmap; temp = strndup(word, l); if (!temp) return log_oom(); - r = safe_atoi(temp, &val); - if (r < 0) { - val = signal_from_string(temp); + /* We need to call exit_status_from_string() first, because we want + * to parse numbers as exit statuses, not signals. */ - if (val <= 0) { - log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to parse value, ignoring: %s", word); + r = exit_status_from_string(temp); + if (r >= 0) { + assert(r >= 0 && r < 256); + bitmap = &status_set->status; + } else { + r = signal_from_string(temp); + + if (r <= 0) { + log_syntax(unit, LOG_ERR, filename, line, 0, + "Failed to parse value, ignoring: %s", word); continue; } bitmap = &status_set->signal; - } else { - if (val < 0 || val > 255) { - log_syntax(unit, LOG_ERR, filename, line, 0, "Value %d is outside range 0-255, ignoring", val); - continue; - } - bitmap = &status_set->status; } - r = bitmap_set(bitmap, val); + r = bitmap_set(bitmap, r); if (r < 0) - return log_oom(); + return log_error_errno(r, "Failed to set signal or status %s: %m", word); } if (!isempty(state)) log_syntax(unit, LOG_ERR, filename, line, 0, "Trailing garbage, ignoring."); diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 99511d338a..e53b9d5ea2 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -10,6 +10,7 @@ #include "cpu-set-util.h" #include "escape.h" #include "exec-util.h" +#include "exit-status.h" #include "hexdecoct.h" #include "hostname-util.h" #include "in-addr-util.h" @@ -1444,7 +1445,6 @@ static int bus_append_service_property(sd_bus_message *m, const char *field, con for (p = eq;;) { _cleanup_free_ char *word = NULL; - int val; r = extract_first_word(&p, &word, NULL, EXTRACT_UNQUOTE); if (r == 0) @@ -1454,24 +1454,30 @@ static int bus_append_service_property(sd_bus_message *m, const char *field, con if (r < 0) return log_error_errno(r, "Invalid syntax in %s: %s", field, eq); - r = safe_atoi(word, &val); - if (r < 0) { - val = signal_from_string(word); - if (val < 0) - return log_error_errno(r, "Invalid status or signal %s in %s: %m", word, field); + /* We need to call exit_status_from_string() first, because we want + * to parse numbers as exit statuses, not signals. */ - signal = reallocarray(signal, n_signal + 1, sizeof(int)); - if (!signal) - return log_oom(); + r = exit_status_from_string(word); + if (r >= 0) { + assert(r >= 0 && r < 256); - signal[n_signal++] = val; - } else { status = reallocarray(status, n_status + 1, sizeof(int)); if (!status) return log_oom(); - status[n_status++] = val; - } + status[n_status++] = r; + + } else if ((r = signal_from_string(word)) >= 0) { + signal = reallocarray(signal, n_signal + 1, sizeof(int)); + if (!signal) + return log_oom(); + + signal[n_signal++] = r; + + } else + /* original r from exit_status_to_string() */ + return log_error_errno(r, "Invalid status or signal %s in %s: %m", + word, field); } r = sd_bus_message_open_container(m, SD_BUS_TYPE_STRUCT, "sv"); From e7b9f4d9fa50e33b5755f5fc4b9717fbfc20c78d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 28 Jul 2019 13:00:01 +0200 Subject: [PATCH 08/12] pid1: fix message about triggers missing services systemd[1]: systemd-tmpfiles-clean.timer: Refusing to start, unit systemd-tmpfiles-cle an.timer to trigger not loaded. --- src/core/unit.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/core/unit.c b/src/core/unit.c index fa89bd4a4d..8fe02462be 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -5764,9 +5764,11 @@ int unit_test_trigger_loaded(Unit *u) { trigger = UNIT_TRIGGER(u); if (!trigger) - return log_unit_error_errno(u, SYNTHETIC_ERRNO(ENOENT), "Refusing to start, unit to trigger not loaded."); + return log_unit_error_errno(u, SYNTHETIC_ERRNO(ENOENT), + "Refusing to start, no unit to trigger."); if (trigger->load_state != UNIT_LOADED) - return log_unit_error_errno(u, SYNTHETIC_ERRNO(ENOENT), "Refusing to start, unit %s to trigger not loaded.", u->id); + return log_unit_error_errno(u, SYNTHETIC_ERRNO(ENOENT), + "Refusing to start, unit %s to trigger not loaded.", trigger->id); return 0; } From 148ffa2e4d43d45c53dbc0799d90400a2ba621ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 28 Jul 2019 13:47:39 +0200 Subject: [PATCH 09/12] systemctl: do print all statuses/signals received from pid1 If for some reason we do not know some signal, instead of silently skipping it, let's print it numerically. Likewise, 'show' is not the right place to do value filtering for exit codes. If pid1 accepted it, let's just print it with no fuss. --- src/systemctl/systemctl.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 9e8095c8b7..880a04411c 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -4941,9 +4941,6 @@ static int print_property(const char *name, const char *expected_value, sd_bus_m } for (i = 0; i < n_status; i++) { - if (status[i] < 0 || status[i] > 255) - continue; - if (first) first = false; else @@ -4956,15 +4953,16 @@ static int print_property(const char *name, const char *expected_value, sd_bus_m const char *str; str = signal_to_string((int) signal[i]); - if (!str) - continue; if (first) first = false; else fputc(' ', stdout); - fputs(str, stdout); + if (str) + fputs(str, stdout); + else + printf("%"PRIi32, status[i]); } fputc('\n', stdout); From 716e6f4488d3ea7bb91c40577b588253cfe00216 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 28 Jul 2019 15:00:08 +0200 Subject: [PATCH 10/12] units: use symbolic exit code names (nspawn uses 133 which doesn't have a name. That's reasonable, because there's less chance of conflict with a return value from the payload.) --- units/systemd-tmpfiles-clean.service.in | 2 +- units/systemd-tmpfiles-setup-dev.service.in | 2 +- units/systemd-tmpfiles-setup.service.in | 2 +- units/user/systemd-tmpfiles-clean.service.in | 2 +- units/user/systemd-tmpfiles-setup.service.in | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/units/systemd-tmpfiles-clean.service.in b/units/systemd-tmpfiles-clean.service.in index 9e2f04bfef..5d70aafb29 100644 --- a/units/systemd-tmpfiles-clean.service.in +++ b/units/systemd-tmpfiles-clean.service.in @@ -18,5 +18,5 @@ Before=shutdown.target [Service] Type=oneshot ExecStart=@rootbindir@/systemd-tmpfiles --clean -SuccessExitStatus=65 +SuccessExitStatus=DATAERR IOSchedulingClass=idle diff --git a/units/systemd-tmpfiles-setup-dev.service.in b/units/systemd-tmpfiles-setup-dev.service.in index 50df15c291..ed52db4953 100644 --- a/units/systemd-tmpfiles-setup-dev.service.in +++ b/units/systemd-tmpfiles-setup-dev.service.in @@ -19,4 +19,4 @@ Before=sysinit.target local-fs-pre.target systemd-udevd.service shutdown.target Type=oneshot RemainAfterExit=yes ExecStart=@rootbindir@/systemd-tmpfiles --prefix=/dev --create --boot -SuccessExitStatus=65 73 +SuccessExitStatus=DATAERR CANTCREAT diff --git a/units/systemd-tmpfiles-setup.service.in b/units/systemd-tmpfiles-setup.service.in index b02bbcd61b..32a475d715 100644 --- a/units/systemd-tmpfiles-setup.service.in +++ b/units/systemd-tmpfiles-setup.service.in @@ -20,4 +20,4 @@ RefuseManualStop=yes Type=oneshot RemainAfterExit=yes ExecStart=@rootbindir@/systemd-tmpfiles --create --remove --boot --exclude-prefix=/dev -SuccessExitStatus=65 73 +SuccessExitStatus=DATAERR CANTCREAT diff --git a/units/user/systemd-tmpfiles-clean.service.in b/units/user/systemd-tmpfiles-clean.service.in index 9cd19720d3..306b064e89 100644 --- a/units/user/systemd-tmpfiles-clean.service.in +++ b/units/user/systemd-tmpfiles-clean.service.in @@ -17,5 +17,5 @@ Before=basic.target shutdown.target [Service] Type=oneshot ExecStart=@rootbindir@/systemd-tmpfiles --user --clean -SuccessExitStatus=65 +SuccessExitStatus=DATAERR IOSchedulingClass=idle diff --git a/units/user/systemd-tmpfiles-setup.service.in b/units/user/systemd-tmpfiles-setup.service.in index 6467dab896..a852ef5748 100644 --- a/units/user/systemd-tmpfiles-setup.service.in +++ b/units/user/systemd-tmpfiles-setup.service.in @@ -19,7 +19,7 @@ RefuseManualStop=yes Type=oneshot RemainAfterExit=yes ExecStart=@rootbindir@/systemd-tmpfiles --user --create --remove --boot -SuccessExitStatus=65 +SuccessExitStatus=DATAERR [Install] WantedBy=basic.target From 76ed04d936f757763c32db5dbaaebd8b13785d7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 29 Jul 2019 15:44:39 +0200 Subject: [PATCH 11/12] analyze: add exit-codes verb --- man/systemd-analyze.xml | 30 +++++++++++++++++++++++++++ man/systemd.service.xml | 5 ++++- src/analyze/analyze.c | 45 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 1 deletion(-) diff --git a/man/systemd-analyze.xml b/man/systemd-analyze.xml index 7112362ac5..8e9f24caac 100644 --- a/man/systemd-analyze.xml +++ b/man/systemd-analyze.xml @@ -83,6 +83,12 @@ OPTIONS unit-paths + + systemd-analyze + OPTIONS + exit-codes + CODE + systemd-analyze OPTIONS @@ -365,6 +371,30 @@ $ eog targets.svg to retrieve the actual list that the manager uses, with any empty directories omitted. + + <command>systemd-analyze exit-codes <optional><replaceable>CODE</replaceable>...</optional></command> + + This command prints a list of exit codes along with their "class", i.e. the source of the + definition (one of glibc, systemd, LSB, or + BSD), see the Process Exit Codes section in + systemd.exec5. + If no additional arguments are specified, all known codes are are shown. Otherwise, only the + definitions for the specified codes are shown. + + + <command>Show some example exit code names</command> + + $ systemd-analyze exit-codes 0 1 {63..65} +NAME CODE CLASS +SUCCESS 0 glibc +FAILURE 1 glibc +- 63 - +USAGE 64 BSD +DATAERR 65 BSD + + + + <command>systemd-analyze condition <replaceable>CONDITION</replaceable>...</command> diff --git a/man/systemd.service.xml b/man/systemd.service.xml index 06116df1b0..40ac052ba5 100644 --- a/man/systemd.service.xml +++ b/man/systemd.service.xml @@ -877,7 +877,10 @@ Exit codes 75 (TEMPFAIL), 250, and the termination signal SIGKILL are considered clean service terminations. - + + + Note: systemd-analyze exit-codes may be used to list exit + codes and translate between numerical code values and names. diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index 92727974d6..c8767e88c8 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -24,6 +24,7 @@ #include "conf-files.h" #include "copy.h" #include "def.h" +#include "exit-status.h" #include "fd-util.h" #include "fileio.h" #include "format-table.h" @@ -1637,6 +1638,48 @@ static void dump_syscall_filter(const SyscallFilterSet *set) { printf(" %s%s%s\n", syscall[0] == '@' ? ansi_underline() : "", syscall, ansi_normal()); } +static int dump_exit_codes(int argc, char *argv[], void *userdata) { + _cleanup_(table_unrefp) Table *table = NULL; + int r; + + table = table_new("name", "code", "class"); + if (!table) + return log_oom(); + + if (strv_isempty(strv_skip(argv, 1))) + for (size_t i = 0; i < ELEMENTSOF(exit_status_mappings); i++) { + if (!exit_status_mappings[i].name) + continue; + + r = table_add_many(table, + TABLE_STRING, exit_status_mappings[i].name, + TABLE_UINT, i, + TABLE_STRING, exit_status_class(i)); + if (r < 0) + return r; + } + else + for (int i = 1; i < argc; i++) { + int code; + + code = exit_status_from_string(argv[i]); + if (code < 0) + return log_error_errno(r, "Invalid exit code \"%s\": %m", argv[i]); + + assert(code >= 0 && (size_t) code < ELEMENTSOF(exit_status_mappings)); + r = table_add_many(table, + TABLE_STRING, exit_status_mappings[code].name ?: "-", + TABLE_UINT, code, + TABLE_STRING, exit_status_class(code) ?: "-"); + if (r < 0) + return r; + } + + (void) pager_open(arg_pager_flags); + + return table_print(table, NULL); +} + static int dump_syscall_filters(int argc, char *argv[], void *userdata) { bool first = true; @@ -2165,6 +2208,7 @@ static int help(int argc, char *argv[], void *userdata) { " dump Output state serialization of service manager\n" " cat-config Show configuration file and drop-ins\n" " unit-paths List load directories for units\n" + " exit-codes List exit code definitions\n" " syscall-filter [NAME...] Print list of syscalls in seccomp filter\n" " condition CONDITION... Evaluate conditions and asserts\n" " verify FILE... Check unit files for correctness\n" @@ -2368,6 +2412,7 @@ static int run(int argc, char *argv[]) { { "dump", VERB_ANY, 1, 0, dump }, { "cat-config", 2, VERB_ANY, 0, cat_config }, { "unit-paths", 1, 1, 0, dump_unit_paths }, + { "exit-codes", VERB_ANY, VERB_ANY, 0, dump_exit_codes }, { "syscall-filter", VERB_ANY, VERB_ANY, 0, dump_syscall_filters }, { "condition", 2, VERB_ANY, 0, do_condition }, { "verify", 2, VERB_ANY, 0, do_verify }, From ae6a32c260ba990f15a31d0bf910227e91135a08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 29 Jul 2019 15:53:24 +0200 Subject: [PATCH 12/12] NEWS: add entry about exit status changes --- NEWS | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/NEWS b/NEWS index 3383357c41..865b09b7f6 100644 --- a/NEWS +++ b/NEWS @@ -104,6 +104,11 @@ CHANGES WITH 243 in spe: long number (with the length varying by architecture), so they can be unambiguously distinguished. + * SuccessExitStatus=, RestartPreventExitStatus=, and + RestartForceExitStatus= now accept exit code names (e.g. "DATAERR" is + equivalent to "65"). systemd-analyze learnt a new 'exit-codes' verb + to display those exit code name mappings. + * /usr/sbin/halt.local is no longer supported. Implementation in distributions was inconsistent and it seems this functionality was very rarely used.