[SCM] Samba Shared Repository - branch v4-5-test updated

Karolin Seeger kseeger at samba.org
Tue Dec 20 15:27:04 UTC 2016


The branch, v4-5-test has been updated
       via  fbd6779 ctdb-tools: Don't trust non-hosting nodes in "ctdb ip all"
       via  cfec216 ctdb-tools: Print PNN as int in "ctdb ip -v"
       via  90edef1 ctdb-tools: Skip GET_PUBLIC_IP_INFO for unassigned addresses
       via  25ba90d ctdb-tools: Fix memory corruption in "ctdb ip -v"
       via  e04ef4c ctdb-tools: Fix sort order of "ctdb ip" output
       via  94c3b81 ctdb-tests: Add unit test for protocol utilities
       via  edf4817 ctdb-protocol: Add generalised socket address comparison
       via  3427e37 ctdb-tests: Fix "ctdb reloadips" simple test
      from  9f718c5 VERSION: Bump version up to 4.5.4

https://git.samba.org/?p=samba.git;a=shortlog;h=v4-5-test


- Log -----------------------------------------------------------------
commit fbd67799a0e43fb0ef572bdca505dede29e6f878
Author: Martin Schwenke <martin at meltin.net>
Date:   Tue Dec 13 11:16:05 2016 +1100

    ctdb-tools: Don't trust non-hosting nodes in "ctdb ip all"
    
    Redundant RELEASE_IPs gives nodes a preview of where an IP address
    will move to.  However, if the associated TAKEOVER_IP fails then the
    node will actually be unhosted.
    
    This is similar to commit 77a29b37334b9df62b755b6f538fb975e105e1ff.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12470
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>
    
    Autobuild-User(master): Amitay Isaacs <amitay at samba.org>
    Autobuild-Date(master): Fri Dec 16 12:32:02 CET 2016 on sn-devel-144
    
    (cherry picked from commit cd20ced3fb9c71d38450f90224677f21a27d2548)
    
    Autobuild-User(v4-5-test): Karolin Seeger <kseeger at samba.org>
    Autobuild-Date(v4-5-test): Tue Dec 20 16:26:27 CET 2016 on sn-devel-144

commit cfec21631bab5fce6ddd3e38961c2731a0479490
Author: Martin Schwenke <martin at meltin.net>
Date:   Thu Dec 8 11:37:06 2016 +1100

    ctdb-tools: Print PNN as int in "ctdb ip -v"
    
    Otherwise it prints 4294967295 for the PNN.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12470
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>
    (cherry picked from commit 2514a9cd17fa9435792308aefdbebcc0a60a68f3)

commit 90edef133d925794b749c4d4cad2a2f46e350536
Author: Martin Schwenke <martin at meltin.net>
Date:   Thu Dec 8 11:35:23 2016 +1100

    ctdb-tools: Skip GET_PUBLIC_IP_INFO for unassigned addresses
    
    The GET_PUBLIC_IP_INFO control fails for unassigned addresses because
    PNN is CTDB_UNKNOWN_PNN.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12470
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>
    (cherry picked from commit a6e5b6abe969e12cf26acf320f2c4bf40b377982)

commit 25ba90d1e406627fb2654d5a56482a387cab1c6e
Author: Martin Schwenke <martin at meltin.net>
Date:   Thu Dec 8 11:29:13 2016 +1100

    ctdb-tools: Fix memory corruption in "ctdb ip -v"
    
    First argument to talloc_asprintf_append() is the string being
    appended to, not a talloc context.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12470
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>
    (cherry picked from commit acaa4407ca3be9fb5637790079656f1eabf3848c)

commit e04ef4c04979d19aa152187d23d58c7e9fcceec2
Author: Martin Schwenke <martin at meltin.net>
Date:   Wed Dec 7 09:23:02 2016 +1100

    ctdb-tools: Fix sort order of "ctdb ip" output
    
    The new hash-table-based method of merging the IP information does not
    sort, whereas the RB-tree method implicitly sorted.  This probably
    only really matters for the "all" case, but sort regardless to ensure
    consistent output format.
    
    Sorting has to be done here instead of when printing to ensure
    consistency between ip[] and ipinfo[].
    
    No longer reverse the sort order.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12470
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>
    (cherry picked from commit 7bcef2f0e2969551134e0d72f0956685eeec10a3)

commit 94c3b8114efeffc8cd4f3ac56a3d7c12fe1c405f
Author: Martin Schwenke <martin at meltin.net>
Date:   Mon May 23 11:53:26 2016 +1000

    ctdb-tests: Add unit test for protocol utilities
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12470
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>
    (cherry picked from commit 3845ff6349421560bdcf9ba13467b2418205cd96)

