Attachment 13569 Details for Bug 13012 – Patches for v4-6

[patch] Patches for v4-6

BZ13012-v4-6.patch (text/plain), 14.20 KB, created by Amitay Isaacs on 2017-09-11 03:43:13 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Amitay Isaacs

Created: 2017-09-11 03:43:13 UTC

Size: 14.20 KB

patch

obsolete

>From 7ba508649335d17c436860072520489519afddfe Mon Sep 17 00:00:00 2001
>From: Amitay Isaacs <amitay@gmail.com>
>Date: Fri, 25 Aug 2017 15:00:59 +1000
>Subject: [PATCH 1/4] 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@gmail.com>
>Reviewed-by: Martin Schwenke <martin@meltin.net>
>(cherry picked from commit d0a20baf43834c7290dfd8f256d9521724202f0c)
>---
> ctdb/server/ctdb_daemon.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
>diff --git a/ctdb/server/ctdb_daemon.c b/ctdb/server/ctdb_daemon.c
>index d0d86a0c703..122d884a0c9 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;
> 	}
> 
>-- 
>2.13.5
>
>
>From 77a30d08838c07b4b602075fcf229457822d237a Mon Sep 17 00:00:00 2001
>From: Amitay Isaacs <amitay@gmail.com>
>Date: Fri, 25 Aug 2017 16:54:47 +1000
>Subject: [PATCH 2/4] 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@gmail.com>
>Reviewed-by: Martin Schwenke <martin@meltin.net>
>(cherry picked from commit 7dec80a7c042d83f9d48c75a8717c3d1b59b1fbf)
>---
> ctdb/tests/src/fake_ctdbd.c | 81 +++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 79 insertions(+), 2 deletions(-)
>
>diff --git a/ctdb/tests/src/fake_ctdbd.c b/ctdb/tests/src/fake_ctdbd.c
>index 7aa3ca229a3..9e2062a5084 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;
>-- 
>2.13.5
>
>
>From 0f39fcc1df19738afd6175c39b5ef755fb94b92c Mon Sep 17 00:00:00 2001
>From: Amitay Isaacs <amitay@gmail.com>
>Date: Wed, 30 Aug 2017 13:05:32 +1000
>Subject: [PATCH 3/4] 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@gmail.com>
>Reviewed-by: Martin Schwenke <martin@meltin.net>
>(cherry picked from commit 3067db5b50162fdae288aaad8e75beb924fc9494)
>---
> ctdb/tests/src/dummy_client.c | 148 ++++++++++++++++++++++++++++++++++++++++++
> ctdb/wscript                  |   3 +-
> 2 files changed, 150 insertions(+), 1 deletion(-)
> create mode 100644 ctdb/tests/src/dummy_client.c
>
>diff --git a/ctdb/tests/src/dummy_client.c b/ctdb/tests/src/dummy_client.c
>new file mode 100644
>index 00000000000..6af41f3b1f9
>--- /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/wscript b/ctdb/wscript
>index fe7d7122308..754cdf3c01f 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:
>-- 
>2.13.5
>
>
>From 14515ddaa094ba2d036b00e2de9e897a693603eb Mon Sep 17 00:00:00 2001
>From: Amitay Isaacs <amitay@gmail.com>
>Date: Fri, 25 Aug 2017 16:55:34 +1000
>Subject: [PATCH 4/4] ctdb-tests: Fix ctdb process-exist tests
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13012
>
>Signed-off-by: Amitay Isaacs <amitay@gmail.com>
>Reviewed-by: Martin Schwenke <martin@meltin.net>
>
>Autobuild-User(master): Amitay Isaacs <amitay@samba.org>
>Autobuild-Date(master): Sat Sep  9 14:44:57 CEST 2017 on sn-devel-144
>
>(cherry picked from commit 87f7d32a906799e83cb9b023978e689a630de017)
>---
> ctdb/tests/simple/07_ctdb_process_exists.sh | 34 ++++++++++++++++++-----------
> ctdb/tests/tool/ctdb.process-exists.001.sh  | 12 +++++-----
> 2 files changed, 28 insertions(+), 18 deletions(-)
>
>diff --git a/ctdb/tests/simple/07_ctdb_process_exists.sh b/ctdb/tests/simple/07_ctdb_process_exists.sh
>index b7492a84f19..c44924b0fe6 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 "Checking for PID $client_pid on node $test_node"
> status=0
>-try_command_on_node $test_node "$CTDB process-exists ${pid}" || status=$?
>+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/tool/ctdb.process-exists.001.sh b/ctdb/tests/tool/ctdb.process-exists.001.sh
>index b153da1e1fe..2339344fec5 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"
>-- 
>2.13.5
>

Flags:

martins: review+

Actions: View

Attachments on bug 13012: 13569 | 13570 | 13585 | 13586