Bug#779244: (pre-approval) transitional /etc/insserv/overrides support for Jessie's systemd
Package: release.debian.org
Severity: normal
X-Debbugs-CC: pkg-systemd-maintainers@lists.alioth.debian.org
Dear release team,
the systemd team would be willing to support parsing
/etc/insserv/overrides in systemd to allow traditional overrides of
init scripts to take effect, but they make doing this conditional on
pre-approval by you (the release team). There is a bug report open on
this topic[0], which the systemd maintainers initially closed as
wontfix, but have reconsidered, due to the fact that systemd currently
does not properly support overriding dependencies in drop-ins (it may
in the future, but definitely not for Jessie, and also upstream's git
currently doesn't yet). For example, if one has a service that has a
dependency After=foo.service, this can only be removed if the service
file is copied verbatim into /etc/systemd/system and the dependency is
removed in the copy. It is not possible to generate a drop-in
(foo.service.d/*.conf) that does that (those only work if one wants to
add dependencies or change/remove non-dependency options). For native
systemd services, this is not an issue, but services for sysvinit
scripts are generated (and only if no native unit with the same name
exists), so overriding dependencies of sysvinit services is non-trivial
(but possible) with Jessie's current version. If systemd would also
parse /etc/insserv/overrides, overriding init script dependencies would
be much easier for administrators. Also note that using
/etc/insserv/overrides to override dependencies was not completely
uncommon in previous Debian versions (mainly Squeeze and Wheezy) and
this could greatly reduce some pain people might feel on upgrades. (It
would certainly help me a lot, even though I know the workaround.)
The systemd maintainers do plan to remove support for this after
Jessie, so this is a transitional measure to ease migration that only
makes sense if included in Jessie.
I wrote a patch that does this, it's attached to the original bug
report, and I've also attached it to this email. One issue with a
previous version of the patch was found and fixed (in case you're
reading the original report). I've used the current version
successfully on a VM for a couple of days now without any issues
arising from this.
I'd like to ask you to consider approving this change, so that one of
the next uploads of systemd for Jessie could include it.
Thank you for considering!
Christian
[0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=759001
[1]
https://bugs.debian.org/cgi-bin/bugreport.cgi?msg=106;filename=sysv-generator-add-support-for-etc-insserv-overrides.patch;att=1;bug=759001
From: Christian Seiler <christian@iwakd.de>
Date: Tue, 17 Feb 2015 00:27:21 +0100
Subject: sysv-generator: add support for /etc/insserv/overrides
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=759001
---
src/sysv-generator/sysv-generator.c | 33 +++++++++++++++++++++++----------
1 file changed, 23 insertions(+), 10 deletions(-)
--- a/src/sysv-generator/sysv-generator.c
+++ b/src/sysv-generator/sysv-generator.c
@@ -41,6 +41,8 @@
#include "fileio.h"
#include "hashmap.h"
+#define SYSV_OVERRIDE_PATH "/etc/insserv/overrides/"
+
typedef enum RunlevelType {
RUNLEVEL_SYSINIT,
RUNLEVEL_UP,
@@ -76,6 +78,7 @@ static const struct {
typedef struct SysvStub {
char *name;
char *path;
+ char *override_path;
char *description;
bool sysinit;
int sysv_start_priority;
@@ -207,6 +210,12 @@ static int generate_unit_file(SysvStub *
if (!isempty(conflicts))
fprintf(f, "Conflicts=%s\n", conflicts);
+ /* make systemctl status show the information that the headers
+ * were overridden; not the most elegant way, but SourcePath=
+ * only accepts a single entry */
+ if (s->override_path)
+ fprintf(f, "Documentation=file://%s\n", s->override_path);
+
fprintf(f,
"\n[Service]\n"
"Type=forking\n"
@@ -351,7 +360,7 @@ finish:
return 1;
}
-static int load_sysv(SysvStub *s) {
+static int load_sysv(SysvStub *s, const char *fpath, bool check_for_usage_only) {
_cleanup_fclose_ FILE *f;
unsigned line = 0;
int r;
@@ -368,7 +377,7 @@ static int load_sysv(SysvStub *s) {
assert(s);
- f = fopen(s->path, "re");
+ f = fopen(fpath, "re");
if (!f)
return errno == ENOENT ? 0 : -errno;
@@ -381,7 +390,7 @@ static int load_sysv(SysvStub *s) {
log_error_unit(s->name,
"Failed to read configuration file '%s': %m",
- s->path);
+ fpath);
return -errno;
}
@@ -406,6 +415,11 @@ static int load_sysv(SysvStub *s) {
continue;
}
+ /* If this is just the run to determine whether the init
+ * script supports reload. */
+ if (check_for_usage_only)
+ continue;
+
if (state == NORMAL && streq(t, "### BEGIN INIT INFO")) {
state = LSB;
s->has_lsb = true;
@@ -456,7 +470,7 @@ static int load_sysv(SysvStub *s) {
if (!path_is_absolute(fn)) {
log_error_unit(s->name,
"[%s:%u] PID file not absolute. Ignoring.",
- s->path, line);
+ fpath, line);
continue;
}
@@ -547,7 +561,7 @@ static int load_sysv(SysvStub *s) {
if (r < 0)
log_error_unit(s->name,
"[%s:%u] Failed to add LSB Provides name %s, ignoring: %s",
- s->path, line, m, strerror(-r));
+ fpath, line, m, strerror(-r));
}
} else if (startswith_no_case(t, "Required-Start:") ||
@@ -571,7 +585,7 @@ static int load_sysv(SysvStub *s) {
if (r < 0) {
log_error_unit(s->name,
"[%s:%u] Failed to translate LSB dependency %s, ignoring: %s",
- s->path, line, n, strerror(-r));
+ fpath, line, n, strerror(-r));
continue;
}
@@ -605,7 +619,7 @@ static int load_sysv(SysvStub *s) {
if (r < 0)
log_error_unit(s->name,
"[%s:%u] Failed to add dependency on %s, ignoring: %s",
- s->path, line, m, strerror(-r));
+ fpath, line, m, strerror(-r));
}
} else if (startswith_no_case(t, "Description:")) {
@@ -761,6 +775,7 @@ static int native_unit_exists(LookupPath
static int enumerate_sysv(LookupPaths lp, Hashmap *all_services) {
char **path;
+ int had_override = 0;
STRV_FOREACH(path, lp.sysvinit_path) {
_cleanup_closedir_ DIR *d = NULL;
@@ -776,7 +791,7 @@ static int enumerate_sysv(LookupPaths lp
while ((de = readdir(d))) {
SysvStub *service;
struct stat st;
- _cleanup_free_ char *fpath = NULL, *name = NULL;
+ _cleanup_free_ char *fpath = NULL, *name = NULL, *override_fpath = NULL;
int r;
if (ignore_file(de->d_name))
@@ -792,6 +807,19 @@ static int enumerate_sysv(LookupPaths lp
if (!(st.st_mode & S_IXUSR))
continue;
+ override_fpath = strjoin(SYSV_OVERRIDE_PATH, de->d_name, NULL);
+ if (!override_fpath)
+ return log_oom();
+
+ if (stat(override_fpath, &st) < 0) {
+ free(override_fpath);
+ override_fpath = NULL;
+ } else if (!had_override) {
+ /* Only display this message once. */
+ had_override = 1;
+ log_info("Using overrides in %s. This is only supported in Jessie as a transitional measure.", SYSV_OVERRIDE_PATH);
+ }
+
name = sysv_translate_name(de->d_name);
if (!name)
return log_oom();
@@ -812,8 +840,17 @@ static int enumerate_sysv(LookupPaths lp
service->sysv_start_priority = -1;
service->name = name;
service->path = fpath;
+ service->override_path = override_fpath;
+
+ if (override_fpath) {
+ r = load_sysv(service, override_fpath, false);
+ if (r < 0)
+ continue;
+ }
- r = load_sysv(service);
+ /* always read in the init script to determine whether
+ * it supports reload action */
+ r = load_sysv(service, fpath, (bool) override_fpath);
if (r < 0)
continue;
@@ -821,7 +858,7 @@ static int enumerate_sysv(LookupPaths lp
if (r < 0)
return log_oom();
- name = fpath = NULL;
+ name = fpath = override_fpath = NULL;
}
}
Reply to: