From fbb5ac8404525a0828504cd744992dfcb7c8e216 Mon Sep 17 00:00:00 2001
From: Amitay Isaacs <amitay@gmail.com>
Date: Fri, 10 Nov 2017 12:10:05 +1100
Subject: [PATCH] ctdb-common: Return status from sock_daemon
 startup()/reconfigure()

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Martin Schwenke <martin@meltin.net>
---
 ctdb/common/sock_daemon.c                | 21 +++++++++++--
 ctdb/common/sock_daemon.h                | 14 ++++++---
 ctdb/tests/cunit/sock_daemon_test_001.sh | 14 +++++++++
 ctdb/tests/src/sock_daemon_test.c        | 54 +++++++++++++++++++++++++-------
 4 files changed, 85 insertions(+), 18 deletions(-)

diff --git a/ctdb/common/sock_daemon.c b/ctdb/common/sock_daemon.c
index a41a5d5a7ea..bb241e0ebd8 100644
--- a/ctdb/common/sock_daemon.c
+++ b/ctdb/common/sock_daemon.c
@@ -648,7 +648,16 @@ static void sock_daemon_run_started(struct tevent_req *subreq)
 	D_NOTICE("daemon started, pid=%u\n", getpid());
 
 	if (sockd->funcs != NULL && sockd->funcs->startup != NULL) {
-		sockd->funcs->startup(sockd->private_data);
+		int ret;
+
+		ret = sockd->funcs->startup(sockd->private_data);
+		if (ret != 0) {
+			D_ERR("startup failed, ret=%d\n", ret);
+			tevent_req_error(req, EIO);
+			return;
+		}
+
+		D_NOTICE("startup completed successfully\n");
 	}
 }
 
@@ -680,7 +689,15 @@ static void sock_daemon_run_reconfigure(struct tevent_req *req)
 	struct sock_daemon_context *sockd = state->sockd;
 
 	if (sockd->funcs != NULL && sockd->funcs->reconfigure != NULL) {
-		sockd->funcs->reconfigure(sockd->private_data);
+		int ret;
+
+		ret = sockd->funcs->reconfigure(sockd->private_data);
+		if (ret != 0) {
+			D_ERR("reconfigure failed, ret=%d\n", ret);
+			return;
+		}
+
+		D_NOTICE("reconfigure completed successfully\n");
 	}
 }
 
diff --git a/ctdb/common/sock_daemon.h b/ctdb/common/sock_daemon.h
index 18210287771..190a4ef172f 100644
--- a/ctdb/common/sock_daemon.h
+++ b/ctdb/common/sock_daemon.h
@@ -48,8 +48,14 @@ struct sock_client_context;
  * @brief The callback routines called during daemon life cycle
  *
  * startup() is called when the daemon starts running
- *            either via sock_daemon_run() or via sock_daemon_run_send()
- * reconfigure() is called when process receives SIGUSR1 or SIGHUP
+ *	either via sock_daemon_run() or via sock_daemon_run_send()
+ *	startup() should return 0 for success, non-zero value on failure
+ *	On failure, sock_daemon_run() will return error.
+ *
+ * reconfigure() is called when the daemon receives SIGUSR1 or SIGHUP
+ *	reconfigure() should return 0 for success, non-zero value on failure
+ *	On failure, sock_daemon_run() will continue to run.
+ *
  * shutdown() is called when process receives SIGINT or SIGTERM or
  *             when wait computation has finished
  *
