From bd11b2b373e325f99f7f9a690166a3bad446113a Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Thu, 15 Dec 2016 10:17:25 +1100 Subject: [PATCH 1/8] 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 Reviewed-by: Amitay Isaacs (cherry picked from commit 25aad0df06038d0b595f09d947b9977dcc0ec8a8) --- ctdb/tests/simple/18_ctdb_reloadips.sh | 2 -- 1 file changed, 2 deletions(-) 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" -- 2.9.3 From cddb115d979ea9c1f3b7dfc46251037ccb5f674a Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Mon, 23 May 2016 10:35:10 +1000 Subject: [PATCH 2/8] 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 Reviewed-by: Amitay Isaacs (cherry picked from commit 362f066d617c8a186164db613537867329702ab7) --- ctdb/protocol/protocol_api.h | 10 +++++-- ctdb/protocol/protocol_util.c | 68 ++++++++++++++++++++++++++++++------------- 2 files changed, 55 insertions(+), 23 deletions(-) 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); } -- 2.9.3 From 84ee12274c2a0abae84817f91b475fc63a83fb7b Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Mon, 23 May 2016 11:53:26 +1000 Subject: [PATCH 3/8] ctdb-tests: Add unit test for protocol utilities BUG: https://bugzilla.samba.org/show_bug.cgi?id=12470 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit 3845ff6349421560bdcf9ba13467b2418205cd96) --- ctdb/tests/cunit/protocol_test_003.sh | 6 +++ ctdb/tests/src/protocol_util_test.c | 82 +++++++++++++++++++++++++++++++++++ ctdb/wscript | 6 +++ 3 files changed, 94 insertions(+) create mode 100755 ctdb/tests/cunit/protocol_test_003.sh create mode 100644 ctdb/tests/src/protocol_util_test.c diff --git a/ctdb/tests/cunit/protocol_test_003.sh b/ctdb/tests/cunit/protocol_test_003.sh new file mode 100755 index 0000000..012db90 --- /dev/null +++ b/ctdb/tests/cunit/protocol_test_003.sh @@ -0,0 +1,6 @@ +#!/bin/sh + +. "${TEST_SCRIPTS_DIR}/unit.sh" + +ok_null +unit_test protocol_util_test 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 . +*/ + +#include "replace.h" +#include "system/network.h" + +#include + +#include + +#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/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', -- 2.9.3 From 3b05a5de36b3635fbfab58f9794ca6b3849dc1c5 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Wed, 7 Dec 2016 09:23:02 +1100 Subject: [PATCH 4/8] 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 Reviewed-by: Amitay Isaacs (cherry picked from commit 7bcef2f0e2969551134e0d72f0956685eeec10a3) --- ctdb/tools/ctdb.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/ctdb/tools/ctdb.c b/ctdb/tools/ctdb.c index 9df6b4e..b2e8a47 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, @@ -1598,6 +1605,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; -- 2.9.3 From 3c0ad5fb7dd56f6b51cde01b1635b31890b27442 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Thu, 8 Dec 2016 11:29:13 +1100 Subject: [PATCH 5/8] 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 Reviewed-by: Amitay Isaacs (cherry picked from commit acaa4407ca3be9fb5637790079656f1eabf3848c) --- ctdb/tools/ctdb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ctdb/tools/ctdb.c b/ctdb/tools/ctdb.c index b2e8a47..42b52b4 100644 --- a/ctdb/tools/ctdb.c +++ b/ctdb/tools/ctdb.c @@ -1444,7 +1444,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) { @@ -1459,7 +1459,7 @@ 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); } } -- 2.9.3 From 6ff30f270ab98c33d38824e2a37306ed8a76bfe8 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Thu, 8 Dec 2016 11:35:23 +1100 Subject: [PATCH 6/8] 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 Reviewed-by: Amitay Isaacs (cherry picked from commit a6e5b6abe969e12cf26acf320f2c4bf40b377982) --- ctdb/tools/ctdb.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ctdb/tools/ctdb.c b/ctdb/tools/ctdb.c index 42b52b4..0579ca4 100644 --- a/ctdb/tools/ctdb.c +++ b/ctdb/tools/ctdb.c @@ -1436,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; jifaces->num; j++) { struct ctdb_iface *iface; @@ -1463,6 +1467,8 @@ static void print_ip(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb, } } + skip_ipinfo: + if (options.machinereadable == 1) { printf("%s%s%s%s%s%s\n", active ? active : "", options.sep, @@ -1620,6 +1626,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, -- 2.9.3 From 1b9d2918e56d8c85298cc8e270855b50a2326e87 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Thu, 8 Dec 2016 11:37:06 +1100 Subject: [PATCH 7/8] 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 Reviewed-by: Amitay Isaacs (cherry picked from commit 2514a9cd17fa9435792308aefdbebcc0a60a68f3) --- ctdb/tools/ctdb.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ctdb/tools/ctdb.c b/ctdb/tools/ctdb.c index 0579ca4..ac00bcb 100644 --- a/ctdb/tools/ctdb.c +++ b/ctdb/tools/ctdb.c @@ -1475,8 +1475,9 @@ static void print_ip(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb, 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 : ""); } } -- 2.9.3 From 2b4ac3a3d13eeda9e6a0054fe5de89bbb5870d54 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Tue, 13 Dec 2016 11:16:05 +1100 Subject: [PATCH 8/8] 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 Reviewed-by: Amitay Isaacs Autobuild-User(master): Amitay Isaacs Autobuild-Date(master): Fri Dec 16 12:32:02 CET 2016 on sn-devel-144 (cherry picked from commit cd20ced3fb9c71d38450f90224677f21a27d2548) --- ctdb/tools/ctdb.c | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/ctdb/tools/ctdb.c b/ctdb/tools/ctdb.c index ac00bcb..9d48889 100644 --- a/ctdb/tools/ctdb.c +++ b/ctdb/tools/ctdb.c @@ -1535,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; + } + } } } -- 2.9.3