cluster-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
* [Cluster-devel] cluster4 dlm: startup notification for systemd
@ 2012-11-03 15:27 Jacek Konieczny
  2012-11-03 15:27 ` [Cluster-devel] [PATCH 1/2] --foreground option added to dlm_controld Jacek Konieczny
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Jacek Konieczny @ 2012-11-03 15:27 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Hello,

The two patches:

   [PATCH 1/2] --foreground option added to dlm_controld
   [PATCH 2/2] Startup notification by sd_notify()

add startup notification for the systemd service unit. This way startup
of services depending on DLM can be properly serialized.

Currently dlm_controld forks immediately and the parent exits befor the DLM
subsystem is properly initialized. If clvmd is started next, it will fail
with some cryptic error messages (like 'dlm: no local IP address has been
set'). With the startup notification clvmd startup can be delayed until
dlm_controld reports it is ready.

Similar thing could be implemented with forking and no external dependency, ?
but that is a bit more complicated, as requires communication between the
parent and child process or moving the initialization code before fork().

sd_notify() could also be used to provide status information during the
start-up phase too, but I am not familiar enough with what is happening there
to provide reasonable status messages.

Greets,
	Jacek



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Cluster-devel] [PATCH 1/2] --foreground option added to dlm_controld
  2012-11-03 15:27 [Cluster-devel] cluster4 dlm: startup notification for systemd Jacek Konieczny
@ 2012-11-03 15:27 ` Jacek Konieczny
  2012-11-03 15:27 ` [Cluster-devel] [PATCH 2/2] Startup notification by sd_notify() Jacek Konieczny
  2012-11-05 16:20 ` [Cluster-devel] cluster4 dlm: startup notification for systemd David Teigland
  2 siblings, 0 replies; 4+ messages in thread
From: Jacek Konieczny @ 2012-11-03 15:27 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Foreground is the preferred way to run services from systemd
and it is easier to provide startup notification this way.

Signed-off-by: Jacek Konieczny <jajcus@jajcus.net>
---
 dlm_controld/dlm_controld.8 |    3 +++
 dlm_controld/dlm_daemon.h   |    1 +
 dlm_controld/main.c         |    7 ++++++-
 3 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/dlm_controld/dlm_controld.8 b/dlm_controld/dlm_controld.8
index 262ef8a..c9011fd 100644
--- a/dlm_controld/dlm_controld.8
+++ b/dlm_controld/dlm_controld.8
@@ -23,6 +23,9 @@ For default settings, see dlm_controld -h.
 .B --daemon_debug | -D
         enable debugging to stderr and don't fork
 
+.B --foreground
+        don't fork
+
 .B --log_debug | -K
         enable kernel dlm debugging messages
 
diff --git a/dlm_controld/dlm_daemon.h b/dlm_controld/dlm_daemon.h
index 916ee58..377f28d 100644
--- a/dlm_controld/dlm_daemon.h
+++ b/dlm_controld/dlm_daemon.h
@@ -109,6 +109,7 @@ enum {
         enable_quorum_lockspace_ind,
         help_ind,
         version_ind,
+        foreground_ind,
         dlm_options_max,
 };
 
diff --git a/dlm_controld/main.c b/dlm_controld/main.c
index fd469f9..deda60c 100644
--- a/dlm_controld/main.c
+++ b/dlm_controld/main.c
@@ -1245,6 +1245,11 @@ static void set_opt_defaults(void)
 			0, NULL,
 			"enable debugging to stderr and don't fork");
 
