[SCM] Samba Shared Repository - branch master updated

Volker Lendecke vlendec at samba.org
Tue Aug 20 14:25:01 UTC 2024


The branch, master has been updated
       via  8edb1fd13c1 ctdb-tcp: Remove a use of ctdb_addr_to_str()
       via  afaf1511938 ctdb-tcp: Consolidate failure code
       via  f7aac2f7553 ctdb-tcp: Use already constructed node name
       via  02c9e7a63f0 ctdb-tcp: Use path_rundir_append() to construct lock_path
       via  17959ccb4b1 ctdb-ib: Remove a use of ctdb_set_error()
       via  b4336634145 ctdb-tcp: Factor out listening code to avoid repetition
       via  2c75bb86873 ctdb-tcp: Use talloc_strdup() instead of repeating logic
       via  f36f03172af ctdb-daemon: Remove a use of ctdb_errstr()
      from  a18c45046d2 libsmb: Remove cli_state->raw_status

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 8edb1fd13c1d1c02a25f5991d985c1c81b066760
Author: Martin Schwenke <mschwenke at ddn.com>
Date:   Mon Aug 19 08:48:43 2024 +1000

    ctdb-tcp: Remove a use of ctdb_addr_to_str()
    
    This one is in a rarely used error path, so call a function that
    talloc()s the string instead.
    
    Again, this will also print the port, which might be useful if we ever
    add the ability to also specify ports in the nodes list.
    
    Signed-off-by: Martin Schwenke <mschwenke at ddn.com>
    Reviewed-by: Volker Lendecke <vl at samba.org>
    
    Autobuild-User(master): Volker Lendecke <vl at samba.org>
    Autobuild-Date(master): Tue Aug 20 14:24:14 UTC 2024 on atb-devel-224

commit afaf151193816e354b10f76be2798d642e2d27fa
Author: Martin Schwenke <mschwenke at ddn.com>
Date:   Mon Aug 19 08:45:43 2024 +1000

    ctdb-tcp: Consolidate failure code
    
    Same thing several times, so change to common failure code.
    
    Signed-off-by: Martin Schwenke <mschwenke at ddn.com>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit f7aac2f7553bcdb609719529988aa8b33555dda9
Author: Martin Schwenke <mschwenke at ddn.com>
Date:   Sun Aug 18 20:49:30 2024 +1000

    ctdb-tcp: Use already constructed node name
    
    Node has been found, so use the pre-constructed name instead of
    calling ctdb_addr_to_str().
    
    This will also print the port, which might be useful if we ever add
    the ability to also specify ports in the nodes list.
    
    Signed-off-by: Martin Schwenke <mschwenke at ddn.com>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit 02c9e7a63f00998bb0dae2e8c6bd8e020e9408da
Author: Martin Schwenke <mschwenke at ddn.com>
Date:   Fri Aug 9 11:20:06 2024 +1000

    ctdb-tcp: Use path_rundir_append() to construct lock_path
    
    The current constant value doesn't respect CTDB_TEST_MODE/CTDB_BASE.
    Instead use the path module to allow automatic listening in test mode
    with local daemons.
    
    A single node can be tested with local daemons, using something like:
    
      $ tests/local_daemons.sh foo setup -n 1 -C "node address"
      $ grep "node address" foo/node.0/ctdb.conf
          # node address = 127.0.0.1
      $ tests/local_daemons.sh foo start all
      $ tests/local_daemons.sh foo print-log 0 | grep -i chose
      ... node.0 ctdbd[24546]: ctdb chose network address 127.0.0.1:4379
    
    The trick is that commenting out the node address in ctdb.conf means
    the chosen node address is the first one from the nodes file that
    allows bind/listen.  In this case it is the only line.
    
    The following ensures that automatic listening works for a node that
    isn't the first:
    
      $ cat >mynodes
      192.168.1.1
      127.0.0.1
      $ tests/local_daemons.sh foo setup -n 2 -N mynodes -C "node address"
      $ grep "node address" foo/node.1/ctdb.conf
          # node address = 127.0.0.1
      $ tests/local_daemons.sh foo start 1
      $ tests/local_daemons.sh foo print-log 1 | grep -i chose
      [...] node.1 ctdbd[22787]: ctdb chose network address 127.0.0.1:4379
    
    Note that the first address isn't local on this host, so will always
    fail.
    
    So, doing the above and starting both nodes yields...
    
      ...
      $ tests/local_daemons.sh foo start 1
      $ sleep 3; tests/local_daemons.sh foo start 0
      $ tests/local_daemons.sh foo print-log all | grep -i 'chose\|bind'
      [...] node.1 ctdbd[26351]: ctdb chose network address 127.0.0.1:4379
      [...] node.0 ctdbd[26438]: ctdb_tcp_listen_addr: Failed to bind() to socket - Address already in use (98)
      [...] node.0 ctdbd[26438]: Unable to bind to any node address - giving up
    
    ... as expected.
    
    It would be nice to add tests for this, but we don't really have
    infrastructure for that.  At least manual testing shows, for the
    obvious cases, the previous commits didn't break anything.  :-)
    
    Signed-off-by: Martin Schwenke <mschwenke at ddn.com>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit 17959ccb4b141612524f34270bc761d07fa1e54a
