PATCHSET: Avoid coredumps if ctdbd is not around

Michael Adam obnox at samba.org
Thu Oct 17 07:51:19 MDT 2013


Hi,

On 2013-10-11 at 12:45 +0200, Volker Lendecke wrote:
> 
> Attached find a patchset that avoids nasty corefiles and
> error messages if smbd and winbind are started in cluster
> mode without ctdbd being functional.
> 
> Please review & push!

Generally, the patches are good, but I have a few comments:

- The last patch does not compile when configuring without
  cluster support because ctdbd_probe is then not defined.
  I added a patch to always implement ctdbd_probe.
  This can be squashed with your patch.

- I don't like the code duplication in smbd and winbindd.
  I therefore I added a new module "util_cluster.c" with
  a function cluster_probe_ok() and use that in smbd and
  winbindd instead.

- The corresponding probe for nmbd is missing.
  I added a patch for that.

Attached find the patchset with review-tags and my patches
on top. Please consider, and feel free to reject/squash/push... :-)

Also found in my master-clustering-wip branch:
http://gitweb.samba.org/?p=obnox/samba/samba-obnox.git;a=shortlog;h=refs/heads/master-clustering-wip

Thanks - Michael

-------------- next part --------------
From d7693ad1d9e42265246bd5d138f583945b7305b1 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 31 Jan 2013 10:54:48 +0100
Subject: [PATCH 1/9] ctdbd_conn: Lift the cluster_fatal call in
 get_cluster_vnn

We have to report a proper error when ctdbd is not around

Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Michael Adam <obnox at samba.org>
---
 source3/lib/ctdbd_conn.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/source3/lib/ctdbd_conn.c b/source3/lib/ctdbd_conn.c
index f960541..70ce824 100644
--- a/source3/lib/ctdbd_conn.c
+++ b/source3/lib/ctdbd_conn.c
@@ -128,7 +128,8 @@ static NTSTATUS get_cluster_vnn(struct ctdbd_connection *conn, uint32_t *vnn)
 			       CTDB_CURRENT_NODE, CTDB_CONTROL_GET_PNN, 0, 0,
 			       tdb_null, NULL, NULL, &cstatus);
 	if (!NT_STATUS_IS_OK(status)) {
-		cluster_fatal("ctdbd_control failed\n");
+		DEBUG(1, ("ctdbd_control failed: %s\n", nt_errstr(status)));
+		return status;
 	}
 	*vnn = (uint32_t)cstatus;
 	return status;
@@ -529,6 +530,7 @@ static NTSTATUS ctdbd_init_connection(TALLOC_CTX *mem_ctx,
 
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(10, ("get_cluster_vnn failed: %s\n", nt_errstr(status)));
+		cluster_fatal("get_cluster_vnn failed");
 		goto fail;
 	}
 
-- 
1.7.9.5


From 6a5a63cfd920063561f48bfc5ff53de2f89e460b Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 31 Jan 2013 11:02:52 +0100
Subject: [PATCH 2/9] ctdbd_conn: Remove one call to cluster_fatal

This is during startup of a ctdb connection, thus it is not as important
as in other cases to immediately exit to free up resources

Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Michael Adam <obnox at samba.org>
---
 source3/lib/ctdbd_conn.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/source3/lib/ctdbd_conn.c b/source3/lib/ctdbd_conn.c