+	set_opt_default(foreground_ind,
+			"foreground", '\0', no_arg,
+			0, NULL,
+			"don't fork");
+
 	set_opt_default(log_debug_ind,
 			"log_debug", 'K', no_arg,
 			0, NULL,
@@ -1562,7 +1567,7 @@ int main(int argc, char **argv)
 	INIT_LIST_HEAD(&fs_register_list);
 	init_daemon();
 
-	if (!opt(daemon_debug_ind)) {
+	if (!opt(daemon_debug_ind) && !opt(foreground_ind)) {
 		if (daemon(0, 0) < 0) {
 			perror("daemon error");
 			exit(EXIT_FAILURE);
-- 
1.7.7.4



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [Cluster-devel] [PATCH 2/2] Startup notification by sd_notify()
  2012-11-03 15:27 [Cluster-devel] cluster4 dlm: startup notification for systemd Jacek Konieczny
  2012-11-03 15:27 ` [Cluster-devel] [PATCH 1/2] --foreground option added to dlm_controld Jacek Konieczny
@ 2012-11-03 15:27 ` Jacek Konieczny
  2012-11-05 16:20 ` [Cluster-devel] cluster4 dlm: startup notification for systemd David Teigland
  2 siblings, 0 replies; 4+ messages in thread
From: Jacek Konieczny @ 2012-11-03 15:27 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Implements simple start-up status notification via the sd_notify(3)
function from the libsystemd-daemon library. This allows proper
service startup serialization by the init daemon ? services depending
on DLM will be started after dlm_controld completes the initialization.

As this pulls additional dependency, the feature my by disabled on
compile time by calling 'make' with 'USE_SD_NOTIFY=no'.

Signed-off-by: Jacek Konieczny <jajcus@jajcus.net>
---
 dlm_controld/Makefile |    8 ++++++++
 dlm_controld/main.c   |    8 ++++++++
 init/dlm.service      |    5 +++--
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/dlm_controld/Makefile b/dlm_controld/Makefile
index f42a913..13d6339 100644
--- a/dlm_controld/Makefile
+++ b/dlm_controld/Makefile
@@ -6,6 +6,8 @@ LIBDIR=$(PREFIX)/$(LIBNUM)
 HDRDIR=$(PREFIX)/include 
 MANDIR=$(PREFIX)/share/man
 
+USE_SD_NOTIFY=yes
+
 BIN_TARGET = dlm_controld
 
 LIB_NAME = libdlmcontrol
@@ -59,6 +61,12 @@ BIN_LDFLAGS += -lpthread -lrt -lcpg -lcmap -lcfg -lquorum
 LIB_CFLAGS += $(BIN_CFLAGS)
 LIB_LDFLAGS += -Wl,-z,relro -pie
 
+ifeq ($(USE_SD_NOTIFY),yes)
+	BIN_CFLAGS += $(shell pkg-config --cflags libsystemd-daemon) \
+		      -DUSE_SD_NOTIFY
+	BIN_LDFLAGS += $(shell pkg-config --libs libsystemd-daemon)
+endif
+
 all: $(LIB_TARGET) $(BIN_TARGET)
 
 $(BIN_TARGET): $(BIN_SOURCE)
diff --git a/dlm_controld/main.c b/dlm_controld/main.c
index deda60c..d1fdb2a 100644
--- a/dlm_controld/main.c
+++ b/dlm_controld/main.c
@@ -14,6 +14,10 @@
 #include <linux/genetlink.h>
 #include <linux/dlm_netlink.h>
 
+#ifdef USE_SD_NOTIFY
+#include <systemd/sd-daemon.h>
+#endif
+
 #include "copyright.cf"
 #include "version.cf"
 
@@ -1012,6 +1016,10 @@ static void loop(void)
 	plock_fd = rv;
 	plock_ci = client_add(rv, process_plocks, NULL);
 
+#ifdef USE_SD_NOTIFY
+	sd_notify(0, "READY=1");
+#endif
+
 	for (;;) {
 		rv = poll(pollfd, client_maxi + 1, poll_timeout);
 		if (rv == -1 && errno == EINTR) {
diff --git a/init/dlm.service b/init/dlm.service
index 3c4e53e..f068c63 100644
--- a/init/dlm.service
+++ b/init/dlm.service
@@ -3,10 +3,11 @@ Description=dlm control daemon
 After=syslog.target network.target corosync.service sys-kernel-config.mount
 
 [Service]
-Type=forking
+Type=notify
+NotifyAccess=main
 EnvironmentFile=/etc/sysconfig/dlm
 ExecStartPre=/sbin/modprobe dlm 
-ExecStart=/usr/sbin/dlm_controld $DLM_CONTROLD_OPTS
+ExecStart=/usr/sbin/dlm_controld --foreground $DLM_CONTROLD_OPTS
 #ExecStopPost=/sbin/modprobe -r dlm
 
 [Install]
-- 
1.7.7.4



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [Cluster-devel] cluster4 dlm: startup notification for systemd
  2012-11-03 15:27 [Cluster-devel] cluster4 dlm: startup notification for systemd Jacek Konieczny
  2012-11-03 15:27 ` [Cluster-devel] [PATCH 1/2] --foreground option added to dlm_controld Jacek Konieczny
  2012-11-03 15:27 ` [Cluster-devel] [PATCH 2/2] Startup notification by sd_notify() Jacek Konieczny
@ 2012-11-05 16:20 ` David Teigland
  2 siblings, 0 replies; 4+ messages in thread
From: David Teigland @ 2012-11-05 16:20 UTC (permalink / raw)
  To: cluster-devel.redhat.com

On Sat, Nov 03, 2012 at 04:27:54PM +0100, Jacek Konieczny wrote:
> Hello,
> 
> The two patches:
> 
>    [PATCH 1/2] --foreground option added to dlm_controld
>    [PATCH 2/2] Startup notification by sd_notify()
> 
> add startup notification for the systemd service unit. This way startup
> of services depending on DLM can be properly serialized.

Thanks, I just got my first systemd system set up, so I'll give this a
try.  I've also traced this error back to systemd:
"could not set SCHED_RR priority 99 err 1" which google seems to confirm,
along with the suggestion of using "ControlGroup=cpu:/" ... I don't know
if that's advise is still current.



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-11-05 16:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-03 15:27 [Cluster-devel] cluster4 dlm: startup notification for systemd Jacek Konieczny
2012-11-03 15:27 ` [Cluster-devel] [PATCH 1/2] --foreground option added to dlm_controld Jacek Konieczny
2012-11-03 15:27 ` [Cluster-devel] [PATCH 2/2] Startup notification by sd_notify() Jacek Konieczny
2012-11-05 16:20 ` [Cluster-devel] cluster4 dlm: startup notification for systemd David Teigland

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).