Author: Martin Schwenke <mschwenke at ddn.com>
Date:   Fri Jul 26 10:45:36 2024 +1000

    ctdb-ib: Remove a use of ctdb_set_error()
    
    Now the transport code is free of ctdb_set_error().
    
    Signed-off-by: Martin Schwenke <mschwenke at ddn.com>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit b43366341456e6848c401727e9d71633ad8aeef9
Author: Martin Schwenke <mschwenke at ddn.com>
Date:   Fri Jul 5 15:51:25 2024 +1000

    ctdb-tcp: Factor out listening code to avoid repetition
    
    Modernise debug and comments while here.
    
    Signed-off-by: Martin Schwenke <mschwenke at ddn.com>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit 2c75bb868733c631e1526e5555233ffdeb2ad8ff
Author: Martin Schwenke <mschwenke at ddn.com>
Date:   Fri Jul 5 15:52:51 2024 +1000

    ctdb-tcp: Use talloc_strdup() instead of repeating logic
    
    The node name is already constructed when the nodes file is loaded, so
    just copy the node name.
    
    Signed-off-by: Martin Schwenke <mschwenke at ddn.com>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit f36f03172af7c01d14bac2241a9514229df54574
Author: Martin Schwenke <mschwenke at ddn.com>
Date:   Fri Jul 26 10:49:16 2024 +1000

    ctdb-daemon: Remove a use of ctdb_errstr()
    
    Code to setup the transport is about to be cleaned up, including
    removing uses of ctdb_set_error(), so avoid logging a NULL pointer or
    some other old error.
    
    Signed-off-by: Martin Schwenke <mschwenke at ddn.com>
    Reviewed-by: Volker Lendecke <vl at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 ctdb/ib/ibw_ctdb.c     |   3 +-
 ctdb/server/ctdbd.c    |   2 +-
 ctdb/tcp/tcp_connect.c | 303 ++++++++++++++++++++++++-------------------------
 ctdb/wscript           |   2 +-
 4 files changed, 150 insertions(+), 160 deletions(-)


Changeset truncated at 500 lines:

diff --git a/ctdb/ib/ibw_ctdb.c b/ctdb/ib/ibw_ctdb.c
index 38314c32ebc..a509989d52b 100644
--- a/ctdb/ib/ibw_ctdb.c
+++ b/ctdb/ib/ibw_ctdb.c
@@ -44,8 +44,7 @@ int ctdb_ibw_get_address(struct ctdb_context *ctdb,
 	if (inet_pton(AF_INET, address, addr) <= 0) {
 		struct hostent *he = gethostbyname(address);
 		if (he == NULL || he->h_length > sizeof(*addr)) {
-			ctdb_set_error(ctdb, "invalid network address '%s'\n", 
-				       address);
+			DBG_ERR("invalid network address '%s'\n", address);
 			return -1;
 		}
 		memcpy(addr, he->h_addr, he->h_length);
diff --git a/ctdb/server/ctdbd.c b/ctdb/server/ctdbd.c
index 524c51b91e5..0ce958a4883 100644
--- a/ctdb/server/ctdbd.c
+++ b/ctdb/server/ctdbd.c
@@ -281,7 +281,7 @@ int main(int argc, const char *argv[])
 
 	ret = ctdb_set_transport(ctdb, ctdb_config.transport);
 	if (ret == -1) {
-		D_ERR("ctdb_set_transport failed - %s\n", ctdb_errstr(ctdb));
+		D_ERR("Failed to setup transport\n");
 		goto fail;
 	}
 
diff --git a/ctdb/tcp/tcp_connect.c b/ctdb/tcp/tcp_connect.c
index 6f5862af599..1ad3fa74d55 100644
--- a/ctdb/tcp/tcp_connect.c
+++ b/ctdb/tcp/tcp_connect.c
@@ -34,6 +34,9 @@
 #include "common/system.h"
 #include "common/common.h"
 #include "common/logging.h"
+#include "common/path.h"
+
+#include "protocol/protocol_util.h"
 
 #include "ctdb_tcp.h"
 
@@ -318,10 +321,15 @@ static void ctdb_listen_event(struct tevent_context *ev, struct tevent_fd *fde,
 
 	node = ctdb_ip_to_node(ctdb, &addr);
 	if (node == NULL) {
-		D_ERR("Refused connection from unknown node %s\n",
-		      ctdb_addr_to_str(&addr));
-		close(fd);
-		return;
+		char *t = ctdb_sock_addr_to_string(ctcp, &addr, true);
+		if (t == NULL) {
+			DBG_ERR("Refused connection from unparsable node\n");
+			goto failed;
+		}
+
+		D_ERR("Refused connection from unknown node %s\n", t);
+		talloc_free(t);
+		goto failed;
 	}
 
 	tnode = talloc_get_type_abort(node->transport_data,
@@ -329,24 +337,21 @@ static void ctdb_listen_event(struct tevent_context *ev, struct tevent_fd *fde,
 	if (tnode == NULL) {
 		/* This can't happen - see ctdb_tcp_initialise() */
 		DBG_ERR("INTERNAL ERROR setting up connection from node %s\n",
-			ctdb_addr_to_str(&addr));
-		close(fd);
-		return;
+			node->name);
+		goto failed;
 	}
 
 	if (tnode->in_queue != NULL) {
 		DBG_ERR("Incoming queue active, rejecting connection from %s\n",
-			ctdb_addr_to_str(&addr));
-		close(fd);
-		return;
+			node->name);
+		goto failed;
 	}
 
 	ret = set_blocking(fd, false);
 	if (ret != 0) {
 		DBG_ERR("Failed to set socket non-blocking (%s)\n",
 			strerror(errno));
-		close(fd);
-		return;
+		goto failed;
 	}
 
 	set_close_on_exec(fd);
@@ -370,11 +375,10 @@ static void ctdb_listen_event(struct tevent_context *ev, struct tevent_fd *fde,
 					   ctdb_tcp_read_cb,
 					   node,
 					   "ctdbd-%s",
-					   ctdb_addr_to_str(&addr));
+					   node->name);
 	if (tnode->in_queue == NULL) {
 		DBG_ERR("Failed to set up incoming queue\n");
-		close(fd);
-		return;
+		goto failed;
 	}
 
        /*
@@ -384,26 +388,112 @@ static void ctdb_listen_event(struct tevent_context *ev, struct tevent_fd *fde,
 	if (tnode->out_queue != NULL) {
 		node->ctdb->upcalls->node_connected(node);
 	}
+
+	return;
+
+failed:
+	close(fd);
  }
 
+static int ctdb_tcp_listen_addr(struct ctdb_context *ctdb,
+				ctdb_sock_addr *addr,
+				bool strict)
+{
+	struct ctdb_tcp *ctcp = talloc_get_type_abort(
+		ctdb->transport_data, struct ctdb_tcp);
+	ctdb_sock_addr sock;
+	int sock_size;
+	int one = 1;
+	struct tevent_fd *fde = NULL;
+	int ret;
+
+	sock = *addr;
+	ctcp->listen_fd = -1;
+
+	switch (sock.sa.sa_family) {
+	case AF_INET:
+		sock_size = sizeof(sock.ip);
+		break;
+	case AF_INET6:
+		sock_size = sizeof(sock.ip6);
+		break;
+	default:
+		DBG_ERR("Unknown family %u\n", sock.sa.sa_family);
+		goto failed;
+	}
+
+	ctcp->listen_fd = socket(sock.sa.sa_family, SOCK_STREAM, IPPROTO_TCP);
+	if (ctcp->listen_fd == -1) {
+		DBG_ERR("Socket failed - %s (%d)\n", strerror(errno), errno);
+		goto failed;
+	}
+
+	set_close_on_exec(ctcp->listen_fd);
+
+	ret = setsockopt(ctcp->listen_fd,
+			 SOL_SOCKET,
+			 SO_REUSEADDR,
+			 (char *)&one,
+			 sizeof(one));
+	if (ret == -1) {
+		DBG_WARNING("Failed to set REUSEADDR on fd - %s (%d)\n",
+			    strerror(errno),
+			    errno);
+	}
+
+	ret =bind(ctcp->listen_fd, (struct sockaddr * )&sock, sock_size);
+	if (ret != 0) {
+		if (strict || errno != EADDRNOTAVAIL) {
+			DBG_ERR("Failed to bind() to socket - %s (%d)\n",
+				strerror(errno),
+				errno);
+		} else {
+			DBG_DEBUG("Failed to bind() to socket - %s (%d)\n",
+				  strerror(errno),
+				  errno);
+		}
+		goto failed;
+	}
+
+	ret = listen(ctcp->listen_fd, 10);
+	if (ret == -1) {
+		DBG_ERR("Failed to listen() on socket - %s (%d)\n",
+			strerror(errno),
+			errno);
+		goto failed;
+	}
+
+	fde = tevent_add_fd(ctdb->ev,
+			    ctcp,
+			    ctcp->listen_fd,
+			    TEVENT_FD_READ,
+			    ctdb_listen_event,
+			    ctdb);
+	tevent_fd_set_auto_close(fde);
+
+	return 0;
+
+failed:
+	if (ctcp->listen_fd != -1) {
+		close(ctcp->listen_fd);
+		ctcp->listen_fd = -1;
+	}
+	return -1;
+}
 
 /*
   automatically find which address to listen on
 */
 static int ctdb_tcp_listen_automatic(struct ctdb_context *ctdb)
 {
-	struct ctdb_tcp *ctcp = talloc_get_type(ctdb->transport_data,
-						struct ctdb_tcp);
-        ctdb_sock_addr sock;
 	int lock_fd;
 	unsigned int i;
-	const char *lock_path = CTDB_RUNDIR "/.socket_lock";
+	char *lock_path = NULL;
 	struct flock lock;
-	int one = 1;
-	int sock_size;
-	struct tevent_fd *fde;
+	int ret;
 
-	/* If there are no nodes, then it won't be possible to find
+	/*
+	 * If there are no nodes, then it won't be possible to find
 	 * the first one.  Log a failure and short circuit the whole
 	 * process.
 	 */
@@ -412,13 +502,21 @@ static int ctdb_tcp_listen_automatic(struct ctdb_context *ctdb)
 		return -1;
 	}
 
-	/* in order to ensure that we don't get two nodes with the
-	   same address, we must make the bind() and listen() calls
-	   atomic. The SO_REUSEADDR setsockopt only prevents double
-	   binds if the first socket is in LISTEN state  */
+	/*
+	 * In order to ensure that we don't get two nodes with the
+	 * same address, we must make the bind() and listen() calls
+	 * atomic. The SO_REUSEADDR setsockopt only prevents double
+	 * binds if the first socket is in LISTEN state.
+	 */
+	lock_path = path_rundir_append(ctdb, ".socket_lock");
+	if (lock_path == NULL) {
+		DBG_ERR("Memory allocation error\n");
+		return -1;
+	}
 	lock_fd = open(lock_path, O_RDWR|O_CREAT, 0666);
 	if (lock_fd == -1) {
-		DEBUG(DEBUG_CRIT,("Unable to open %s\n", lock_path));
+		DBG_ERR("Unable to open %s\n", lock_path);
+		talloc_free(lock_path);
 		return -1;
 	}
 
@@ -429,102 +527,47 @@ static int ctdb_tcp_listen_automatic(struct ctdb_context *ctdb)
 	lock.l_pid = 0;
 
 	if (fcntl(lock_fd, F_SETLKW, &lock) != 0) {
-		DEBUG(DEBUG_CRIT,("Unable to lock %s\n", lock_path));
+		DBG_ERR("Unable to lock %s\n", lock_path);
 		close(lock_fd);
+		talloc_free(lock_path);
 		return -1;
 	}
+	talloc_free(lock_path);
 
 	for (i=0; i < ctdb->num_nodes; i++) {
 		if (ctdb->nodes[i]->flags & NODE_FLAGS_DELETED) {
 			continue;
 		}
-		sock = ctdb->nodes[i]->address;
 
-		switch (sock.sa.sa_family) {
-		case AF_INET:
-			sock_size = sizeof(sock.ip);
-			break;
-		case AF_INET6:
-			sock_size = sizeof(sock.ip6);
-			break;
-		default:
-			DEBUG(DEBUG_ERR, (__location__ " unknown family %u\n",
-				sock.sa.sa_family));
-			continue;
-		}
-
-		ctcp->listen_fd = socket(sock.sa.sa_family, SOCK_STREAM, IPPROTO_TCP);
-		if (ctcp->listen_fd == -1) {
-			ctdb_set_error(ctdb, "socket failed\n");
-			continue;
-		}
-
-		set_close_on_exec(ctcp->listen_fd);
-
-	        if (setsockopt(ctcp->listen_fd,SOL_SOCKET,SO_REUSEADDR,
-			       (char *)&one,sizeof(one)) == -1) {
-			DEBUG(DEBUG_WARNING, ("Failed to set REUSEADDR on fd - %s\n",
-					      strerror(errno)));
-		}
-
-		if (bind(ctcp->listen_fd, (struct sockaddr * )&sock, sock_size) == 0) {
+		ret = ctdb_tcp_listen_addr(ctdb,
+					   &ctdb->nodes[i]->address,
+					   false);
+		if (ret == 0) {
 			break;
 		}
-
-		if (errno == EADDRNOTAVAIL) {
-			DEBUG(DEBUG_DEBUG,(__location__ " Failed to bind() to socket. %s(%d)\n",
-					strerror(errno), errno));
-		} else {
-			DEBUG(DEBUG_ERR,(__location__ " Failed to bind() to socket. %s(%d)\n",
-					strerror(errno), errno));
-		}
-
-		close(ctcp->listen_fd);
-		ctcp->listen_fd = -1;
 	}
 
 	if (i == ctdb->num_nodes) {
-		DEBUG(DEBUG_CRIT,("Unable to bind to any of the node addresses - giving up\n"));
-		goto failed;
+		D_ERR("Unable to bind to any node address - giving up\n");
+		return -1;
 	}
+
 	ctdb->address = talloc_memdup(ctdb,
 				      &ctdb->nodes[i]->address,
 				      sizeof(ctdb_sock_addr));
 	if (ctdb->address == NULL) {
-		ctdb_set_error(ctdb, "Out of memory at %s:%d",
-			       __FILE__, __LINE__);
-		goto failed;
+		DBG_ERR("Memory allocation error\n");
+		return -1;
 	}
 
-	ctdb->name = talloc_asprintf(ctdb, "%s:%u",
-				     ctdb_addr_to_str(ctdb->address),
-				     ctdb_addr_to_port(ctdb->address));
+	ctdb->name = talloc_strdup(ctdb, ctdb->nodes[i]->name);
 	if (ctdb->name == NULL) {
-		ctdb_set_error(ctdb, "Out of memory at %s:%d",
-			       __FILE__, __LINE__);
-		goto failed;
-	}
-	DEBUG(DEBUG_INFO,("ctdb chose network address %s\n", ctdb->name));
-
-	if (listen(ctcp->listen_fd, 10) == -1) {
-		goto failed;
+		DBG_ERR("Memory allocation error\n");
+		return -1;
 	}
 
-	fde = tevent_add_fd(ctdb->ev, ctcp, ctcp->listen_fd, TEVENT_FD_READ,
-			    ctdb_listen_event, ctdb);
-	tevent_fd_set_auto_close(fde);
-
-	close(lock_fd);
-
+	D_INFO("ctdb chose network address %s\n", ctdb->name);
 	return 0;
-
-failed:
-	close(lock_fd);
-	if (ctcp->listen_fd != -1) {
-		close(ctcp->listen_fd);
-		ctcp->listen_fd = -1;
-	}
-	return -1;
 }
 
 
@@ -533,67 +576,15 @@ failed:
 */
 int ctdb_tcp_listen(struct ctdb_context *ctdb)
 {
-	struct ctdb_tcp *ctcp = talloc_get_type(ctdb->transport_data,
-						struct ctdb_tcp);
-        ctdb_sock_addr sock;
-	int sock_size;
-	int one = 1;
-	struct tevent_fd *fde;
+	int ret;
 
 	/* we can either auto-bind to the first available address, or we can
 	   use a specified address */
 	if (!ctdb->address) {
-		return ctdb_tcp_listen_automatic(ctdb);
-	}
-
-	sock = *ctdb->address;
-
-	switch (sock.sa.sa_family) {
-	case AF_INET:
-		sock_size = sizeof(sock.ip);
-		break;
-	case AF_INET6:
-		sock_size = sizeof(sock.ip6);
-		break;
-	default:
-		DEBUG(DEBUG_ERR, (__location__ " unknown family %u\n",
-			sock.sa.sa_family));
-		goto failed;
-	}
-
-	ctcp->listen_fd = socket(sock.sa.sa_family, SOCK_STREAM, IPPROTO_TCP);
-	if (ctcp->listen_fd == -1) {
-		ctdb_set_error(ctdb, "socket failed\n");
-		return -1;
+		ret = ctdb_tcp_listen_automatic(ctdb);
+		return ret;
 	}
 
-	set_close_on_exec(ctcp->listen_fd);
-
-        if (setsockopt(ctcp->listen_fd,SOL_SOCKET,SO_REUSEADDR,(char *)&one,sizeof(one)) == -1) {
-		DEBUG(DEBUG_WARNING, ("Failed to set REUSEADDR on fd - %s\n",
-				      strerror(errno)));
-	}
-
-	if (bind(ctcp->listen_fd, (struct sockaddr * )&sock, sock_size) != 0) {
-		DEBUG(DEBUG_ERR,(__location__ " Failed to bind() to socket. %s(%d)\n", strerror(errno), errno));
-		goto failed;
-	}
-
-	if (listen(ctcp->listen_fd, 10) == -1) {
-		goto failed;
-	}
-
-	fde = tevent_add_fd(ctdb->ev, ctcp, ctcp->listen_fd, TEVENT_FD_READ,
-			    ctdb_listen_event, ctdb);
-	tevent_fd_set_auto_close(fde);
-
-	return 0;
-
-failed:
-	if (ctcp->listen_fd != -1) {
-		close(ctcp->listen_fd);
-	}
-	ctcp->listen_fd = -1;
-	return -1;
+	ret = ctdb_tcp_listen_addr(ctdb, ctdb->address, true);
+	return ret;
 }
-
diff --git a/ctdb/wscript b/ctdb/wscript
index a211afbc77d..855989a2493 100644
--- a/ctdb/wscript
+++ b/ctdb/wscript
@@ -400,7 +400,7 @@ def build(bld):
                         source=bld.SUBDIR('tcp',
                                           'tcp_connect.c tcp_init.c tcp_io.c'),
                         includes='include',
-                        deps='replace tdb talloc tevent')
+                        deps='replace tdb talloc tevent ctdb-protocol-util')
 
     ib_deps = ''
     if bld.env.HAVE_INFINIBAND:


-- 
Samba Shared Repository



More information about the samba-cvs mailing list