index 70ce824..d30e680 100644
--- a/source3/lib/ctdbd_conn.c
+++ b/source3/lib/ctdbd_conn.c
@@ -152,7 +152,8 @@ static bool ctdbd_working(struct ctdbd_connection *conn, uint32_t vnn)
 			       CTDB_CONTROL_GET_NODEMAP, 0, 0,
 			       tdb_null, talloc_tos(), &outdata, &cstatus);
 	if (!NT_STATUS_IS_OK(status)) {
-		cluster_fatal("ctdbd_control failed\n");
+		DEBUG(1, ("ctdbd_control failed: %s\n", nt_errstr(status)));
+		return false;
 	}
 	if ((cstatus != 0) || (outdata.dptr == NULL)) {
 		DEBUG(2, ("Received invalid ctdb data\n"));
-- 
1.7.9.5


From d7e9cc1fc7871b213640f860ebad209538b55ba5 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 31 Jan 2013 11:02:52 +0100
Subject: [PATCH 3/9] ctdbd_conn: Remove one call to cluster_fatal

This is during startup of a ctdb connection, thus it is not as important
as in other cases to immediately exit to free up resources

Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Michael Adam <obnox at samba.org>
---
 source3/lib/ctdbd_conn.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/source3/lib/ctdbd_conn.c b/source3/lib/ctdbd_conn.c
index d30e680..70833cb 100644
--- a/source3/lib/ctdbd_conn.c
+++ b/source3/lib/ctdbd_conn.c
@@ -531,7 +531,6 @@ static NTSTATUS ctdbd_init_connection(TALLOC_CTX *mem_ctx,
 
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(10, ("get_cluster_vnn failed: %s\n", nt_errstr(status)));
-		cluster_fatal("get_cluster_vnn failed");
 		goto fail;
 	}
 
-- 
1.7.9.5


From fc72080e7f3f4f7db1b87c79f16e46ee3a0cad6d Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 31 Jan 2013 11:15:09 +0100
Subject: [PATCH 4/9] smbd/winbindd: Do an early check if ctdbd is functional

This will avoid panic calls when smbd and winbind is started in cluster
mode before ctdb is functional. It still logs something sane at debug
level 0, but it does not panic and core anymore.

Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Michael Adam <obnox at samba.org>
---
 source3/include/ctdbd_conn.h |    1 +
 source3/lib/ctdbd_conn.c     |   19 +++++++++++++++++++
 source3/smbd/server.c        |   11 +++++++++++
 source3/winbindd/winbindd.c  |   12 ++++++++++++
 4 files changed, 43 insertions(+)

diff --git a/source3/include/ctdbd_conn.h b/source3/include/ctdbd_conn.h
index 64cb1d5..ce5c8ba 100644
--- a/source3/include/ctdbd_conn.h
+++ b/source3/include/ctdbd_conn.h
@@ -89,5 +89,6 @@ NTSTATUS ctdbd_control_local(struct ctdbd_connection *conn, uint32_t opcode,
 NTSTATUS ctdb_watch_us(struct ctdbd_connection *conn);
 NTSTATUS ctdb_unwatch(struct ctdbd_connection *conn);
 NTSTATUS register_with_ctdbd(struct ctdbd_connection *conn, uint64_t srvid);
+NTSTATUS ctdbd_probe(void);
 
 #endif /* _CTDBD_CONN_H */
diff --git a/source3/lib/ctdbd_conn.c b/source3/lib/ctdbd_conn.c
index 70833cb..2230c8f 100644
--- a/source3/lib/ctdbd_conn.c
+++ b/source3/lib/ctdbd_conn.c
@@ -1822,6 +1822,25 @@ NTSTATUS ctdb_unwatch(struct ctdbd_connection *conn)
 	return status;
 }
 
+NTSTATUS ctdbd_probe(void)
+{
+	/*
+	 * Do a very early check if ctdbd is around to avoid an abort and core
+	 * later
+	 */
+	struct ctdbd_connection *conn = NULL;
+	NTSTATUS status;
+
+	status = ctdbd_messaging_connection(talloc_tos(), &conn);
+
+	/*
+	 * We only care if we can connect.
+	 */
+	TALLOC_FREE(conn);
+
+	return status;
+}
+
 #else
 
 NTSTATUS ctdbd_messaging_send_blob(struct ctdbd_connection *conn,
diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index d3cd33e..3536f18 100644
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -1214,6 +1214,17 @@ extern void build_options(bool screen);
 		exit(1);
 	}
 
+	if (lp_clustering()) {
+		NTSTATUS status;
+
+		status = ctdbd_probe();
+		if (!NT_STATUS_IS_OK(status)) {
+			DEBUG(0, ("clustering=yes but ctdbd connect failed: "
+				  "%s\n", nt_errstr(status)));
+			exit(1);
+		}
+	}
+
 	/* Init the security context and global current_user */
 	init_sec_ctx();
 
diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
index 31280c2..b8b9c21 100644
--- a/source3/winbindd/winbindd.c
+++ b/source3/winbindd/winbindd.c
@@ -37,6 +37,7 @@
 #include "auth.h"
 #include "messages.h"
 #include "../lib/util/pidfile.h"
+#include "ctdbd_conn.h"
 
 #undef DBGC_CLASS
 #define DBGC_CLASS DBGC_WINBIND
@@ -1464,6 +1465,17 @@ int main(int argc, char **argv, char **envp)
 		exit(1);
 	}
 
