[SCM] Samba Shared Repository - branch master updated

Amitay Isaacs amitay at samba.org
Tue May 14 09:00:04 UTC 2019


The branch, master has been updated
       via  b1f4c86eea0 ctdb-common: Fix memory leak in run_proc
       via  30bc6e2529c ctdb-common: Fix memory leak
       via  6a2941e2a9f ctdb-recoverd: Fix memory leak
       via  dc89db8ca6a ctdb-tests: Fix logic error in simple ctdb reloadips test
       via  8be4ee1a28d ctdb-tests: Make ctdb reloadips tests more reliable
       via  cf00db40355 ctdb-tests: Capture output in $out on failure as well
      from  b1a32dd7f50 selftest: enable undefined behaviour sanitizer

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


- Log -----------------------------------------------------------------
commit b1f4c86eea022999d5439e4a6ef3494fe41479b6
Author: Amitay Isaacs <amitay at gmail.com>
Date:   Mon May 13 17:07:59 2019 +1000

    ctdb-common: Fix memory leak in run_proc
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13943
    
    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): Tue May 14 08:59:03 UTC 2019 on sn-devel-184

commit 30bc6e2529cdd444d4ec7902844c3a6fb0858090
Author: Martin Schwenke <martin at meltin.net>
Date:   Sat May 11 17:33:57 2019 +1000

    ctdb-common: Fix memory leak
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13943
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>

commit 6a2941e2a9fd6ab2d5b8dbac042b61a7b1b0b914
Author: Martin Schwenke <martin at meltin.net>
Date:   Sat May 11 14:24:24 2019 +1000

    ctdb-recoverd: Fix memory leak
    
    state is always freed before exiting this function, so allocate fde
    off it instead of long-lived ctdb context.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13943
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>

commit dc89db8ca6aadd4a9f7e8a85843c53709d04587c
Author: Martin Schwenke <martin at meltin.net>
Date:   Tue May 7 15:42:49 2019 +1000

    ctdb-tests: Fix logic error in simple ctdb reloadips test
    
    There is a chance that restoring IP addresses to the test node will
    result in different IP addresses being assigned to that node.
    Removing a single IP address may then fail (or be a no-op) if it is
    done after the restore.
    
    So, swap the single IP address removal to happen first, then restore,
    then remove all IP addresses.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13924
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>

commit 8be4ee1a28d5c037955832b6f827d40f28f02796
Author: Martin Schwenke <martin at meltin.net>
Date:   Tue May 7 15:29:19 2019 +1000

    ctdb-tests: Make ctdb reloadips tests more reliable
    
    ctdb reloadips will fail if it can't disable takover runs.  The most
    likely reason for this is that there is already a takeover run in
    progress.  We can't predict when this will happen, so retry if this
    occurs.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13924
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>

commit cf00db40355b49443263187f9d97934f91287e51
Author: Martin Schwenke <martin at meltin.net>
Date:   Mon May 13 17:40:15 2019 +1000

    ctdb-tests: Capture output in $out on failure as well
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13924
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>

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

Summary of changes:
 ctdb/common/event_script.c              |  3 +-
 ctdb/common/run_proc.c                  |  7 +++-
 ctdb/server/ctdb_recoverd.c             |  2 +-
 ctdb/tests/complex/18_ctdb_reloadips.sh | 35 +++++++++++++++--
 ctdb/tests/scripts/integration.bash     |  8 ++--
 ctdb/tests/simple/18_ctdb_reloadips.sh  | 70 +++++++++++++++++++++++----------
 6 files changed, 94 insertions(+), 31 deletions(-)


Changeset truncated at 500 lines:

diff --git a/ctdb/common/event_script.c b/ctdb/common/event_script.c
index 8978d1452c0..8bdfdd0b5ca 100644
--- a/ctdb/common/event_script.c
+++ b/ctdb/common/event_script.c
@@ -117,7 +117,8 @@ int event_script_get_list(TALLOC_CTX *mem_ctx,
 	}
 
 	*out = script_list;
-	return 0;
+	ret = 0;
+	goto done;
 
 nomem:
 	ret = ENOMEM;
diff --git a/ctdb/common/run_proc.c b/ctdb/common/run_proc.c
index 037b6d9651d..0c3c1de72fe 100644
--- a/ctdb/common/run_proc.c
+++ b/ctdb/common/run_proc.c
@@ -302,13 +302,15 @@ again:
 		proc->fd = -1;
 	}
 
+	DLIST_REMOVE(run_ctx->plist, proc);
+
 	/* Active run_proc request */
 	if (proc->req != NULL) {
 		run_proc_done(proc->req);
+	} else {
+		talloc_free(proc);
 	}
 