commit edf48177b96cd04f212118a047041d0b3596673d
Author: Martin Schwenke <martin at meltin.net>
Date:   Mon May 23 10:35:10 2016 +1000

    ctdb-protocol: Add generalised socket address comparison
    
    Add new function ctdb_sock_addr_cmp(), which returns a 3-way result
    useful for qsort(3).  Reimplent ctdb_sock_addr_same() using this.
    
    In the process, make arguments const so that ctdb_sock_addr_cmp() can
    be used with qsort().
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12470
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>
    (cherry picked from commit 362f066d617c8a186164db613537867329702ab7)

commit 3427e37115899ba1d765de64b0ad0b8e500f0f52
Author: Martin Schwenke <martin at meltin.net>
Date:   Thu Dec 15 10:17:25 2016 +1100

    ctdb-tests: Fix "ctdb reloadips" simple test
    
    The name of the addresses file to modify is based on the original
    selection of a test node at the top of the test.  Repeating the
    selection a test node can result in a mismatch between the new test
    node and the addresses file.  This occurs on local daemons, because
    the addresses file name has the original node number in it but the
    test is being performed on the the newly selected node number.
    
    For some reason this test has only occasionally failed.  An upcoming
    commit that stops the output of "ctdb ip" from being reversed causes
    this test to fail (nearly?) every time.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12470
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>
    (cherry picked from commit 25aad0df06038d0b595f09d947b9977dcc0ec8a8)

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

Summary of changes:
 ctdb/protocol/protocol_api.h                       | 10 ++-
 ctdb/protocol/protocol_util.c                      | 68 ++++++++++++------
 .../{db_hash_test_001.sh => protocol_test_003.sh}  |  3 +-
 ctdb/tests/simple/18_ctdb_reloadips.sh             |  2 -
 ctdb/tests/src/protocol_util_test.c                | 82 ++++++++++++++++++++++
 ctdb/tools/ctdb.c                                  | 66 ++++++++++++++---
 ctdb/wscript                                       |  6 ++
 7 files changed, 199 insertions(+), 38 deletions(-)
 copy ctdb/tests/cunit/{db_hash_test_001.sh => protocol_test_003.sh} (64%)
 create mode 100644 ctdb/tests/src/protocol_util_test.c


Changeset truncated at 500 lines:

diff --git a/ctdb/protocol/protocol_api.h b/ctdb/protocol/protocol_api.h
index d5d00da..7a8357a 100644
--- a/ctdb/protocol/protocol_api.h
+++ b/ctdb/protocol/protocol_api.h
@@ -658,7 +658,13 @@ const char *ctdb_event_to_string(enum ctdb_event event);
 enum ctdb_event ctdb_event_from_string(const char *event_str);
 
 const char *ctdb_sock_addr_to_string(TALLOC_CTX *mem_ctx, ctdb_sock_addr *addr);
-bool ctdb_sock_addr_same_ip(ctdb_sock_addr *addr1, ctdb_sock_addr *addr2);
-bool ctdb_sock_addr_same(ctdb_sock_addr *addr1, ctdb_sock_addr *addr2);
+int ctdb_sock_addr_cmp_ip(const ctdb_sock_addr *addr1,
+			  const ctdb_sock_addr *addr2);
+int ctdb_sock_addr_cmp(const ctdb_sock_addr *addr1,
+		       const ctdb_sock_addr *addr2);
+bool ctdb_sock_addr_same_ip(const ctdb_sock_addr *addr1,
+			    const ctdb_sock_addr *addr2);
+bool ctdb_sock_addr_same(const ctdb_sock_addr *addr1,
+			 const ctdb_sock_addr *addr2);
 
 #endif /* __CTDB_PROTOCOL_API_H__ */
diff --git a/ctdb/protocol/protocol_util.c b/ctdb/protocol/protocol_util.c
index b91c652..0e1bf28 100644
--- a/ctdb/protocol/protocol_util.c
+++ b/ctdb/protocol/protocol_util.c
@@ -141,55 +141,81 @@ const char *ctdb_sock_addr_to_string(TALLOC_CTX *mem_ctx, ctdb_sock_addr *addr)
 	return cip;
 }
 
