[systemd-commits] 3 commits - man/systemctl.xml src/libsystemd
Zbigniew JÄdrzejewski-Szmek
zbyszek at kemper.freedesktop.org
Wed Oct 8 16:43:47 PDT 2014
man/systemctl.xml | 7 +
src/libsystemd/sd-event/sd-event.c | 154 ++++++++++++++++++++++++-------------
2 files changed, 106 insertions(+), 55 deletions(-)
New commits:
commit f95387cda829bc053992c398789ce3aa6f42f81e
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date: Sat Oct 4 23:17:45 2014 -0400
sd-event: also update signal mask when disconnecting sources
diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c
index c5f062b..80a2ae9 100644
--- a/src/libsystemd/sd-event/sd-event.c
+++ b/src/libsystemd/sd-event/sd-event.c
@@ -598,6 +598,36 @@ static bool need_signal(sd_event *e, int signal) {
e->n_enabled_child_sources > 0);
}
+static int event_update_signal_fd(sd_event *e) {
+ struct epoll_event ev = {};
+ bool add_to_epoll;
+ int r;
+
+ assert(e);
+
+ add_to_epoll = e->signal_fd < 0;
+
+ r = signalfd(e->signal_fd, &e->sigset, SFD_NONBLOCK|SFD_CLOEXEC);
+ if (r < 0)
+ return -errno;
+
+ e->signal_fd = r;
+
+ if (!add_to_epoll)
+ return 0;
+
+ ev.events = EPOLLIN;
+ ev.data.ptr = INT_TO_PTR(SOURCE_SIGNAL);
+
+ r = epoll_ctl(e->epoll_fd, EPOLL_CTL_ADD, e->signal_fd, &ev);
+ if (r < 0) {
+ e->signal_fd = safe_close(e->signal_fd);
+ return -errno;
+ }
+
+ return 0;
+}
+
static void source_disconnect(sd_event_source *s) {
sd_event *event;
@@ -640,6 +670,10 @@ static void source_disconnect(sd_event_source *s) {
/* If the signal was on and now it is off... */
if (s->enabled != SD_EVENT_OFF && !need_signal(s->event, s->signal.sig)) {
assert_se(sigdelset(&s->event->sigset, s->signal.sig) == 0);
+
+ (void) event_update_signal_fd(s->event);
+ /* If disabling failed, we might get a spurious event,
+ * but otherwise nothing bad should happen. */
}
}
@@ -654,6 +688,10 @@ static void source_disconnect(sd_event_source *s) {
/* We know the signal was on, if it is off now... */
if (!need_signal(s->event, SIGCHLD)) {
assert_se(sigdelset(&s->event->sigset, SIGCHLD) == 0);
+
+ (void) event_update_signal_fd(s->event);
+ /* If disabling failed, we might get a spurious event,
+ * but otherwise nothing bad should happen. */
}
}
@@ -929,36 +967,6 @@ fail:
return r;
}
-static int event_update_signal_fd(sd_event *e) {
- struct epoll_event ev = {};
- bool add_to_epoll;
- int r;
-
- assert(e);
-
- add_to_epoll = e->signal_fd < 0;
-
- r = signalfd(e->signal_fd, &e->sigset, SFD_NONBLOCK|SFD_CLOEXEC);
- if (r < 0)
- return -errno;
-
- e->signal_fd = r;
-
- if (!add_to_epoll)
- return 0;
-
- ev.events = EPOLLIN;
- ev.data.ptr = INT_TO_PTR(SOURCE_SIGNAL);
-
- r = epoll_ctl(e->epoll_fd, EPOLL_CTL_ADD, e->signal_fd, &ev);
- if (r < 0) {
- e->signal_fd = safe_close(e->signal_fd);
- return -errno;
- }
-
- return 0;
-}
-
static int signal_exit_callback(sd_event_source *s, const struct signalfd_siginfo *si, void *userdata) {
assert(s);
commit 4807d2d068ae9fc08b87121fc0a574394f8acc5b
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date: Sat Oct 4 22:57:43 2014 -0400
sd-event: be more careful when enabling/disabling signals
When a child event is disabled (in order to be freed) and there is no
SIGCHLD signal event, sd_event_source_set_enabled will disable SIGCHLD
even if there are other child events.
Also remove some unneeded signalfd updates.
https://bugs.freedesktop.org/show_bug.cgi?id=84659
Based-on-a-patch-by: Hristo Venev <mustrumr97 at gmail.com>
diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c
index 4c67ee8..c5f062b 100644
--- a/src/libsystemd/sd-event/sd-event.c
+++ b/src/libsystemd/sd-event/sd-event.c
@@ -590,6 +590,14 @@ static struct clock_data* event_get_clock_data(sd_event *e, EventSourceType t) {
}
}
+static bool need_signal(sd_event *e, int signal) {
+ return (e->signal_sources && e->signal_sources[signal] &&
+ e->signal_sources[signal]->enabled != SD_EVENT_OFF)
+ ||
+ (signal == SIGCHLD &&
+ e->n_enabled_child_sources > 0);
+}
+
static void source_disconnect(sd_event_source *s) {
sd_event *event;
@@ -626,11 +634,13 @@ static void source_disconnect(sd_event_source *s) {
case SOURCE_SIGNAL:
if (s->signal.sig > 0) {
- if (s->signal.sig != SIGCHLD || s->event->n_enabled_child_sources == 0)
- assert_se(sigdelset(&s->event->sigset, s->signal.sig) == 0);
-
if (s->event->signal_sources)
s->event->signal_sources[s->signal.sig] = NULL;
+
+ /* If the signal was on and now it is off... */
+ if (s->enabled != SD_EVENT_OFF && !need_signal(s->event, s->signal.sig)) {
+ assert_se(sigdelset(&s->event->sigset, s->signal.sig) == 0);
+ }
}
break;
@@ -640,10 +650,12 @@ static void source_disconnect(sd_event_source *s) {
if (s->enabled != SD_EVENT_OFF) {
assert(s->event->n_enabled_child_sources > 0);
s->event->n_enabled_child_sources--;
- }
- if (!s->event->signal_sources || !s->event->signal_sources[SIGCHLD])
- assert_se(sigdelset(&s->event->sigset, SIGCHLD) == 0);
+ /* We know the signal was on, if it is off now... */
+ if (!need_signal(s->event, SIGCHLD)) {
+ assert_se(sigdelset(&s->event->sigset, SIGCHLD) == 0);
+ }
+ }
hashmap_remove(s->event->child_sources, INT_TO_PTR(s->child.pid));
}
@@ -963,6 +975,7 @@ _public_ int sd_event_add_signal(
sd_event_source *s;
sigset_t ss;
int r;
+ bool previous;
assert_return(e, -EINVAL);
assert_return(sig > 0, -EINVAL);
@@ -987,6 +1000,8 @@ _public_ int sd_event_add_signal(
} else if (e->signal_sources[sig])
return -EBUSY;
+ previous = need_signal(e, sig);
+
s = source_new(e, !ret, SOURCE_SIGNAL);
if (!s)
return -ENOMEM;
@@ -997,9 +1012,10 @@ _public_ int sd_event_add_signal(
s->enabled = SD_EVENT_ON;
e->signal_sources[sig] = s;
- assert_se(sigaddset(&e->sigset, sig) == 0);
- if (sig != SIGCHLD || e->n_enabled_child_sources == 0) {
+ if (!previous) {
+ assert_se(sigaddset(&e->sigset, sig) == 0);
+
r = event_update_signal_fd(e);
if (r < 0) {
source_free(s);
@@ -1023,6 +1039,7 @@ _public_ int sd_event_add_child(
sd_event_source *s;
int r;
+ bool previous;
assert_return(e, -EINVAL);
assert_return(pid > 1, -EINVAL);
@@ -1039,6 +1056,8 @@ _public_ int sd_event_add_child(
if (hashmap_contains(e->child_sources, INT_TO_PTR(pid)))
return -EBUSY;
+ previous = need_signal(e, SIGCHLD);
+
s = source_new(e, !ret, SOURCE_CHILD);
if (!s)
return -ENOMEM;
@@ -1057,9 +1076,9 @@ _public_ int sd_event_add_child(
e->n_enabled_child_sources ++;
- assert_se(sigaddset(&e->sigset, SIGCHLD) == 0);
+ if (!previous) {
+ assert_se(sigaddset(&e->sigset, SIGCHLD) == 0);
- if (!e->signal_sources || !e->signal_sources[SIGCHLD]) {
r = event_update_signal_fd(e);
if (r < 0) {
source_free(s);
@@ -1437,23 +1456,32 @@ _public_ int sd_event_source_set_enabled(sd_event_source *s, int m) {
}
case SOURCE_SIGNAL:
+ assert(need_signal(s->event, s->signal.sig));
+
s->enabled = m;
- if (s->signal.sig != SIGCHLD || s->event->n_enabled_child_sources == 0) {
+
+ if (!need_signal(s->event, s->signal.sig)) {
assert_se(sigdelset(&s->event->sigset, s->signal.sig) == 0);
- event_update_signal_fd(s->event);
+
+ (void) event_update_signal_fd(s->event);
+ /* If disabling failed, we might get a spurious event,
+ * but otherwise nothing bad should happen. */
}
break;
case SOURCE_CHILD:
+ assert(need_signal(s->event, SIGCHLD));
+
s->enabled = m;
assert(s->event->n_enabled_child_sources > 0);
s->event->n_enabled_child_sources--;
- if (!s->event->signal_sources || !s->event->signal_sources[SIGCHLD]) {
+ if (!need_signal(s->event, SIGCHLD)) {
assert_se(sigdelset(&s->event->sigset, SIGCHLD) == 0);
- event_update_signal_fd(s->event);
+
+ (void) event_update_signal_fd(s->event);
}
break;
@@ -1501,22 +1529,34 @@ _public_ int sd_event_source_set_enabled(sd_event_source *s, int m) {
}
case SOURCE_SIGNAL:
- s->enabled = m;
-
- if (s->signal.sig != SIGCHLD || s->event->n_enabled_child_sources == 0) {
+ /* Check status before enabling. */
+ if (!need_signal(s->event, s->signal.sig)) {
assert_se(sigaddset(&s->event->sigset, s->signal.sig) == 0);
- event_update_signal_fd(s->event);
+
+ r = event_update_signal_fd(s->event);
+ if (r < 0) {
+ s->enabled = SD_EVENT_OFF;
+ return r;
+ }
}
+
+ s->enabled = m;
break;
case SOURCE_CHILD:
+ /* Check status before enabling. */
if (s->enabled == SD_EVENT_OFF) {
- s->event->n_enabled_child_sources++;
-
- if (!s->event->signal_sources || !s->event->signal_sources[SIGCHLD]) {
- assert_se(sigaddset(&s->event->sigset, SIGCHLD) == 0);
- event_update_signal_fd(s->event);
+ if (!need_signal(s->event, SIGCHLD)) {
+ assert_se(sigaddset(&s->event->sigset, s->signal.sig) == 0);
+
+ r = event_update_signal_fd(s->event);
+ if (r < 0) {
+ s->enabled = SD_EVENT_OFF;
+ return r;
+ }
}
+
+ s->event->n_enabled_child_sources++;
}
s->enabled = m;
commit 751ea8deaf96cabd4f1321735cd86535840a3faf
Author: Jan Synacek <jsynacek at redhat.com>
Date: Wed Oct 8 08:04:52 2014 +0200
man/systemctl: document enable on masked units
diff --git a/man/systemctl.xml b/man/systemctl.xml
index b2aa17f..61a23de 100644
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -924,6 +924,9 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service
the system, or for all future logins of all users, or only this
boot. Note that in the last case, no systemd daemon
configuration is reloaded.</para>
+
+ <para>Using <command>enable</command> on masked units
+ results in an error.</para>
</listitem>
</varlistentry>
@@ -1080,8 +1083,8 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service
<filename>/dev/null</filename>, making it impossible to
start them. This is a stronger version of
<command>disable</command>, since it prohibits all kinds of
- activation of the unit, including manual activation. Use
- this option with care. This honors the
+ activation of the unit, including enablement and manual
+ activation. Use this option with care. This honors the
<option>--runtime</option> option to only mask temporarily
until the next reboot of the system.</para>
</listitem>
More information about the systemd-commits
mailing list