+	if (lp_clustering()) {
+		NTSTATUS status;
+
+		status = ctdbd_probe();
+		if (!NT_STATUS_IS_OK(status)) {
+			DEBUG(0, ("clustering=yes but ctdbd connect failed: "
+				  "%s\n", nt_errstr(status)));
+			exit(1);
+		}
+	}
+
 	/* Initialise messaging system */
 
 	if (winbind_messaging_context() == NULL) {
-- 
1.7.9.5


From 05190829d6a5fe9568f4945037054ced18fbd773 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 17 Oct 2013 15:27:48 +0200
Subject: [PATCH 5/9] s3:ctdbd_conn: always implement ctdbd_probe()

Return STATUS_OK in the non-cluster build case.

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/lib/ctdbd_conn.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/source3/lib/ctdbd_conn.c b/source3/lib/ctdbd_conn.c
index 2230c8f..6ab4bbe 100644
--- a/source3/lib/ctdbd_conn.c
+++ b/source3/lib/ctdbd_conn.c
@@ -1843,6 +1843,11 @@ NTSTATUS ctdbd_probe(void)
 
 #else
 
+NTSTATUS ctdbd_probe(void)
+{
+	return NT_STATUS_OK;
+}
+
 NTSTATUS ctdbd_messaging_send_blob(struct ctdbd_connection *conn,
 				   uint32_t dst_vnn, uint64_t dst_srvid,
 				   const uint8_t *buf, size_t buflen)
-- 
1.7.9.5


From 7aebb80dc502ce3419a18401623a966cfd6c4f29 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 17 Oct 2013 15:10:11 +0200
Subject: [PATCH 6/9] s3: add cluster_probe_ok() in a new module util_cluster.

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/lib/util_cluster.c |   39 +++++++++++++++++++++++++++++++++++++++
 source3/lib/util_cluster.h |   27 +++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)
 create mode 100644 source3/lib/util_cluster.c
 create mode 100644 source3/lib/util_cluster.h

diff --git a/source3/lib/util_cluster.c b/source3/lib/util_cluster.c
new file mode 100644
index 0000000..ef79c8b
--- /dev/null
+++ b/source3/lib/util_cluster.c
@@ -0,0 +1,39 @@
+/*
+ *  Unix SMB/CIFS implementation.
+ *  cluster utility functions
+ *  Copyright (C) Volker Lendecke 2013
+ *  Copyright (C) Michael Adam 2013
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "includes.h"
+#include "ctdbd_conn.h"
+#include "util_cluster.h"
+
+bool cluster_probe_ok(void)
+{
+	if (lp_clustering()) {
+		NTSTATUS status;
+
+		status = ctdbd_probe();
+		if (!NT_STATUS_IS_OK(status)) {
+			DEBUG(0, ("clustering=yes but ctdbd connect failed: "
+				  "%s\n", nt_errstr(status)));
+			return false;
+		}
+	}
+
+	return true;
+}
diff --git a/source3/lib/util_cluster.h b/source3/lib/util_cluster.h
new file mode 100644
index 0000000..6d05987
--- /dev/null
+++ b/source3/lib/util_cluster.h
@@ -0,0 +1,27 @@
+/*
+ *  Unix SMB/CIFS implementation.
+ *  cluster utility functions
+ *  Copyright (C) Volker Lendecke 2013
+ *  Copyright (C) Michael Adam 2013
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+
+#ifndef __UTIL_CLUSTER_H__
+#define __UTIL_CLUSTER_H__
+
+bool cluster_probe_ok(void);
+
+#endif /* __UTIL_CLUSTER_H__ */
-- 
1.7.9.5