-	DLIST_REMOVE(run_ctx->plist, proc);
-
 	goto again;
 }
 
@@ -426,6 +428,7 @@ static void run_proc_done(struct tevent_req *req)
 	if (state->proc->output != NULL) {
 		state->output = talloc_steal(state, state->proc->output);
 	}
+	talloc_steal(state, state->proc);
 
 	tevent_req_done(req);
 }
diff --git a/ctdb/server/ctdb_recoverd.c b/ctdb/server/ctdb_recoverd.c
index 9b3559b2a92..3e63bd1e7a5 100644
--- a/ctdb/server/ctdb_recoverd.c
+++ b/ctdb/server/ctdb_recoverd.c
@@ -1134,7 +1134,7 @@ static int helper_run(struct ctdb_recoverd *rec, TALLOC_CTX *mem_ctx,
 
 	state->done = false;
 
-	fde = tevent_add_fd(rec->ctdb->ev, rec->ctdb, state->fd[0],
+	fde = tevent_add_fd(rec->ctdb->ev, state, state->fd[0],
 			    TEVENT_FD_READ, helper_handler, state);
 	if (fde == NULL) {
 		goto fail;
diff --git a/ctdb/tests/complex/18_ctdb_reloadips.sh b/ctdb/tests/complex/18_ctdb_reloadips.sh
index 4ba1b26a8e8..26782130430 100755
--- a/ctdb/tests/complex/18_ctdb_reloadips.sh
+++ b/ctdb/tests/complex/18_ctdb_reloadips.sh
@@ -184,6 +184,33 @@ check_ips ()
     fi
 }
 
+# ctdb reloadips will fail if it can't disable takover runs.  The most
+# likely reason for this is that there is already a takeover run in
+# progress.  We can't predict when this will happen, so retry if this
+# occurs.
+do_ctdb_reloadips ()
+{
+	local retry_max=10
+	local retry_count=0
+	while : ; do
+		if try_command_on_node "$test_node" "$CTDB reloadips" ; then
+			return 0
+		fi
+
+		if [ "$out" != "Failed to disable takeover runs" ] ; then
+			return 1
+		fi
+
+		if [ $retry_count -ge $retry_max ] ; then
+			return 1
+		fi
+
+		retry_count=$((retry_count + 1))
+		echo "Retrying..."
+		sleep_for 1
+	done
+}
+
 ####################
 
 new_ip_max=100
@@ -193,7 +220,7 @@ new_ip_max=100
 add_ips_to_original_config \
     $test_node "$addresses" "$iface" "$prefix" 1 $new_ip_max
 
-try_command_on_node $test_node "$CTDB reloadips"
+do_ctdb_reloadips
 
 check_ips $test_node "$iface" "$prefix" 1 $new_ip_max
 
@@ -207,7 +234,7 @@ echo "Using 'ctdb reloadips' to remove the 1st address just added..."
 add_ips_to_original_config \
     $test_node "$addresses" "$iface" "$prefix" 2 $new_ip_max
 
-try_command_on_node $test_node "$CTDB reloadips"
+do_ctdb_reloadips
 
 check_ips $test_node "$iface" "$prefix" 2 $new_ip_max
 
@@ -222,7 +249,7 @@ echo "Updating to include only about 1/2 of the new IPs..."
 add_ips_to_original_config \
     $test_node "$addresses" "$iface" "$prefix" $start $new_ip_max
 
-try_command_on_node $test_node "$CTDB reloadips"
+do_ctdb_reloadips
 
 check_ips $test_node "$iface" "$prefix" $start $new_ip_max
 
@@ -234,6 +261,6 @@ try_command_on_node any $CTDB sync
 echo "Restoring original IP configuration..."
 restore_public_addresses
 
-try_command_on_node $test_node "$CTDB reloadips"
+do_ctdb_reloadips
 
 check_ips $test_node "$iface" "$prefix" 0
diff --git a/ctdb/tests/scripts/integration.bash b/ctdb/tests/scripts/integration.bash
index 32a729d0249..30725c48e53 100644
--- a/ctdb/tests/scripts/integration.bash
+++ b/ctdb/tests/scripts/integration.bash
@@ -104,7 +104,11 @@ try_command_on_node ()
 
     local cmd="$*"
 
-    if ! onnode -q $onnode_opts "$nodespec" "$cmd" >"$outfile" 2>&1 ; then
+    local status=0
+    onnode -q $onnode_opts "$nodespec" "$cmd" >"$outfile" 2>&1 || status=$?
+    out=$(dd if="$outfile" bs=1k count=1 2>/dev/null)
+
+    if [ $status -ne 0 ] ; then
 	echo "Failed to execute \"$cmd\" on node(s) \"$nodespec\""
 	cat "$outfile"
 	return 1
@@ -114,8 +118,6 @@ try_command_on_node ()
 	echo "Output of \"$cmd\":"
 	cat "$outfile"
     fi
-
-    out=$(dd if="$outfile" bs=1k count=1 2>/dev/null)
 }
 
 sanity_check_output ()
diff --git a/ctdb/tests/simple/18_ctdb_reloadips.sh b/ctdb/tests/simple/18_ctdb_reloadips.sh
index 9c60a14a93e..451fca3a866 100755
--- a/ctdb/tests/simple/18_ctdb_reloadips.sh
+++ b/ctdb/tests/simple/18_ctdb_reloadips.sh
@@ -29,8 +29,6 @@ cluster_is_healthy
 
 select_test_node_and_ips
 
-echo "Emptying public addresses file on $test_node"
-
 try_command_on_node $test_node $CTDB_TEST_WRAPPER ctdb_base_show
 addresses="${out}/public_addresses"
 echo "Public addresses file on node $test_node is \"$addresses\""
@@ -42,29 +40,62 @@ restore_public_addresses ()
 }
 ctdb_test_exit_hook_add restore_public_addresses
 
-try_command_on_node $test_node "mv $addresses $backup && touch $addresses"
+# ctdb reloadips will fail if it can't disable takover runs.  The most
+# likely reason for this is that there is already a takeover run in
+# progress.  We can't predict when this will happen, so retry if this
+# occurs.
+do_ctdb_reloadips ()
+{
+	local retry_max=10
+	local retry_count=0
+	while : ; do
+		if try_command_on_node any "$CTDB reloadips all" ; then
+			return 0
+		fi
+
+		if [ "$out" != "Failed to disable takeover runs" ] ; then
+			return 1
+		fi
+
+		if [ $retry_count -ge $retry_max ] ; then
+			return 1
+		fi
+
+		retry_count=$((retry_count + 1))
+		echo "Retrying..."
+		sleep_for 1
+	done
+}
 
-try_command_on_node any $CTDB reloadips all
 
-echo "Getting list of public IPs on node $test_node"
-try_command_on_node $test_node "$CTDB ip | tail -n +2"
+echo "Removing IP $test_ip from node $test_node"
 
-if [ -n "$out" ] ; then
+try_command_on_node $test_node "mv $addresses $backup && grep -v '^${test_ip}/' $backup >$addresses"
+
+do_ctdb_reloadips
+
+try_command_on_node $test_node $CTDB ip
+
+if grep "^${test_ip} " <<<"$out" ; then
     cat <<EOF
-BAD: node $test_node still has ips:
+BAD: node $test_node can still host IP $test_ip:
 $out
 EOF
     exit 1
 fi
 
-echo "GOOD: no IPs left on node $test_node"
+cat <<EOF
+GOOD: node $test_node is no longer hosting IP $test_ip:
+$out
+EOF
 
 try_command_on_node any $CTDB sync
 
+
 echo "Restoring addresses"
 restore_public_addresses
 
-try_command_on_node any $CTDB reloadips all
+do_ctdb_reloadips
 
 echo "Getting list of public IPs on node $test_node"
 try_command_on_node $test_node "$CTDB ip | tail -n +2"
@@ -81,23 +112,22 @@ EOF
 
 try_command_on_node any $CTDB sync
 
-echo "Removing IP $test_ip from node $test_node"
 
-try_command_on_node $test_node "mv $addresses $backup && grep -v '^${test_ip}/' $backup >$addresses"
+echo "Emptying public addresses file on $test_node"
 
-try_command_on_node any $CTDB reloadips all
+try_command_on_node $test_node "mv $addresses $backup && touch $addresses"
 
-try_command_on_node $test_node $CTDB ip
+do_ctdb_reloadips
 
-if grep "^${test_ip} " <<<"$out" ; then
+echo "Getting list of public IPs on node $test_node"
+try_command_on_node $test_node "$CTDB ip | tail -n +2"
+
+if [ -n "$out" ] ; then
     cat <<EOF
-BAD: node $test_node can still host IP $test_ip:
+BAD: node $test_node still has ips:
 $out
 EOF
     exit 1
 fi
 
-cat <<EOF
-GOOD: node $test_node is no longer hosting IP $test_ip:
-$out
-EOF
+echo "GOOD: no IPs left on node $test_node"


-- 
Samba Shared Repository



More information about the samba-cvs mailing list