[SCM] Samba Shared Repository - branch v4-6-test updated
Karolin Seeger
kseeger at samba.org
Wed Sep 13 16:49:03 UTC 2017
The branch, v4-6-test has been updated
via c258b78 ctdb-daemon: GET_DB_SEQNUM should read database conditionally
via 9b93e44 ctdb-daemon: Add a function to check if db access is allowed
via 0ce69f5 ctdb-tests: Fix ctdb test binary name in path testing
via bae034a ctdb-tests: Wait up to 30 seconds for process to be registered in ctdbd
via 7b4d686 ctdb-tests: Fix ctdb process-exist tests
via 037483d ctdb-tests: Add a dummy ctdb client for testing
via 919e8b8 ctdb-tests: Fix the implementation of process-exists in fake daemon
via e9896f6 ctdb-daemon: Fix implementation of process_exists control
from dc47600 messaging: Avoid a socket leak after fork
https://git.samba.org/?p=samba.git;a=shortlog;h=v4-6-test
- Log -----------------------------------------------------------------
commit c258b783e013f7832873bbe9404aeaa60aed9e96
Author: Amitay Isaacs <amitay at gmail.com>
Date: Thu Sep 7 17:21:03 2017 +1000
ctdb-daemon: GET_DB_SEQNUM should read database conditionally
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13021
Once the recovery starts and databases are frozen, then all the record
access is postponed till the recovery is complete except reading the
database sequence number. Database access for reading sequence number
is done via a control which does not check if the databases are frozen
or not.
If the database is frozen and if the freeze transaction is not started
(this can happen when a node is inactive, or during recovery when the
database is frozen but the transaction has not yet started), then trying
to read sequence number will cause ctdb daemon to deadlock.
Before reading the sequence number, check if the database access is
allowed.
Signed-off-by: Amitay Isaacs <amitay at gmail.com>
Reviewed-by: Martin Schwenke <martin at meltin.net>
(cherry picked from commit f57d379446c551bca5906247c622e857c77089b0)
Autobuild-User(v4-6-test): Karolin Seeger <kseeger at samba.org>
Autobuild-Date(v4-6-test): Wed Sep 13 18:48:58 CEST 2017 on sn-devel-144
commit 9b93e4476dbebea6c6fe4991e8e4445254858ab6
Author: Amitay Isaacs <amitay at gmail.com>
Date: Thu Sep 7 17:18:18 2017 +1000
ctdb-daemon: Add a function to check if db access is allowed
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13021
Signed-off-by: Amitay Isaacs <amitay at gmail.com>
Reviewed-by: Martin Schwenke <martin at meltin.net>
(cherry picked from commit 5d2f2677de65a0fd6683bb759d80ebced604fa6b)
commit 0ce69f53a8e22e27d42a3d1ea6660f0eabbad9e2
Author: Amitay Isaacs <amitay at gmail.com>
Date: Tue Sep 5 13:52:47 2017 +1000
ctdb-tests: Fix ctdb test binary name in path testing
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13012
Signed-off-by: Amitay Isaacs <amitay at gmail.com>
Reviewed-by: Martin Schwenke <martin at meltin.net>
(cherry picked from commit 96aef2371c6c1e0c6bd13874a71583eb9609959b)
commit bae034aaa3b6049957e18b302bc27d9b8d0a4b26
Author: Martin Schwenke <martin at meltin.net>
Date: Tue Sep 12 11:51:19 2017 +1000
ctdb-tests: Wait up to 30 seconds for process to be registered in ctdbd
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13012
This avoids a potential race where the client is not properly
registered before "ctdb process-exists" is called.
Signed-off-by: Martin Schwenke <martin at meltin.net>
Reviewed-by: Amitay Isaacs <amitay at gmail.com>
(cherry picked from commit ff75f0836aef56476ec45a3bc8f3ca22c118e3a4)
commit 7b4d686b5ae6537a25636569d6331435c7090eba
Author: Amitay Isaacs <amitay at gmail.com>
Date: Fri Aug 25 16:55:34 2017 +1000
ctdb-tests: Fix ctdb process-exist tests
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13012
Signed-off-by: Amitay Isaacs <amitay at gmail.com>
Reviewed-by: Martin Schwenke <martin at meltin.net>
Autobuild-User(master): Amitay Isaacs <amitay at samba.org>
Autobuild-Date(master): Sat Sep 9 14:44:57 CEST 2017 on sn-devel-144
(cherry picked from commit 87f7d32a906799e83cb9b023978e689a630de017)
commit 037483d19e1388821ba18b999382fd653f5a02b6
Author: Amitay Isaacs <amitay at gmail.com>
Date: Wed Aug 30 13:05:32 2017 +1000
ctdb-tests: Add a dummy ctdb client for testing
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13012
Signed-off-by: Amitay Isaacs <amitay at gmail.com>
Reviewed-by: Martin Schwenke <martin at meltin.net>
(cherry picked from commit 3067db5b50162fdae288aaad8e75beb924fc9494)
commit 919e8b8890cf11275c9ee27851984291c793e0a8
Author: Amitay Isaacs <amitay at gmail.com>
Date: Fri Aug 25 16:54:47 2017 +1000
ctdb-tests: Fix the implementation of process-exists in fake daemon
Keep track of clients and their pids.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13012
Signed-off-by: Amitay Isaacs <amitay at gmail.com>
Reviewed-by: Martin Schwenke <martin at meltin.net>
(cherry picked from commit 7dec80a7c042d83f9d48c75a8717c3d1b59b1fbf)
commit e9896f6cf57c59608f2f3b3d79150c1a34452fac
Author: Amitay Isaacs <amitay at gmail.com>
Date: Fri Aug 25 15:00:59 2017 +1000
ctdb-daemon: Fix implementation of process_exists control
Only check processes that are CTDB clients.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13012
Signed-off-by: Amitay Isaacs <amitay at gmail.com>
Reviewed-by: Martin Schwenke <martin at meltin.net>
(cherry picked from commit d0a20baf43834c7290dfd8f256d9521724202f0c)
-----------------------------------------------------------------------
Summary of changes:
ctdb/include/ctdb_private.h | 1 +
ctdb/server/ctdb_daemon.c | 16 +--
ctdb/server/ctdb_freeze.c | 18 ++++
ctdb/server/ctdb_persistent.c | 5 +
ctdb/tests/scripts/test_wrap | 2 +-
ctdb/tests/simple/07_ctdb_process_exists.sh | 34 ++++---
ctdb/tests/src/dummy_client.c | 148 ++++++++++++++++++++++++++++
ctdb/tests/src/fake_ctdbd.c | 81 ++++++++++++++-
ctdb/tests/tool/ctdb.process-exists.001.sh | 12 ++-
ctdb/wscript | 3 +-
10 files changed, 292 insertions(+), 28 deletions(-)
create mode 100644 ctdb/tests/src/dummy_client.c
Changeset truncated at 500 lines:
diff --git a/ctdb/include/ctdb_private.h b/ctdb/include/ctdb_private.h
index d81ed56..491dd78 100644
--- a/ctdb/include/ctdb_private.h
+++ b/ctdb/include/ctdb_private.h
@@ -626,6 +626,7 @@ int32_t ctdb_control_wipe_database(struct ctdb_context *ctdb, TDB_DATA indata);
bool ctdb_db_frozen(struct ctdb_db_context *ctdb_db);
bool ctdb_db_all_frozen(struct ctdb_context *ctdb);
+bool ctdb_db_allow_access(struct ctdb_db_context *ctdb_db);
/* from server/ctdb_keepalive.c */
diff --git a/ctdb/server/ctdb_daemon.c b/ctdb/server/ctdb_daemon.c
index d0d86a0..122d884 100644
--- a/ctdb/server/ctdb_daemon.c
+++ b/ctdb/server/ctdb_daemon.c
@@ -1800,12 +1800,16 @@ int32_t ctdb_control_process_exists(struct ctdb_context *ctdb, pid_t pid)
{
struct ctdb_client *client;
- if (ctdb->nodes[ctdb->pnn]->flags & (NODE_FLAGS_BANNED|NODE_FLAGS_STOPPED)) {
- client = ctdb_find_client_by_pid(ctdb, pid);
- if (client != NULL) {
- DEBUG(DEBUG_NOTICE,(__location__ " Killing client with pid:%d on banned/stopped node\n", (int)pid));
- talloc_free(client);
- }
+ client = ctdb_find_client_by_pid(ctdb, pid);
+ if (client == NULL) {
+ return -1;
+ }
+
+ if (ctdb->nodes[ctdb->pnn]->flags & NODE_FLAGS_INACTIVE) {
+ DEBUG(DEBUG_NOTICE,
+ ("Killing client with pid:%d on banned/stopped node\n",
+ (int)pid));
+ talloc_free(client);
return -1;
}
diff --git a/ctdb/server/ctdb_freeze.c b/ctdb/server/ctdb_freeze.c
index 2666013..e2b12c7 100644
--- a/ctdb/server/ctdb_freeze.c
+++ b/ctdb/server/ctdb_freeze.c
@@ -874,3 +874,21 @@ bool ctdb_db_all_frozen(struct ctdb_context *ctdb)
}
return true;
}
+
+bool ctdb_db_allow_access(struct ctdb_db_context *ctdb_db)
+{
+ if (ctdb_db->freeze_mode == CTDB_FREEZE_NONE) {
+ /* If database is not frozen, then allow access. */
+ return true;
+ } else if (ctdb_db->freeze_transaction_started) {
+ /* If database is frozen, allow access only if the
+ * transaction is started. This is required during
+ * recovery.
+ *
+ * If a node is inactive, then transaction is not started.
+ */
+ return true;
+ }
+
+ return false;
+}
diff --git a/ctdb/server/ctdb_persistent.c b/ctdb/server/ctdb_persistent.c
index 1811ae8..fc28655 100644
--- a/ctdb/server/ctdb_persistent.c
+++ b/ctdb/server/ctdb_persistent.c
@@ -344,6 +344,11 @@ static int32_t ctdb_get_db_seqnum(struct ctdb_context *ctdb,
goto done;
}
+ if (! ctdb_db_allow_access(ctdb_db)) {
+ ret = -1;
+ goto done;
+ }
+
key.dptr = (uint8_t *)discard_const(keyname);
key.dsize = strlen(keyname) + 1;
diff --git a/ctdb/tests/scripts/test_wrap b/ctdb/tests/scripts/test_wrap
index 176310e..3db3180 100755
--- a/ctdb/tests/scripts/test_wrap
+++ b/ctdb/tests/scripts/test_wrap
@@ -10,7 +10,7 @@ TEST_SCRIPTS_DIR=$(dirname $0)
# We need the test binaries (i.e. tests/bin/) to be in $PATH. If they
# aren't already in $PATH then we know that tests/bin/ sits alongside
# tests/scripts/.
-f="ctdb_bench"
+f="fetch_ring"
if [ ! $(which $f >/dev/null 2>&1) ] ; then
d=$(dirname "$TEST_SCRIPTS_DIR")/bin
[ -x "$d/$f" ] && PATH="$d:$PATH"
diff --git a/ctdb/tests/simple/07_ctdb_process_exists.sh b/ctdb/tests/simple/07_ctdb_process_exists.sh
index b7492a8..f24e93a 100755
--- a/ctdb/tests/simple/07_ctdb_process_exists.sh
+++ b/ctdb/tests/simple/07_ctdb_process_exists.sh
@@ -15,11 +15,10 @@ Prerequisites:
Steps:
1. Verify that the status on all of the ctdb nodes is 'OK'.
-2. On one of the cluster nodes, get the PID of an existing process
- (using ps wax).
+2. On one of the cluster nodes, get the PID of a ctdb client.
3. Run 'ctdb process-exists <pid>' on the node and verify that the
correct output is shown.
-4. Run 'ctdb process-exists <pid>' with a pid of a non-existent
+4. Run 'ctdb process-exists <pid>' with a pid of ctdb daemon
process and verify that the correct output is shown.
Expected results:
@@ -38,15 +37,25 @@ cluster_is_healthy
test_node=1
-# Create a background process on $test_node that will last for 60 seconds.
+# Execute a ctdb client on $test_node that will last for 60 seconds.
# It should still be there when we check.
-try_command_on_node $test_node 'sleep 60 >/dev/null 2>&1 & echo $!'
-pid="$out"
+try_command_on_node -v $test_node \
+ "$CTDB_TEST_WRAPPER exec dummy_client >/dev/null 2>&1 & echo \$!"
+client_pid="$out"
-echo "Checking for PID $pid on node $test_node"
-# set -e is good, but avoid it here
+cleanup ()
+{
+ if [ -n "$client_pid" ] ; then
+ onnode $test_node kill -9 "$client_pid"
+ fi
+}
+
+ctdb_test_exit_hook_add cleanup
+
+echo "Waiting until PID $client_pid is registered on node $test_node"
status=0
-try_command_on_node $test_node "$CTDB process-exists ${pid}" || status=$?
+wait_until 30 try_command_on_node $test_node \
+ "$CTDB process-exists ${client_pid}" || status=$?
echo "$out"
if [ $status -eq 0 ] ; then
@@ -56,10 +65,9 @@ else
testfailures=1
fi
-# Now just echo the PID of the shell from the onnode process on node
-# 2. This PID will disappear and PIDs shouldn't roll around fast
-# enough to trick the test... but there is a chance that will happen!
-try_command_on_node $test_node 'echo $$'
+# Now just echo the PID of the ctdb daemon on test node.
+# This is not a ctdb client and process-exists should return error.
+try_command_on_node $test_node "ctdb getpid"
pid="$out"
echo "Checking for PID $pid on node $test_node"
diff --git a/ctdb/tests/src/dummy_client.c b/ctdb/tests/src/dummy_client.c
new file mode 100644
index 0000000..6af41f3
--- /dev/null
+++ b/ctdb/tests/src/dummy_client.c
@@ -0,0 +1,148 @@
+/*
+ Dummy CTDB client for testing
+
+ Copyright (C) Amitay Isaacs 2017
+
+ 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 <popt.h>
+#include <talloc.h>
+#include <tevent.h>
+
+#include "common/logging.h"
+
+#include "client/client.h"
+
+static struct {
+ const char *sockpath;
+ const char *debuglevel;
+ int timelimit;
+ const char *srvidstr;
+} options;
+
+static struct poptOption cmdline_options[] = {
+ POPT_AUTOHELP
+ { "socket", 's', POPT_ARG_STRING, &options.sockpath, 0,
+ "Unix domain socket path", "filename" },
+ { "debug", 'd', POPT_ARG_STRING, &options.debuglevel, 0,
+ "debug level", "ERR|WARNING|NOTICE|INFO|DEBUG" } ,
+ { "timelimit", 't', POPT_ARG_INT, &options.timelimit, 0,
+ "time limit", "seconds" },
+ { "srvid", 'S', POPT_ARG_STRING, &options.srvidstr, 0,
+ "srvid to register", "srvid" },
+ POPT_TABLEEND
+};
+
+static void dummy_handler(uint64_t srvid, TDB_DATA data, void *private_data)
+{
+ bool *done = (bool *)private_data;
+
+ *done = true;
+}
+
+int main(int argc, const char *argv[])
+{
+ TALLOC_CTX *mem_ctx;
+ struct tevent_context *ev;
+ struct ctdb_client_context *client;
+ const char *ctdb_socket;
+ poptContext pc;
+ int opt, ret;
+ int log_level;
+ bool status, done;
+
+ /* Set default options */
+ options.sockpath = CTDB_SOCKET;
+ options.debuglevel = "ERR";
+ options.timelimit = 60;
+ options.srvidstr = NULL;
+
+ ctdb_socket = getenv("CTDB_SOCKET");
+ if (ctdb_socket != NULL) {
+ options.sockpath = ctdb_socket;
+ }
+
+ pc = poptGetContext(argv[0], argc, argv, cmdline_options,
+ POPT_CONTEXT_KEEP_FIRST);
+ while ((opt = poptGetNextOpt(pc)) != -1) {
+ fprintf(stderr, "Invalid option %s\n", poptBadOption(pc, 0));
+ exit(1);
+ }
+
+ if (options.sockpath == NULL) {
+ fprintf(stderr, "Please specify socket path\n");
+ poptPrintHelp(pc, stdout, 0);
+ exit(1);
+ }
+
+ mem_ctx = talloc_new(NULL);
+ if (mem_ctx == NULL) {
+ fprintf(stderr, "Memory allocation error\n");
+ exit(1);
+ }
+
+ ev = tevent_context_init(mem_ctx);
+ if (ev == NULL) {
+ fprintf(stderr, "Memory allocation error\n");
+ exit(1);
+ }
+
+ status = debug_level_parse(options.debuglevel, &log_level);
+ if (! status) {
+ fprintf(stderr, "Invalid debug level\n");
+ poptPrintHelp(pc, stdout, 0);
+ exit(1);
+ }
+
+ setup_logging("dummy_client", DEBUG_STDERR);
+ DEBUGLEVEL = log_level;
+
+ ret = ctdb_client_init(mem_ctx, ev, options.sockpath, &client);
+ if (ret != 0) {
+ D_ERR("Failed to initialize client, ret=%d\n", ret);
+ exit(1);
+ }
+
+ done = false;
+ if (options.srvidstr != NULL) {
+ uint64_t srvid;
+
+ srvid = strtoull(options.srvidstr, NULL, 0);
+
+ ret = ctdb_client_set_message_handler(ev, client, srvid,
+ dummy_handler, &done);
+ if (ret != 0) {
+ D_ERR("Failed to register srvid, ret=%d\n", ret);
+ talloc_free(client);
+ exit(1);
+ }
+
+ D_INFO("Registered SRVID 0x%"PRIx64"\n", srvid);
+ }
+
+ ret = ctdb_client_wait_timeout(ev, &done,
+ tevent_timeval_current_ofs(options.timelimit, 0));
+ if (ret != 0 && ret == ETIME) {
+ D_ERR("client_wait_timeout() failed, ret=%d\n", ret);
+ talloc_free(client);
+ exit(1);
+ }
+
+ talloc_free(client);
+ exit(0);
+}
diff --git a/ctdb/tests/src/fake_ctdbd.c b/ctdb/tests/src/fake_ctdbd.c
index 7aa3ca2..9e2062a 100644
--- a/ctdb/tests/src/fake_ctdbd.c
+++ b/ctdb/tests/src/fake_ctdbd.c
@@ -108,6 +108,13 @@ struct fake_control_failure {
const char *comment;
};
+struct ctdb_client {
+ struct ctdb_client *prev, *next;
+ struct ctdbd_context *ctdb;
+ pid_t pid;
+ void *state;
+};
+
struct ctdbd_context {
struct node_map *node_map;
struct interface_map *iface_map;
@@ -126,6 +133,7 @@ struct ctdbd_context {
char *reclock;
struct ctdb_public_ip_list *known_ips;
struct fake_control_failure *control_failures;
+ struct ctdb_client *client_list;
};
/*
@@ -820,6 +828,48 @@ fail:
}
/*
+ * Manage clients
+ */
+
+static int ctdb_client_destructor(struct ctdb_client *client)
+{
+ DLIST_REMOVE(client->ctdb->client_list, client);
+ return 0;
+}
+
+static int client_add(struct ctdbd_context *ctdb, pid_t client_pid,
+ void *client_state)
+{
+ struct ctdb_client *client;
+
+ client = talloc_zero(client_state, struct ctdb_client);
+ if (client == NULL) {
+ return ENOMEM;
+ }
+
+ client->ctdb = ctdb;
+ client->pid = client_pid;
+ client->state = client_state;
+
+ DLIST_ADD(ctdb->client_list, client);
+ talloc_set_destructor(client, ctdb_client_destructor);
+ return 0;
+}
+
+static void *client_find(struct ctdbd_context *ctdb, pid_t client_pid)
+{
+ struct ctdb_client *client;
+
+ for (client=ctdb->client_list; client != NULL; client=client->next) {
+ if (client->pid == client_pid) {
+ return client->state;
+ }
+ }
+
+ return NULL;
+}
+
+/*
* CTDB context setup
*/
@@ -1143,6 +1193,7 @@ struct client_state {
int fd;
struct ctdbd_context *ctdb;
int pnn;
+ pid_t pid;
struct comm_context *comm;
struct srvid_register_state *rstate;
int status;
@@ -1256,11 +1307,22 @@ static void control_process_exists(TALLOC_CTX *mem_ctx,
struct ctdb_req_header *header,
struct ctdb_req_control *request)
{
+ struct client_state *state = tevent_req_data(
+ req, struct client_state);
+ struct ctdbd_context *ctdb = state->ctdb;
+ struct client_state *cstate;
struct ctdb_reply_control reply;
reply.rdata.opcode = request->opcode;
- reply.status = kill(request->rdata.data.pid, 0);
- reply.errmsg = NULL;
+
+ cstate = client_find(ctdb, request->rdata.data.pid);
+ if (cstate == NULL) {
+ reply.status = -1;
+ reply.errmsg = "No client for PID";
+ } else {
+ reply.status = kill(request->rdata.data.pid, 0);
+ reply.errmsg = NULL;
+ }
client_send_control(req, header, &reply);
}
@@ -2975,6 +3037,8 @@ static struct tevent_req *client_send(TALLOC_CTX *mem_ctx,
{
struct tevent_req *req;
struct client_state *state;
+ struct ucred cr;
+ socklen_t crl = sizeof(struct ucred);
int ret;
req = tevent_req_create(mem_ctx, &state, struct client_state);
@@ -2987,6 +3051,13 @@ static struct tevent_req *client_send(TALLOC_CTX *mem_ctx,
state->ctdb = ctdb;
state->pnn = pnn;
+ ret = getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &cr, &crl);
+ if (ret != 0) {
+ tevent_req_error(req, ret);
+ return tevent_req_post(req, ev);
+ }
+ state->pid = cr.pid;
+
ret = comm_setup(state, ev, fd, client_read_handler, req,
client_dead_handler, req, &state->comm);
if (ret != 0) {
@@ -2994,6 +3065,12 @@ static struct tevent_req *client_send(TALLOC_CTX *mem_ctx,
return tevent_req_post(req, ev);
}
+ ret = client_add(ctdb, state->pid, state);
+ if (ret != 0) {
+ tevent_req_error(req, ret);
+ return tevent_req_post(req, ev);
+ }
+
DEBUG(DEBUG_INFO, ("New client fd=%d\n", fd));
return req;
diff --git a/ctdb/tests/tool/ctdb.process-exists.001.sh b/ctdb/tests/tool/ctdb.process-exists.001.sh
index b153da1..2339344 100755
--- a/ctdb/tests/tool/ctdb.process-exists.001.sh
+++ b/ctdb/tests/tool/ctdb.process-exists.001.sh
@@ -11,12 +11,14 @@ NODEMAP
2 192.168.20.43 0x0
EOF
-pid=$(ctdbd_getpid)
+dummy_client -s $ctdbd_socket &
+pid=$!
ok "PID $pid exists"
simple_test "$pid"
-# Use a PID that is probably impossible. It must fit into 32 bits but
-# should be larger than most settings for pid_max.
-required_result 1 "PID 99999999 does not exist"
-simple_test "99999999"
+kill -9 $pid
+
+pid=$(ctdbd_getpid)
+required_result 1 "PID $pid does not exist"
+simple_test "$pid"
diff --git a/ctdb/wscript b/ctdb/wscript
index fe7d712..754cdf3 100644
--- a/ctdb/wscript
+++ b/ctdb/wscript
@@ -793,7 +793,8 @@ def build(bld):
'transaction_loop',
'update_record',
'update_record_persistent',
- 'lock_tdb'
+ 'lock_tdb',
+ 'dummy_client'
]
for target in ctdb_tests:
--
Samba Shared Repository
More information about the samba-cvs
mailing list