Lukáš Nykrýn píše v St 25. 07. 2012 v 10:26 +0200: > Zbigniew Jędrzejewski-Szmek píše v St 25. 07. 2012 v 01:19 +0200: > > On 07/24/2012 04:43 PM, Lukas Nykryn wrote: > > > In some cases, like wrong configuration, restarting after error > > > exit code does not help, so administrator can specify RestartIgnoreCodes > > > which will not cause restart of a service. > > Hi, > > > > maybe this should be made more general, not only limited to restarting: > > wouldn't it be better to simply specify, which exit codes are supposed > > to mean success? Before there was a case with java, which would exit > > returning 1, even on success. It would be nice to solve that problem too. > > > > Zbyszek > > Thanks for the suggestion, I did it this way, because this should be > solution for https://bugzilla.redhat.com/show_bug.cgi?id=807886. I am > not sure if I should rewrite it in more general way. If I am not > mistaken such behavior would also affect for example systemctl status > and then when service fails with wrong configuration, it would show as > success in status. > > But I do not say that specifying concrete return codes as success is not > acceptable feature, but I would rather see this two option separately. > I am going to rewrite this patch to use set instead of array, so I will > also write the second patch with "general approach". > > Lukas > > _______________________________________________ > systemd-devel mailing list > systemd-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Here is slightly modified original patch and patch to add possibility to specify successful return codes. Lukas
>From 0884a764af5dfd919aeb0f08ca5cef25d8074110 Mon Sep 17 00:00:00 2001 From: Lukas Nykryn <lnyk...@redhat.com> Date: Thu, 26 Jul 2012 10:02:37 +0200 Subject: [PATCH 1/2] service: allow service to inhibit respawn with special return code In some cases, like wrong configuration, restarting after error exit code does not help, so administrator can specify RestartInhibitCodes which will not cause restart of a service. --- man/systemd.service.xml | 7 ++++ src/core/load-fragment-gperf.gperf.m4 | 1 + src/core/service.c | 7 +++- src/core/service.h | 1 + src/shared/conf-parser.c | 60 +++++++++++++++++++++++++++++++++ src/shared/conf-parser.h | 1 + 6 files changed, 76 insertions(+), 1 deletions(-) diff --git a/man/systemd.service.xml b/man/systemd.service.xml index f43201d..db70930 100644 --- a/man/systemd.service.xml +++ b/man/systemd.service.xml @@ -558,6 +558,13 @@ </varlistentry> <varlistentry> + <term><varname>RestartInhibitCodes=</varname></term> + <listitem><para>Specify return codes list, which + will prevent service from restart. Codes are + separated by whitespace.</para></listitem> + </varlistentry> + + <varlistentry> <term><varname>PermissionsStartOnly=</varname></term> <listitem><para>Takes a boolean argument. If true, the permission diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4 index d6a4711..e865aec 100644 --- a/src/core/load-fragment-gperf.gperf.m4 +++ b/src/core/load-fragment-gperf.gperf.m4 @@ -155,6 +155,7 @@ Service.PermissionsStartOnly, config_parse_bool, 0, Service.RootDirectoryStartOnly, config_parse_bool, 0, offsetof(Service, root_directory_start_only) Service.RemainAfterExit, config_parse_bool, 0, offsetof(Service, remain_after_exit) Service.GuessMainPID, config_parse_bool, 0, offsetof(Service, guess_main_pid) +Service.RestartInhibitCodes, config_parse_set_int, 0, offsetof(Service, restart_inhibit_rcs) m4_ifdef(`HAVE_SYSV_COMPAT', `Service.SysVStartPriority, config_parse_sysv_priority, 0, offsetof(Service, sysv_start_priority)', `Service.SysVStartPriority, config_parse_warn_compat, 0, 0') diff --git a/src/core/service.c b/src/core/service.c index 1c127bd..e18ccec 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -293,6 +293,9 @@ static void service_done(Unit *u) { s->control_command = NULL; s->main_command = NULL; + set_free(s->restart_inhibit_rcs); + s->restart_inhibit_rcs = NULL; + /* This will leak a process, but at least no memory or any of * our resources */ service_unwatch_main_pid(s); @@ -1897,7 +1900,9 @@ static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart) (s->restart == SERVICE_RESTART_ON_SUCCESS && s->result == SERVICE_SUCCESS) || (s->restart == SERVICE_RESTART_ON_FAILURE && s->result != SERVICE_SUCCESS) || (s->restart == SERVICE_RESTART_ON_ABORT && (s->result == SERVICE_FAILURE_SIGNAL || - s->result == SERVICE_FAILURE_CORE_DUMP)))) { + s->result == SERVICE_FAILURE_CORE_DUMP))) && + (s->result != SERVICE_FAILURE_EXIT_CODE || + set_get(s->restart_inhibit_rcs, INT_TO_PTR(s->main_exec_status.status)) != INT_TO_PTR(s->main_exec_status.status) )) { r = unit_watch_timer(UNIT(s), s->restart_usec, &s->timer_watch); if (r < 0) diff --git a/src/core/service.h b/src/core/service.h index cc63347..e9fd979 100644 --- a/src/core/service.h +++ b/src/core/service.h @@ -115,6 +115,7 @@ struct Service { ServiceType type; ServiceRestart restart; + Set *restart_inhibit_rcs; /* If set we'll read the main daemon PID from this file */ char *pid_file; diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index 1eccec5..dc55a84 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -32,6 +32,7 @@ #include "log.h" #include "utf8.h" #include "path-util.h" +#include "set.h" int config_item_table_lookup( void *table, @@ -933,3 +934,62 @@ int config_parse_level( *o = (*o & LOG_FACMASK) | x; return 0; } + +int config_parse_set_int( + const char *filename, + unsigned line, + const char *section, + const char *lvalue, + int ltype, + const char *rvalue, + void *data, + void *userdata) { + + char *w; + size_t l; + char *state; + int r; + Set **set = data; + int val; + + assert(filename); + assert(lvalue); + assert(rvalue); + assert(data); + + if (!*set) { + set_free(*set); + *set = NULL; + } + + *set = set_new(trivial_hash_func, trivial_compare_func); + if (!*set) + return -ENOMEM; + + FOREACH_WORD(w, l, rvalue, state) + { + char *temp = new0(char, l + 1); + if (!temp) + return -ENOMEM; + + strncpy(temp, w, l); + r = safe_atoi(temp, &val); + free(temp); + if (r < 0) { + log_error("[%s:%u] Failed to parse numeric value: %s", filename, line, w); + set_free(*set); + *set = NULL; + return r; + } + r = set_put(*set, INT_TO_PTR(val)); + if (r < 0) { + log_error("[%s:%u] Unable to store RestartInhibitCodes set: %s", filename, line, w); + set_free(*set); + *set = NULL; + return r; + } + + } + + return 0; +} diff --git a/src/shared/conf-parser.h b/src/shared/conf-parser.h index 4f94b3b..66a5f4e 100644 --- a/src/shared/conf-parser.h +++ b/src/shared/conf-parser.h @@ -105,6 +105,7 @@ int config_parse_nsec(const char *filename, unsigned line, const char *section, int config_parse_mode(const char *filename, unsigned line, const char *section, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_facility(const char *filename, unsigned line, const char *section, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_level(const char *filename, unsigned line, const char *section, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); +int config_parse_set_int(const char *filename, unsigned line, const char *section, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); #define DEFINE_CONFIG_PARSE_ENUM(function,name,type,msg) \ int function( \ -- 1.7.6.5
>From a86b9d70128ce2a019ecb2d3296b361d04752080 Mon Sep 17 00:00:00 2001 From: Lukas Nykryn <lnyk...@redhat.com> Date: Thu, 26 Jul 2012 13:40:25 +0200 Subject: [PATCH 2/2] service: allow to specify successful return codes Option SuccessReturnCodes can specify return codes which will mean successful end of the service. --- man/systemd.service.xml | 7 ++++ src/core/load-fragment-gperf.gperf.m4 | 1 + src/core/mount.c | 2 +- src/core/service.c | 5 ++- src/core/service.h | 2 + src/core/socket.c | 2 +- src/core/swap.c | 2 +- src/remount-fs/remount-fs.c | 2 +- src/shared/conf-parser.c | 60 +++++++++++++++++++++++++++++++++ src/shared/exit-status.c | 15 +++++--- src/shared/exit-status.h | 5 ++- src/shared/util.c | 2 +- src/systemctl/systemctl.c | 2 +- 13 files changed, 93 insertions(+), 14 deletions(-) diff --git a/man/systemd.service.xml b/man/systemd.service.xml index f43201d..2698690 100644 --- a/man/systemd.service.xml +++ b/man/systemd.service.xml @@ -450,6 +450,13 @@ supported.</para></listitem> </varlistentry> + <varlistentry> + <term><varname>SuccessReturnCodes=</varname></term> + <listitem><para>Specify which return codes + besides 0 should be considered as successful + exit.</para></listitem> + </varlistentry> + <varlistentry> <term><varname>RestartSec=</varname></term> <listitem><para>Configures the time to diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4 index d6a4711..0288b1c 100644 --- a/src/core/load-fragment-gperf.gperf.m4 +++ b/src/core/load-fragment-gperf.gperf.m4 @@ -143,6 +143,7 @@ Service.ExecStartPost, config_parse_exec, SERVICE_EXE Service.ExecReload, config_parse_exec, SERVICE_EXEC_RELOAD, offsetof(Service, exec_command) Service.ExecStop, config_parse_exec, SERVICE_EXEC_STOP, offsetof(Service, exec_command) Service.ExecStopPost, config_parse_exec, SERVICE_EXEC_STOP_POST, offsetof(Service, exec_command) +Service.SuccessReturnCodes, config_parse_set_int, 0, offsetof(Service, success_rcs) Service.RestartSec, config_parse_usec, 0, offsetof(Service, restart_usec) Service.TimeoutSec, config_parse_service_timeout, 0, offsetof(Service, timeout_usec) Service.WatchdogSec, config_parse_usec, 0, offsetof(Service, watchdog_usec) diff --git a/src/core/mount.c b/src/core/mount.c index 83e51a7..fc981c7 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1225,7 +1225,7 @@ static void mount_sigchld_event(Unit *u, pid_t pid, int code, int status) { m->control_pid = 0; - if (is_clean_exit(code, status)) + if (is_clean_exit(code, status, NULL)) f = MOUNT_SUCCESS; else if (code == CLD_EXITED) f = MOUNT_FAILURE_EXIT_CODE; diff --git a/src/core/service.c b/src/core/service.c index 1c127bd..852a18a 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -285,6 +285,9 @@ static void service_done(Unit *u) { s->sysv_runlevels = NULL; #endif + set_free(s->success_rcs); + s->success_rcs = NULL; + free(s->status_text); s->status_text = NULL; @@ -2867,7 +2870,7 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) { assert(s); assert(pid >= 0); - if (UNIT(s)->fragment_path ? is_clean_exit(code, status) : is_clean_exit_lsb(code, status)) + if (UNIT(s)->fragment_path ? is_clean_exit(code, status, s->success_rcs) : is_clean_exit_lsb(code, status, s->success_rcs)) f = SERVICE_SUCCESS; else if (code == CLD_EXITED) f = SERVICE_FAILURE_EXIT_CODE; diff --git a/src/core/service.h b/src/core/service.h index cc63347..c8036df 100644 --- a/src/core/service.h +++ b/src/core/service.h @@ -116,6 +116,8 @@ struct Service { ServiceType type; ServiceRestart restart; + Set *success_rcs; + /* If set we'll read the main daemon PID from this file */ char *pid_file; diff --git a/src/core/socket.c b/src/core/socket.c index 837b166..cbbfb0c 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -1884,7 +1884,7 @@ static void socket_sigchld_event(Unit *u, pid_t pid, int code, int status) { s->control_pid = 0; - if (is_clean_exit(code, status)) + if (is_clean_exit(code, status, NULL)) f = SOCKET_SUCCESS; else if (code == CLD_EXITED) f = SOCKET_FAILURE_EXIT_CODE; diff --git a/src/core/swap.c b/src/core/swap.c index 458e00e..cd4d9ab 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -917,7 +917,7 @@ static void swap_sigchld_event(Unit *u, pid_t pid, int code, int status) { s->control_pid = 0; - if (is_clean_exit(code, status)) + if (is_clean_exit(code, status, NULL)) f = SWAP_SUCCESS; else if (code == CLD_EXITED) f = SWAP_FAILURE_EXIT_CODE; diff --git a/src/remount-fs/remount-fs.c b/src/remount-fs/remount-fs.c index 8b3aaeb..3fdb7b3 100644 --- a/src/remount-fs/remount-fs.c +++ b/src/remount-fs/remount-fs.c @@ -145,7 +145,7 @@ int main(int argc, char *argv[]) { s = hashmap_remove(pids, UINT_TO_PTR(si.si_pid)); if (s) { - if (!is_clean_exit(si.si_code, si.si_status)) { + if (!is_clean_exit(si.si_code, si.si_status, NULL)) { if (si.si_code == CLD_EXITED) log_error("/bin/mount for %s exited with exit status %i.", s, si.si_status); else diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index 1eccec5..dc55a84 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -32,6 +32,7 @@ #include "log.h" #include "utf8.h" #include "path-util.h" +#include "set.h" int config_item_table_lookup( void *table, @@ -933,3 +934,62 @@ int config_parse_level( *o = (*o & LOG_FACMASK) | x; return 0; } + +int config_parse_set_int( + const char *filename, + unsigned line, + const char *section, + const char *lvalue, + int ltype, + const char *rvalue, + void *data, + void *userdata) { + + char *w; + size_t l; + char *state; + int r; + Set **set = data; + int val; + + assert(filename); + assert(lvalue); + assert(rvalue); + assert(data); + + if (!*set) { + set_free(*set); + *set = NULL; + } + + *set = set_new(trivial_hash_func, trivial_compare_func); + if (!*set) + return -ENOMEM; + + FOREACH_WORD(w, l, rvalue, state) + { + char *temp = new0(char, l + 1); + if (!temp) + return -ENOMEM; + + strncpy(temp, w, l); + r = safe_atoi(temp, &val); + free(temp); + if (r < 0) { + log_error("[%s:%u] Failed to parse numeric value: %s", filename, line, w); + set_free(*set); + *set = NULL; + return r; + } + r = set_put(*set, INT_TO_PTR(val)); + if (r < 0) { + log_error("[%s:%u] Unable to store RestartInhibitCodes set: %s", filename, line, w); + set_free(*set); + *set = NULL; + return r; + } + + } + + return 0; +} diff --git a/src/shared/exit-status.c b/src/shared/exit-status.c index 0dc82b2..7381097 100644 --- a/src/shared/exit-status.c +++ b/src/shared/exit-status.c @@ -23,6 +23,8 @@ #include <sys/wait.h> #include "exit-status.h" +#include "set.h" +#include "macro.h" const char* exit_status_to_string(ExitStatus status, ExitStatusLevel level) { @@ -158,10 +160,13 @@ const char* exit_status_to_string(ExitStatus status, ExitStatusLevel level) { } -bool is_clean_exit(int code, int status) { +bool is_clean_exit(int code, int status, Set *success_codes) { - if (code == CLD_EXITED) - return status == 0; + if (code == CLD_EXITED){ + return + set_get(success_codes, INT_TO_PTR(status)) == INT_TO_PTR(status) || + status == 0; + } /* If a daemon does not implement handlers for some of the * signals that's not considered an unclean shutdown */ @@ -175,9 +180,9 @@ bool is_clean_exit(int code, int status) { return false; } -bool is_clean_exit_lsb(int code, int status) { +bool is_clean_exit_lsb(int code, int status, Set *success_codes) { - if (is_clean_exit(code, status)) + if (is_clean_exit(code, status, success_codes)) return true; return diff --git a/src/shared/exit-status.h b/src/shared/exit-status.h index 3f42c15..77545ca 100644 --- a/src/shared/exit-status.h +++ b/src/shared/exit-status.h @@ -22,6 +22,7 @@ ***/ #include <stdbool.h> +#include "set.h" typedef enum ExitStatus { /* EXIT_SUCCESS defined by libc */ @@ -79,5 +80,5 @@ typedef enum ExitStatusLevel { const char* exit_status_to_string(ExitStatus status, ExitStatusLevel level); -bool is_clean_exit(int code, int status); -bool is_clean_exit_lsb(int code, int status); +bool is_clean_exit(int code, int status, Set *success_codes); +bool is_clean_exit_lsb(int code, int status, Set *success_codes); diff --git a/src/shared/util.c b/src/shared/util.c index 2e7ae63..6843824 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -4329,7 +4329,7 @@ void execute_directory(const char *directory, DIR *d, char *argv[]) { } if ((path = hashmap_remove(pids, UINT_TO_PTR(si.si_pid)))) { - if (!is_clean_exit(si.si_code, si.si_status)) { + if (!is_clean_exit(si.si_code, si.si_status, NULL)) { if (si.si_code == CLD_EXITED) log_error("%s exited with exit status %i.", path, si.si_status); else diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index d493733..920811e 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -2476,7 +2476,7 @@ static void print_status_info(UnitStatusInfo *i) { printf("\t Process: %u %s=%s ", p->pid, p->name, strna(t)); free(t); - good = is_clean_exit_lsb(p->code, p->status); + good = is_clean_exit_lsb(p->code, p->status, NULL); if (!good) { on = ansi_highlight_red(true); off = ansi_highlight_red(false); -- 1.7.6.5
_______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel