Lennart Poettering píše v Po 06. 08. 2012 v 16:52 +0200:
> On Mon, 06.08.12 16:45, Lukáš Nykrýn (lnyk...@redhat.com) wrote:
>
> > + if (!*set) {
> > + set_free(*set);
> > + *set = NULL;
> > + }
>
> You probably mean if (*set) here, not (!*set). But honestly I think this
> bit should just go entirely as for most other settings we actually do
> allow adding in additional values later on if it makes sense (example:
> Environment=).
>
> > +
> > + *set = set_new(trivial_hash_func, trivial_compare_func);
> > + if (!*set)
> > + return -ENOMEM;
>
> We probably should start using log_oom() for things like this now that
> it is available.
>
> > +
> > + FOREACH_WORD(w, l, rvalue, state)
> > + {
> > + char *temp = new0(char, l + 1);
> > + if (!temp)
> > + return -ENOMEM;
>
> Please use strndup() here! (And also log_oom() here please).
>
> > +
> > + 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;
>
> I'd leave the hashmap set, as it is freed anyway when the service goes
> away, and makes simpler and easier to read.
>
> > + 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;
>
> Same here.
>
> Otherwise looks good.
>
> (We want the signal stuff eventually I guess, but we can easily add this
> later).
>
> Lennart
>
Again thanks for review. Here is modified patch.
If you think that it would be better to add signal stuff before
accepting this patch I will not disagree.
Lukas
>From 986dc67a74a4386e3e95f1b09cfa36dd71b77321 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 | 50 +++++++++++++++++++++++++++++++++
src/shared/conf-parser.h | 1 +
6 files changed, 66 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..151f19d 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,52 @@ 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);
+
+ *set = set_new(trivial_hash_func, trivial_compare_func);
+ if (!*set)
+ return log_oom();
+
+ FOREACH_WORD(w, l, rvalue, state)
+ {
+ char *temp = strndup(w, l);
+ if (!temp)
+ return log_oom();
+
+ r = safe_atoi(temp, &val);
+ free(temp);
+ if (r < 0) {
+ log_error("[%s:%u] Failed to parse numeric value: %s", filename, line, w);
+ return r;
+ }
+ r = set_put(*set, INT_TO_PTR(val));
+ if (r < 0) {
+ log_error("[%s:%u] Unable to store: %s", filename, line, w);
+ 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
