s390-tools/s390-tools-sles15-mon_tools-Improve-systemctl-start-error-handling.patch

Subject: [PATCH] [BZ 161563] mon_tools: Improve systemctl start error handling
From: Gerald Schaefer <gerald.schaefer@de.ibm.com>

Description:  cpuplugd, mon_tools: Improve systemctl start error handling
Symptom:      "systemctl start cpuplugd/mon_procd/mon_fsstatd" does not
              report any errors in case the startup fails.
Problem:      For type=simple, systemd forks/execs the process and if that
              is successful, it immediately returns. There is no way to
              find out if the initial startup fails.
Solution:     Use type=fork and run the process in the background. In this
              case systemd waits until the initial process returns.
              In addition, use PIDFile and ensure that the pid file is
              already available when the initial process returns. To
              achieve this, use startup synchronization via pipe.
Reproduction: Add invalid values to the config files and start the daemons
              with "systemctl start cpuplugd/mon_procd/mon_fsstatd".
Upstream-ID:  780133f825687bc931489aaf13959c793d2a4501
Problem-ID:   161563

Upstream-Description:

              mon_tools: Improve systemctl start error handling

              This fixes the same issue as in commit 82c8148983fc ("cpuplugd: Improve
              systemctl start error handling") for mon_tools (mon_procd and mon_fsstatd).

              Currently "systemctl start mon_procd/fsstatd" does not report any errors
              in case the startup fails.

              Example (with mon_procd):

               (change interval in /etc/sysconfig/mon_procd to an invalid value "abc")
               # systemctl start mon_procd

              The reason is that for type=simple systemd forks/execs mon_procd and if
              that is successful immediately returns. There is no way to find out if the
              initial startup fails.

              Fix this by using type=fork and running the process in the background. In
              this case systemd waits until the initial process returns.

              In addition use PIDFile and ensure that the pid file is already available
              when the initial process returns. To achieve this, use startup
              synchronization via pipe. Without that systemd would print the following
              warning:

              systemd[1]: mon_procd.service: PID file /var/run/mon_procd.pid not readable
                          (yet?) after start: No such file or directory

              With this patch, an early startup error like in the example above, is now
              reported correctly in "systemctl start":

               # systemctl start mon_procd
                 Job for mon_procd.service failed because the control process exited...
                 See "systemctl status mon_procd.service" and "journalctl -xe" for ...
               # journalctl -xe | grep mon_procd
                 mon_procd[3184]: Error: Invalid interval (needs to be greater than 0)

              Signed-off-by: Gerald Schaefer <gerald.schaefer@de.ibm.com>
              Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>


Signed-off-by: Gerald Schaefer <gerald.schaefer@de.ibm.com>
---
 mon_tools/mon_fsstatd.c        |   40 ++++++++++++++++++++++++++++++++--------
 mon_tools/mon_procd.c          |   40 ++++++++++++++++++++++++++++++++--------
 systemd/mon_fsstatd.service.in |    5 +++--
 systemd/mon_procd.service.in   |    5 +++--
 4 files changed, 70 insertions(+), 20 deletions(-)

--- a/mon_tools/mon_fsstatd.c
+++ b/mon_tools/mon_fsstatd.c
@@ -90,17 +90,18 @@ static void fsstatd_open_monwriter(void)
 /*
  * Store daemon's pid so it can be stopped
  */
-static void store_pid(void)
+static int store_pid(void)
 {
 	FILE *f = fopen(pid_file, "w");
 
 	if (!f) {
 		syslog(LOG_ERR, "cannot open pid file %s: %s", pid_file,
 		       strerror(errno));
-		exit(1);
+		return -1;
 	}
 	fprintf(f, "%d\n", getpid());
 	fclose(f);
+	return 0;
 }
 
 /*
@@ -244,41 +245,64 @@ static void fsstatd_write_ent(struct mnt
  */
 static void fsstatd_daemonize(void)
 {
+	int pipe_fds[2], startup_rc = 1;
 	pid_t pid;
 
+	if (pipe(pipe_fds) == -1) {
+		syslog(LOG_ERR, "pipe error: %s\n", strerror(errno));
+		exit(1);
+	}
+
 	/* Fork off the parent process */
 	pid = fork();
 	if (pid < 0) {
 		syslog(LOG_ERR, "fork error: %s\n", strerror(errno));
 		exit(1);
 	}
-	if (pid > 0)
-		exit(0);
+	if (pid > 0) {
+		/* Wait for startup return code from daemon */
+		if (read(pipe_fds[0], &startup_rc, sizeof(startup_rc)) == -1)
+			syslog(LOG_ERR, "pipe read error: %s\n", strerror(errno));
+		/* With startup_rc == 0, pid file was written at this point */
+		exit(startup_rc);
+	}
 
 	/* Change the file mode mask */
 	umask(0);
 
-	/* Store daemon pid */
-	store_pid();
 	/* Catch SIGINT and SIGTERM to clean up pid file on exit */
 	fsstatd_handle_signals();
 
 	/* Create a new SID for the child process */
 	if (setsid() < 0) {
 		syslog(LOG_ERR, "setsid error: %s\n",  strerror(errno));
-		exit(1);
+		goto notify_parent;
 	}
 
 	/* Change the current working directory */
 	if (chdir("/") < 0) {
 		syslog(LOG_ERR, "chdir error: %s\n",  strerror(errno));
-		exit(1);
+		goto notify_parent;
 	}
 
 	/* Close out the standard file descriptors */
 	close(STDIN_FILENO);
 	close(STDOUT_FILENO);
 	close(STDERR_FILENO);
+
+	/* Store daemon pid */
+	if (store_pid() < 0)
+		goto notify_parent;
+	startup_rc = 0;
+
+notify_parent:
+	/* Inform waiting parent about startup return code */
+	if (write(pipe_fds[1], &startup_rc, sizeof(startup_rc)) == -1) {
+		syslog(LOG_ERR, "pipe write error: %s\n", strerror(errno));
+		exit(1);
+	}
+	if (startup_rc != 0)
+		exit(startup_rc);
 }
 
 static int fsstatd_do_work(void)
--- a/mon_tools/mon_procd.c
+++ b/mon_tools/mon_procd.c
@@ -109,17 +109,18 @@ static void procd_open_monwriter(void)
 /*
  * Store daemon's pid so it can be stopped
  */
-static void store_pid(void)
+static int store_pid(void)
 {
 	FILE *f = fopen(pid_file, "w");
 
 	if (!f) {
 		syslog(LOG_ERR, "cannot open pid file %s: %s", pid_file,
 		       strerror(errno));
-		exit(1);
+		return -1;
 	}
 	fprintf(f, "%d\n", getpid());
 	fclose(f);
+	return 0;
 }
 
 /*
@@ -897,41 +898,64 @@ static void read_tasks(void)
  */
 static void procd_daemonize(void)
 {
+	int pipe_fds[2], startup_rc = 1;
 	pid_t pid;
 
+	if (pipe(pipe_fds) == -1) {
+		syslog(LOG_ERR, "pipe error: %s\n", strerror(errno));
+		exit(1);
+	}
+
 	/* Fork off the parent process */
 	pid = fork();
 	if (pid < 0) {
 		syslog(LOG_ERR, "fork error: %s\n", strerror(errno));
 		exit(1);
 	}
-	if (pid > 0)
-		exit(0);
+	if (pid > 0) {
+		/* Wait for startup return code from daemon */
+		if (read(pipe_fds[0], &startup_rc, sizeof(startup_rc)) == -1)
+			syslog(LOG_ERR, "pipe read error: %s\n", strerror(errno));
+		/* With startup_rc == 0, pid file was written at this point */
+		exit(startup_rc);
+	}
 
 	/* Change the file mode mask */
 	umask(0);
 
-	/* Store daemon pid */
-	store_pid();
 	/* Catch SIGINT and SIGTERM to clean up pid file on exit */
 	procd_handle_signals();
 
 	/* Create a new SID for the child process */
 	if (setsid() < 0) {
 		syslog(LOG_ERR, "setsid error: %s\n",  strerror(errno));
-		exit(1);
+		goto notify_parent;
 	}
 
 	/* Change the current working directory */
 	if (chdir("/") < 0) {
 		syslog(LOG_ERR, "chdir error: %s\n",  strerror(errno));
-		exit(1);
+		goto notify_parent;
 	}
 
 	/* Close out the standard file descriptors */
 	close(STDIN_FILENO);
 	close(STDOUT_FILENO);
 	close(STDERR_FILENO);
+
+	/* Store daemon pid */
+	if (store_pid() < 0)
+		goto notify_parent;
+	startup_rc = 0;
+
+notify_parent:
+	/* Inform waiting parent about startup return code */
+	if (write(pipe_fds[1], &startup_rc, sizeof(startup_rc)) == -1) {
+		syslog(LOG_ERR, "pipe write error: %s\n", strerror(errno));
+		exit(1);
+	}
+	if (startup_rc != 0)
+		exit(startup_rc);
 }
 
 static int procd_do_work(void)
--- a/systemd/mon_fsstatd.service.in
+++ b/systemd/mon_fsstatd.service.in
@@ -30,10 +30,11 @@ EnvironmentFile=/etc/sysconfig/mon_fssta
 
 ExecStartPre=-/sbin/modprobe monwriter
 ExecStartPre=/sbin/udevadm settle --timeout=10
-ExecStart=@usrsbin_path@/mon_fsstatd -a -i $FSSTAT_INTERVAL
+ExecStart=@usrsbin_path@/mon_fsstatd -i $FSSTAT_INTERVAL
 ExecReload=/bin/kill -HUP $MAINPID
 KillMode=process
-Type=simple
+Type=forking
+PIDFile=/var/run/mon_fsstatd.pid
 
 [Install]
 WantedBy=multi-user.target
--- a/systemd/mon_procd.service.in
+++ b/systemd/mon_procd.service.in
@@ -30,10 +30,11 @@ EnvironmentFile=/etc/sysconfig/mon_procd
 
 ExecStartPre=-/sbin/modprobe monwriter
 ExecStartPre=/sbin/udevadm settle --timeout=10
-ExecStart=@usrsbin_path@/mon_procd -a -i $PROC_INTERVAL
+ExecStart=@usrsbin_path@/mon_procd -i $PROC_INTERVAL
 ExecReload=/bin/kill -HUP $MAINPID
 KillMode=process
-Type=simple
+Type=forking
+PIDFile=/var/run/mon_procd.pid
 
 [Install]
 WantedBy=multi-user.target