From f952cad11468a92d2af331e8c4b57177f021ba16 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 17 Oct 2013 15:15:51 +0200
Subject: [PATCH 7/9] s3:smbd: use new function cluster_probe_ok()

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/smbd/server.c |   12 +++---------
 source3/wscript_build |    1 +
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index 3536f18..8173a62 100644
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -31,6 +31,7 @@
 #include "secrets.h"
 #include "memcache.h"
 #include "ctdbd_conn.h"
+#include "util_cluster.h"
 #include "printing/queue_process.h"
 #include "rpc_server/rpc_service_setup.h"
 #include "rpc_server/rpc_config.h"
@@ -1214,15 +1215,8 @@ extern void build_options(bool screen);
 		exit(1);
 	}
 
-	if (lp_clustering()) {
-		NTSTATUS status;
-
-		status = ctdbd_probe();
-		if (!NT_STATUS_IS_OK(status)) {
-			DEBUG(0, ("clustering=yes but ctdbd connect failed: "
-				  "%s\n", nt_errstr(status)));
-			exit(1);
-		}
+	if (!cluster_probe_ok()) {
+		exit(1);
 	}
 
 	/* Init the security context and global current_user */
diff --git a/source3/wscript_build b/source3/wscript_build
index dd2e054..4803e9b 100755
--- a/source3/wscript_build
+++ b/source3/wscript_build
@@ -285,6 +285,7 @@ bld.SAMBA3_SUBSYSTEM('samba3core',
                    lib/ctdb_packet.c
                    lib/ctdbd_conn.c
                    lib/ctdb_conn.c
+                   lib/util_cluster.c
                    lib/msg_channel.c
                    lib/id_cache.c
                    lib/talloc_dict.c
-- 
1.7.9.5


From 7286bf22b55c671a22d4e4cb94ca6df4ed240607 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 17 Oct 2013 15:16:19 +0200
Subject: [PATCH 8/9] s3:winbindd: use cluster_probe_ok()

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/winbindd/winbindd.c |   13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
index b8b9c21..361d02c 100644
--- a/source3/winbindd/winbindd.c
+++ b/source3/winbindd/winbindd.c
@@ -37,7 +37,7 @@
 #include "auth.h"
 #include "messages.h"
 #include "../lib/util/pidfile.h"
-#include "ctdbd_conn.h"
+#include "util_cluster.h"
 
 #undef DBGC_CLASS
 #define DBGC_CLASS DBGC_WINBIND
@@ -1465,15 +1465,8 @@ int main(int argc, char **argv, char **envp)
 		exit(1);
 	}
 
-	if (lp_clustering()) {
-		NTSTATUS status;
-
-		status = ctdbd_probe();
-		if (!NT_STATUS_IS_OK(status)) {
-			DEBUG(0, ("clustering=yes but ctdbd connect failed: "
-				  "%s\n", nt_errstr(status)));
-			exit(1);
-		}
+	if (!cluster_probe_ok()) {
+		exit(1);
 	}
 
 	/* Initialise messaging system */
-- 
1.7.9.5


From c0aa105777c6fc3e6b9ed2294c5cb885415a0988 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 17 Oct 2013 15:19:41 +0200
Subject: [PATCH 9/9] s3:nmbd: do a very early cluster probe when starting
 nmbd.

Just as for smbd and winbindd

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/nmbd/nmbd.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/source3/nmbd/nmbd.c b/source3/nmbd/nmbd.c
index ec0e7d0..f31de08 100644
--- a/source3/nmbd/nmbd.c
+++ b/source3/nmbd/nmbd.c
@@ -26,6 +26,7 @@
 #include "serverid.h"
 #include "messages.h"
 #include "../lib/util/pidfile.h"
+#include "util_cluster.h"
 
 int ClientNMB       = -1;
 int ClientDGRAM     = -1;
@@ -917,6 +918,10 @@ static bool open_sockets(bool isdaemon, int port)
 		exit(1);
 	}
 
+	if (!cluster_probe_ok()) {
+		exit(1);
+	}
+
 	msg = messaging_init(NULL, server_event_context());
 	if (msg == NULL) {
 		return 1;
-- 
1.7.9.5

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 215 bytes
Desc: Digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20131017/b106d11f/attachment.pgp>


More information about the samba-technical mailing list