Hi,

I've just been debugging a weird problem in my 208 build (which is quite
similar to Fedora's - a lot of the same patches).

So far I've noticed two problems:


1. If I do "systemctl enable sysvinitscript" it will print me a warning
about how the units do not carry [Install] sections. This is
unsurprising as there are no units (the systemctl.c mangled_names - or
just names in git master) left after the enable_sysv_units() method is
called and the first item is a pointer to a null string. I suspect
various bus calls could just be harmlessly skipped if this is the case
(i.e. just don't do the various unit related method calls) as we know
they are handled locally.

I've attached two patches which should solve this (one for 208+patches
and one for git master aka 209) although I've not tested either
directly. Some other eyes on it as to whether it's the correct approach
or not would be appreciated. The only difference between the two
versions is a variable name change.



2. This is the much odder part of the problem I'm seeing. The call to
daemon_reload() at the end of the enable_unit() seems to trigger some
kind of broken daemon reload that puts things into a bad state,
including a stale /run/nologin file.

I'm not sure WHY this does this, but it's very reliably reproducible. I
have a native sysvinit script called numlock. All I need to do to
trigger the bad state is "systemctl disable numlock". After the call,
the systemd daemon is reloaded and it goes into this bad state
completely with /run/nologin file.

If I comment out call or use --no-reload, then all is well. If I call
"systemctl daemon-reload" on it's own, all seems well. It just seems to
be this reload call specifically at the end of enable_unit() that
triggers the bad state.



I'm going to try reverting some of the patches I have applied to see
where I get with things, as I see Zbigniew backed a few out of fedora
due to freeze rules, but I did also see some threads from Zbigniew about
the whole /run/nologin, so I suspect he may be interested in this.

Cheers


Col


-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited http://www.tribalogic.net/
Open Source:
  Mageia Contributor http://www.mageia.org/
  PulseAudio Hacker http://www.pulseaudio.org/
  Trac Hacker http://trac.edgewall.org/
>From 4c9ab5d7d7dc8e18769580fc2f7ac16b6d16bd91 Mon Sep 17 00:00:00 2001
From: Colin Guthrie <co...@mageia.org>
Date: Mon, 13 Jan 2014 11:06:35 +0000
Subject: [PATCH] systemctl: Do not attempt native calls for enable/disable if
 sysvinit handleds all units.

If you have sysvinit compat enabled, it might deal with all the passed
in unit names leaving only a null termitated strv structure.

This cannot be iterated (sensibly) and thus we should just bail out
and not bother calling various native methods.

This should prevent a bogus warning regarding the lack of [Install]
sections in systemd units when enabling a sysvinit unit.
---
 src/systemctl/systemctl.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index dd95df1..03dc50c 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -4677,6 +4677,11 @@ static int enable_unit(sd_bus *bus, char **args) {
                 return r;
 
         if (!bus || avoid_bus()) {
+                /* If the sysvinit compat calls have handled everything we may
+                 * have none left to deal with natively... */
+                if (!names[0])
+                        goto finish;
+
                 if (streq(verb, "enable")) {
                         r = unit_file_enable(arg_scope, arg_runtime, arg_root, names, arg_force, &changes, &n_changes);
                         carries_install_info = r;
@@ -4713,6 +4718,11 @@ static int enable_unit(sd_bus *bus, char **args) {
                 bool send_force = true;
                 const char *method;
 
+                /* If the sysvinit compat calls have handled everything we may
+                 * have none left to deal with natively... */
+                if (!names[0])
+                        goto reload;
+
                 if (streq(verb, "enable")) {
                         method = "EnableUnitFiles";
                         expect_carries_install_info = true;
@@ -4775,6 +4785,7 @@ static int enable_unit(sd_bus *bus, char **args) {
                 if (r < 0)
                         return r;
 
+reload:
                 /* Try to reload if enabeld */
                 if (!arg_no_reload)
                         r = daemon_reload(bus, args);
-- 
1.8.4.5

>From 75b710994721fd9dd58715765368ef4cde15172b Mon Sep 17 00:00:00 2001
From: Colin Guthrie <co...@mageia.org>
Date: Mon, 13 Jan 2014 11:06:35 +0000
Subject: [PATCH] systemctl: Do not attempt native calls for enable/disable if
 sysvinit handleds all units.

If you have sysvinit compat enabled, it might deal with all the passed
in unit names leaving only a null termitated strv structure.

This cannot be iterated (sensibly) and thus we should just bail out
and not bother calling various native methods.

This should prevent a bogus warning regarding the lack of [Install]
sections in systemd units when enabling a sysvinit unit.
---
 src/systemctl/systemctl.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index db584b2..9ff4350 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -4437,6 +4437,11 @@ static int enable_unit(DBusConnection *bus, char **args) {
                 return r;
 
         if (!bus || avoid_bus()) {
+                /* If the sysvinit compat calls have handled everything we may
+                 * have none left to deal with natively... */
+                if (!mangled_names[0])
+                        goto finish;
+
                 if (streq(verb, "enable")) {
                         r = unit_file_enable(arg_scope, arg_runtime, arg_root, mangled_names, arg_force, &changes, &n_changes);
                         carries_install_info = r;
@@ -4480,6 +4485,11 @@ static int enable_unit(DBusConnection *bus, char **args) {
                 dbus_bool_t a, b;
                 DBusMessageIter iter, sub, sub2;
 
+                /* If the sysvinit compat calls have handled everything we may
+                 * have none left to deal with natively... */
+                if (!mangled_names[0])
+                        goto reload;
+
                 if (streq(verb, "enable")) {
                         method = "EnableUnitFiles";
                         expect_carries_install_info = true;
@@ -4598,6 +4608,7 @@ static int enable_unit(DBusConnection *bus, char **args) {
                         dbus_message_iter_next(&sub);
                 }
 
+reload:
                 /* Try to reload if enabeld */
                 if (!arg_no_reload)
                         r = daemon_reload(bus, args);
-- 
1.8.4.5

_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Reply via email to