-bool ctdb_sock_addr_same_ip(ctdb_sock_addr *addr1, ctdb_sock_addr *addr2)
+int ctdb_sock_addr_cmp_ip(const ctdb_sock_addr *addr1,
+			  const ctdb_sock_addr *addr2)
 {
-	if (addr1->sa.sa_family != addr2->sa.sa_family) {
-		return false;
+	int ret = 0;
+
+	/* This is somewhat arbitrary.  However, when used for sorting
+	 * it just needs to be consistent.
+	 */
+	if (addr1->sa.sa_family < addr2->sa.sa_family) {
+		return -1;
+	}
+	if (addr1->sa.sa_family > addr2->sa.sa_family) {
+		return 1;
 	}
 
 	switch (addr1->sa.sa_family) {
 	case AF_INET:
-		if (addr1->ip.sin_addr.s_addr != addr2->ip.sin_addr.s_addr) {
-			return false;
-		}
+		ret = memcmp(&addr1->ip.sin_addr.s_addr,
+			     &addr2->ip.sin_addr.s_addr, 4);
 		break;
 
 	case AF_INET6:
-		if (memcmp(addr1->ip6.sin6_addr.s6_addr,
-			   addr2->ip6.sin6_addr.s6_addr, 16) != 0) {
-			return false;
-		}
+		ret = memcmp(addr1->ip6.sin6_addr.s6_addr,
+			     addr2->ip6.sin6_addr.s6_addr, 16);
 		break;
 
 	default:
-		return false;
+		ret = -1;
 	}
 
-	return true;
+	return ret;
 }
 
-bool ctdb_sock_addr_same(ctdb_sock_addr *addr1, ctdb_sock_addr *addr2)
+int ctdb_sock_addr_cmp(const ctdb_sock_addr *addr1,
+		       const ctdb_sock_addr *addr2)
 {
-	if (! ctdb_sock_addr_same_ip(addr1, addr2)) {
-		return false;
+	int ret = 0;
+
+	ret = ctdb_sock_addr_cmp_ip(addr1, addr2);
+	if (ret != 0) {
+		return ret;
 	}
 
 	switch (addr1->sa.sa_family) {
 	case AF_INET:
-		if (addr1->ip.sin_port != addr2->ip.sin_port) {
-			return false;
+		if (addr1->ip.sin_port < addr2->ip.sin_port) {
+			ret = -1;
+		} else if (addr1->ip.sin_port > addr2->ip.sin_port) {
+			ret = 1;
 		}
 		break;
 
 	case AF_INET6:
-		if (addr1->ip6.sin6_port != addr2->ip6.sin6_port) {
-			return false;
+		if (addr1->ip6.sin6_port < addr2->ip6.sin6_port) {
+			ret = -1;
+		} else if (addr1->ip6.sin6_port > addr2->ip6.sin6_port) {
+			ret = 1;
 		}
 		break;
 
 	default:
-		return false;
+		ret = -1;
 	}
 
-	return true;
+	return ret;
+}
+
+bool ctdb_sock_addr_same_ip(const ctdb_sock_addr *addr1,
+			    const ctdb_sock_addr *addr2)
+{
+	return (ctdb_sock_addr_cmp_ip(addr1, addr2) == 0);
+}
+
+bool ctdb_sock_addr_same(const ctdb_sock_addr *addr1,
+			 const ctdb_sock_addr *addr2)
+{
+	return (ctdb_sock_addr_cmp(addr1, addr2) == 0);
 }
diff --git a/ctdb/tests/cunit/db_hash_test_001.sh b/ctdb/tests/cunit/protocol_test_003.sh
similarity index 64%
copy from ctdb/tests/cunit/db_hash_test_001.sh
copy to ctdb/tests/cunit/protocol_test_003.sh
index 76c38fe..012db90 100755
--- a/ctdb/tests/cunit/db_hash_test_001.sh
+++ b/ctdb/tests/cunit/protocol_test_003.sh
@@ -3,5 +3,4 @@
 . "${TEST_SCRIPTS_DIR}/unit.sh"
 
 ok_null
-
-unit_test db_hash_test
+unit_test protocol_util_test
diff --git a/ctdb/tests/simple/18_ctdb_reloadips.sh b/ctdb/tests/simple/18_ctdb_reloadips.sh
index 760e476..b68ecfa 100755
--- a/ctdb/tests/simple/18_ctdb_reloadips.sh
+++ b/ctdb/tests/simple/18_ctdb_reloadips.sh
@@ -81,8 +81,6 @@ EOF
 
 try_command_on_node any $CTDB sync
 
-select_test_node_and_ips
-
 echo "Removing IP $test_ip from node $test_node"
 
 try_command_on_node $test_node "mv $addresses $backup && grep -v '^${test_ip}/' $backup >$addresses"
diff --git a/ctdb/tests/src/protocol_util_test.c b/ctdb/tests/src/protocol_util_test.c
new file mode 100644
index 0000000..752c437
--- /dev/null
+++ b/ctdb/tests/src/protocol_util_test.c
@@ -0,0 +1,82 @@
+/*
+   protocol utilities tests
+
+   Copyright (C) Martin Schwenke  2016
+
+   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 "replace.h"
+#include "system/network.h"
+
+#include <assert.h>
+
+#include <talloc.h>
+
+#include "protocol/protocol.h"
+#include "protocol/protocol_api.h"
+#include "common/system_util.c"
+
+/* Test parsing of IPs, conversion to string */
+static void test_sock_addr_to_string(const char *ip)
+{
+	ctdb_sock_addr sa;
+	const char *s;
+
+	assert(parse_ip(ip, NULL, 0, &sa));
+	s = ctdb_sock_addr_to_string(NULL, &sa);
+	assert(strcmp(ip, s) == 0);
+	talloc_free(discard_const(s));
+}
+
+static void test_sock_addr_cmp(const char *ip1, const char *ip2, int res)
+{
+	ctdb_sock_addr sa1, sa2;
+	int ret;
+
+	assert(parse_ip(ip1, NULL, 0, &sa1));
+	assert(parse_ip(ip2, NULL, 0, &sa2));
+	ret = ctdb_sock_addr_cmp(&sa1, &sa2);
+	if (ret < 0) {
+		ret = -1;
+	} else if (ret > 0) {
+		ret = 1;
+	}
+
+	assert(ret == res);
+}
+
+int main(int argc, char *argv[])
+{
+	test_sock_addr_to_string("0.0.0.0");
+	test_sock_addr_to_string("127.0.0.1");
+	test_sock_addr_to_string("::1");
+	test_sock_addr_to_string("192.168.2.1");
+	test_sock_addr_to_string("fe80::6af7:28ff:fefa:d136");
+
+	test_sock_addr_cmp("127.0.0.1", "127.0.0.1" , 0);
+	test_sock_addr_cmp("127.0.0.1", "127.0.0.2" , -1);
+	test_sock_addr_cmp("127.0.0.2", "127.0.0.1" , 1);
+	test_sock_addr_cmp("127.0.1.2", "127.0.2.1" , -1);
+	test_sock_addr_cmp("127.0.2.1", "127.0.1.2" , 1);
+	test_sock_addr_cmp("fe80::6af7:28ff:fefa:d136", "127.0.1.2" , 1);
+	test_sock_addr_cmp("fe80::6af7:28ff:fefa:d136",
+			   "fe80::6af7:28ff:fefa:d136" , 0);
+	test_sock_addr_cmp("fe80::6af7:28ff:fefa:d136",
+			   "fe80::6af7:28ff:fefa:d137" , -1);
+	test_sock_addr_cmp("fe80::6af7:28ff:fefa:d136",
+			   "fe80:0000:0000:0000:6af7:28ff:fefa:d136" , 0);
+
+	return 0;
+}
diff --git a/ctdb/tools/ctdb.c b/ctdb/tools/ctdb.c
index 9df6b4e..9d48889 100644
--- a/ctdb/tools/ctdb.c
+++ b/ctdb/tools/ctdb.c
@@ -1374,6 +1374,14 @@ static int control_stats(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
 	return 0;
 }
 
+static int ctdb_public_ip_cmp(const void *a, const void *b)
+{
+	const struct ctdb_public_ip *ip_a = a;
+	const struct ctdb_public_ip *ip_b = b;
+
+	return ctdb_sock_addr_cmp(&ip_a->addr, &ip_b->addr);
+}
+
 static void print_ip(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
 		     struct ctdb_public_ip_list *ips,
 		     struct ctdb_public_ip_info **ipinfo,
@@ -1402,8 +1410,7 @@ static void print_ip(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
 		}
 	}
 
-	/* IPs are reverse sorted */
-	for (i=ips->num-1; i>=0; i--) {
+	for (i = 0; i < ips->num; i++) {
 
 		if (options.machinereadable == 1) {
 			printf("%s%s%s%d%s", options.sep,
@@ -1429,6 +1436,10 @@ static void print_ip(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
 		avail = NULL;
 		active = NULL;
 
+		if (ipinfo[i] == NULL) {
+			goto skip_ipinfo;
+		}
+
 		for (j=0; j<ipinfo[i]->ifaces->num; j++) {
 			struct ctdb_iface *iface;
 
@@ -1437,7 +1448,7 @@ static void print_ip(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
 				conf = talloc_strdup(mem_ctx, iface->name);
 			} else {
 				conf = talloc_asprintf_append(
-						mem_ctx, ",%s", iface->name);
+						conf, ",%s", iface->name);
 			}
 
 			if (ipinfo[i]->active_idx == j) {
@@ -1452,18 +1463,21 @@ static void print_ip(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
 				avail = talloc_strdup(mem_ctx, iface->name);
 			} else {
 				avail = talloc_asprintf_append(
-						mem_ctx, ",%s", iface->name);
+						avail, ",%s", iface->name);
 			}
 		}
 
+	skip_ipinfo:
+
 		if (options.machinereadable == 1) {
 			printf("%s%s%s%s%s%s\n",
 			       active ? active : "", options.sep,
 			       avail ? avail : "", options.sep,
 			       conf ? conf : "", options.sep);
 		} else {
-			printf(" node[%u] active[%s] available[%s] configured[%s]\n",
-			       ips->ip[i].pnn, active ? active : "",
+			printf(" node[%d] active[%s] available[%s]"
+			       " configured[%s]\n",
+			       (int)ips->ip[i].pnn, active ? active : "",
 			       avail ? avail : "", conf ? conf : "");
 		}
 	}
@@ -1521,11 +1535,34 @@ static int get_all_public_ips(struct ctdb_context *ctdb, TALLOC_CTX *mem_ctx,
 			ip.pnn = ips->ip[j].pnn;
 			ip.addr = ips->ip[j].addr;
 
-			ret = db_hash_add(ipdb, (uint8_t *)&ip.addr,
-					  sizeof(ip.addr),
-					  (uint8_t *)&ip, sizeof(ip));
-			if (ret != 0) {
-				goto failed;
+			if (pnn_list[i] == ip.pnn) {
+				/* Node claims IP is hosted on it, so
+				 * save that information
+				 */
+				ret = db_hash_add(ipdb, (uint8_t *)&ip.addr,
+						  sizeof(ip.addr),
+						  (uint8_t *)&ip, sizeof(ip));
+				if (ret != 0) {
+					goto failed;
+				}
+			} else {
+				/* Node thinks IP is hosted elsewhere,
+				 * so overwrite with CTDB_UNKNOWN_PNN
+				 * if there's no existing entry
+				 */
+				ret = db_hash_exists(ipdb, (uint8_t *)&ip.addr,
+						     sizeof(ip.addr));
+				if (ret == ENOENT) {
+					ip.pnn = CTDB_UNKNOWN_PNN;
+					ret = db_hash_add(ipdb,
+							  (uint8_t *)&ip.addr,
+							  sizeof(ip.addr),
+							  (uint8_t *)&ip,
+							  sizeof(ip));
+					if (ret != 0) {
+						goto failed;
+					}
+				}
 			}
 		}
 
@@ -1598,6 +1635,9 @@ static int control_ip(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
 		return ret;
 	}
 
+	qsort(ips->ip, ips->num, sizeof(struct ctdb_public_ip),
+	      ctdb_public_ip_cmp);
+
 	ipinfo = talloc_array(mem_ctx, struct ctdb_public_ip_info *, ips->num);
 	if (ipinfo == NULL) {
 		return 1;
@@ -1610,6 +1650,10 @@ static int control_ip(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
 		} else {
 			pnn = ctdb->cmd_pnn;
 		}
+		if (pnn == CTDB_UNKNOWN_PNN) {
+			ipinfo[i] = NULL;
+			continue;
+		}
 		ret = ctdb_ctrl_get_public_ip_info(mem_ctx, ctdb->ev,
 						   ctdb->client, pnn,
 						   TIMEOUT(), &ips->ip[i].addr,
diff --git a/ctdb/wscript b/ctdb/wscript
index c775cb5..f61d087 100644
--- a/ctdb/wscript
+++ b/ctdb/wscript
@@ -697,6 +697,12 @@ def build(bld):
                         includes='include',
                         deps='replace popt talloc tevent tdb')
 
+    bld.SAMBA_BINARY('protocol_util_test',
+                     source='tests/src/protocol_util_test.c',
+                     deps='talloc ctdb-protocol samba-util',
+                     install_path='${CTDB_TEST_LIBEXECDIR}')
+
+    # Test binaries
     ctdb_tests = [
         'g_lock_loop',
         'message_ring',


-- 
Samba Shared Repository



More information about the samba-cvs mailing list