@@ -60,8 +66,8 @@ struct sock_client_context;
  * If wait_send() returns req, then when req is over, daemon will shutdown.
  */
 struct sock_daemon_funcs {
-	void (*startup)(void *private_data);
-	void (*reconfigure)(void *private_data);
+	int (*startup)(void *private_data);
+	int (*reconfigure)(void *private_data);
 	void (*shutdown)(void *private_data);
 
 	struct tevent_req * (*wait_send)(TALLOC_CTX *mem_ctx,
diff --git a/ctdb/tests/cunit/sock_daemon_test_001.sh b/ctdb/tests/cunit/sock_daemon_test_001.sh
index 9da484a7616..cbb5a7e292a 100755
--- a/ctdb/tests/cunit/sock_daemon_test_001.sh
+++ b/ctdb/tests/cunit/sock_daemon_test_001.sh
@@ -23,6 +23,8 @@ result_filter ()
 
 
 ok <<EOF
+test1[PID]: daemon started, pid=PID
+test1[PID]: startup failed, ret=1
 test1[PID]: listening on $sockpath
 test1[PID]: Shutting down
 EOF
@@ -31,8 +33,11 @@ unit_test sock_daemon_test "$pidfile" "$sockpath" 1
 ok <<EOF
 test2[PID]: listening on $sockpath
 test2[PID]: daemon started, pid=PID
+test2[PID]: startup completed successfully
 test2[PID]: Received signal 1
+test2[PID]: reconfigure failed, ret=1
 test2[PID]: Received signal 10
+test2[PID]: reconfigure completed successfully
 test2[PID]: Received signal 15
 test2[PID]: Shutting down
 EOF
@@ -55,6 +60,7 @@ unit_test sock_daemon_test "$pidfile" "$sockpath" 4
 ok <<EOF
 test5[PID]: listening on $sockpath
 test5[PID]: daemon started, pid=PID
+test5[PID]: startup completed successfully
 test5[PID]: Received signal 15
 test5[PID]: Shutting down
 EOF
@@ -63,12 +69,14 @@ unit_test sock_daemon_test "$pidfile" "$sockpath" 5
 ok <<EOF
 test6[PID]: listening on $sockpath
 test6[PID]: daemon started, pid=PID
+test6[PID]: startup completed successfully
 test6[PID]: Shutting down
 EOF
 unit_test sock_daemon_test "$pidfile" "$sockpath" 6
 
 ok <<EOF
 test7[PID]: daemon started, pid=PID
+test7[PID]: startup completed successfully
 test7[PID]: Received signal 15
 test7[PID]: Shutting down
 EOF
@@ -76,9 +84,11 @@ unit_test sock_daemon_test "$pidfile" "$sockpath" 7
 
 ok <<EOF
 test8[PID]: daemon started, pid=PID
+test8[PID]: startup completed successfully
 test8[PID]: Received signal 15
 test8[PID]: Shutting down
 test8[PID]: daemon started, pid=PID
+test8[PID]: startup completed successfully
 test8[PID]: Received signal 15
 test8[PID]: Shutting down
 EOF
@@ -86,9 +96,11 @@ unit_test sock_daemon_test "$pidfile" "$sockpath" 8
 
 ok <<EOF
 test9[PID]: daemon started, pid=PID
+test9[PID]: startup completed successfully
 test9[PID]: Received signal 15
 test9[PID]: Shutting down
 test9[PID]: daemon started, pid=PID
+test9[PID]: startup completed successfully
 test9[PID]: Received signal 15
 test9[PID]: Shutting down
 EOF
@@ -97,8 +109,10 @@ unit_test sock_daemon_test "$pidfile" "$sockpath" 9
 ok <<EOF
 test10[PID]: listening on $sockpath
 test10[PID]: daemon started, pid=PID
+test10[PID]: startup completed successfully
 test10[PID]: listening on $sockpath
 test10[PID]: daemon started, pid=PID
+test10[PID]: startup completed successfully
 test10[PID]: Received signal 15
 test10[PID]: Shutting down
 EOF
diff --git a/ctdb/tests/src/sock_daemon_test.c b/ctdb/tests/src/sock_daemon_test.c
index 95045d175ff..82f4f783a1e 100644
--- a/ctdb/tests/src/sock_daemon_test.c
+++ b/ctdb/tests/src/sock_daemon_test.c
@@ -63,10 +63,10 @@ static bool dummy_wait_recv(struct tevent_req *req, int *perr)
 	return true;
 }
 
-static struct sock_daemon_funcs test1_funcs = {
-	.wait_send = dummy_wait_send,
-	.wait_recv = dummy_wait_recv,
-};
+static int test1_startup_fail(void *private_data)
+{
+	return 1;
+}
 
 static struct tevent_req *dummy_read_send(TALLOC_CTX *mem_ctx,
 					  struct tevent_context *ev,
@@ -101,26 +101,45 @@ static void test1(TALLOC_CTX *mem_ctx, const char *pidfile,
 {
 	struct tevent_context *ev;
 	struct sock_daemon_context *sockd;
+	struct sock_daemon_funcs test1_funcs;
 	struct stat st;
 	int ret;
 
+	ev = tevent_context_init(mem_ctx);
+	assert(ev != NULL);
+
+	test1_funcs = (struct sock_daemon_funcs){
+		.startup = test1_startup_fail,
+	};
+
 	ret = sock_daemon_setup(mem_ctx, "test1", "file:", "NOTICE",
-				&test1_funcs, discard_const(sockpath), &sockd);
+				&test1_funcs, NULL, &sockd);
 	assert(ret == 0);
 	assert(sockd != NULL);
 
 	ret = stat(pidfile, &st);
 	assert(ret == -1);
 
+	ret = sock_daemon_run(ev, sockd, NULL, false, false, -1);
+	assert(ret == EIO);
+	talloc_free(sockd);
+
+	test1_funcs = (struct sock_daemon_funcs){
+		.wait_send = dummy_wait_send,
+		.wait_recv = dummy_wait_recv,
+	};
+
+	ret = sock_daemon_setup(mem_ctx, "test1", "file:", "NOTICE",
+				&test1_funcs, discard_const(sockpath), &sockd);
+	assert(ret == 0);
+	assert(sockd != NULL);
+
 	ret = sock_daemon_add_unix(sockd, sockpath, &dummy_socket_funcs, NULL);
 	assert(ret == 0);
 
 	ret = stat(sockpath, &st);
 	assert(ret == -1);
 
-	ev = tevent_context_init(mem_ctx);
-	assert(ev != NULL);
-
 	ret = sock_daemon_run(ev, sockd, NULL, false, false, -1);
 	assert(ret == 0);
 
@@ -134,7 +153,7 @@ static void test1(TALLOC_CTX *mem_ctx, const char *pidfile,
  * exit code
  */
 
-static void test2_startup(void *private_data)
+static int test2_startup(void *private_data)
 {
 	int fd = *(int *)private_data;
 	int ret = 1;
@@ -142,16 +161,25 @@ static void test2_startup(void *private_data)
 
 	nwritten = write(fd, &ret, sizeof(ret));
 	assert(nwritten == sizeof(ret));
+	return 0;
 }
 
-static void test2_reconfigure(void *private_data)
+static int test2_reconfigure(void *private_data)
 {
+	static bool first_time = true;
 	int fd = *(int *)private_data;
 	int ret = 2;
 	ssize_t nwritten;
 
 	nwritten = write(fd, &ret, sizeof(ret));
 	assert(nwritten == sizeof(ret));
+
+	if (first_time) {
+		first_time = false;
+		return 1;
+	}
+
+	return 0;
 }
 
 static void test2_shutdown(void *private_data)
@@ -649,7 +677,7 @@ static struct sock_socket_funcs test5_client_funcs = {
 	.read_recv = test5_read_recv,
 };
 
-static void test5_startup(void *private_data)
+static int test5_startup(void *private_data)
 {
 	int fd = *(int *)private_data;
 	int ret = 1;
@@ -658,6 +686,7 @@ static void test5_startup(void *private_data)
 	nwritten = write(fd, &ret, sizeof(ret));
 	assert(nwritten == sizeof(ret));
 	close(fd);
+	return 0;
 }
 
 static struct sock_daemon_funcs test5_funcs = {
@@ -881,7 +910,7 @@ static struct sock_socket_funcs test6_client_funcs = {
 	.read_recv = test6_read_recv,
 };
 
-static void test6_startup(void *private_data)
+static int test6_startup(void *private_data)
 {
 	struct test6_server_state *server_state =
 		(struct test6_server_state *)private_data;
@@ -892,6 +921,7 @@ static void test6_startup(void *private_data)
 	assert(nwritten == sizeof(ret));
 	close(server_state->fd);
 	server_state->fd = -1;
+	return 0;
 }
 
 struct test6_wait_state {
-- 
2.11.4.GIT