Lukáš Nykrýn píše v Po 06. 08. 2012 v 15:26 +0200:
> Lennart Poettering píše v Pá 03. 08. 2012 v 20:46 +0200:
> > On Tue, 24.07.12 16:43, Lukas Nykryn (lnyk...@redhat.com) 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.
> >
> > I am not happy with the term "Code" here, it's a bit too generic to be
> > self explanatory.
> >
> > And I think I agree with Zbigniew to a certain degree: we might want to
> > think about whether we should also allow configuring non-failure exit
> > codes or so. I mean, I am not suggesting we should implement that
> > right-away, but just keep in mind how that setting would look like so
> > that we can keep things uniform if we add it later. (In fact, I'd
> > recommend not to implement it right away: adding an option nobody so far
> > needed seems like a bad idea if we want to keep our set of options
> > minimal).
> >
> > Also, I think we need to think further than just exit codes,
> > i.e. signals and stuff.
> >
> > Maybe DontRestartExitStatus=? The libc calls the generalization of
> > exit code and exit signal the "exit status", so that sounds like the
> > best term to use here.
> >
> > And then people could write:
> >
> > DontRestartExitStatus=SIGTERM 1 2 3 4
> >
> > or something like this?
> >
> > (Of course, the "don't" in the name is a negative option, which we try
> > to avoid, but it appears weird to have a positive option for this, so i
> > think it would be ok...)
> >
> > One day, if we really need to make the failure/success sets configurable
> > too, we could then add:
> >
> > DontFailExitStatus=SIGSEGV 1 2
> >
> > (But please, don't implement this bit just yet, let's wait for somebody
> > actually needing this. Note though, that Upstart actually does have
> > functionality like this).
> >
> > > + *ignored = new(int, k);
> >
> > Maybe use a bitfield here, like we do for the syscalls list?
> >
> > (Actualy two bitfields, one for the exit codes, one for the exit signals).
> >
> > > +static int check_ignored_rc(Service *s){
> >
> > Please don't use acronyms in symbols if it's easy to avoid them and no
> > strong incentive to use them.
> >
> > > + int n_restart_ignored_codes;
> >
> > Generally we try to used "unsigned" rather than "int" for values where
> > negative values really never make sense, like in this case. It gives
> > readers a bit of a hint that this field never can be negative and no
> > special cases like that are used.
> >
> > Otherwise looks good.
> >
> > Lennart
> >
>
> Thanks for the feedback. I have altered name of the option to
> RestartIgnoreExitStatus, but feel free to change to anything else and I
> have also stored the codes to a set instead of an array.
>
> Lukas
forgotten patch
>From 81ffeefb01aa73c3be2b2bba7500df5f8ebcdfc9 Mon Sep 17 00:00:00 2001
From: Lukas Nykryn <lnyk...@redhat.com>
Date: Mon, 6 Aug 2012 14:54:03 +0200
Subject: [PATCH] service: allow service to inhibit respawn with special
return code
In some cases, like wrong configuration, restarting after error
does not help, so administrator can specify statuses by RestartPreventExitStatus
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..8533136 100644
--- a/man/systemd.service.xml
+++ b/man/systemd.service.xml
@@ -558,6 +558,13 @@
</varlistentry>
<varlistentry>
+ <term><varname>RestartPreventExitStatus=</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 2b1cfa0..d077376 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -156,6 +156,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.RestartPreventExitStatus, config_parse_set_int, 0, offsetof(Service, restart_ignore_status)
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..ae894b7 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_ignore_status);
+ s->restart_ignore_status = 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_ignore_status, 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..f72ef35 100644
--- a/src/core/service.h
+++ b/src/core/service.h
@@ -115,6 +115,7 @@ struct Service {
ServiceType type;
ServiceRestart restart;
+ Set *restart_ignore_status;
/* 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..76ef5bf 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: %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
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
