Attachment 18501 Details for Bug 15755 – Patch for v4-21-test and v4-20-test

[patch] Patch for v4-21-test and v4-20-test

BZ15755.patch (text/plain), 4.24 KB, created by Martin Schwenke on 2024-11-21 03:17:52 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Schwenke

Created: 2024-11-21 03:17:52 UTC

Size: 4.24 KB

patch

obsolete

>From 64185f4934de9279c6fe3db9c60a328b75aa6455 Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <mschwenke@ddn.com>
>Date: Wed, 20 Nov 2024 14:37:09 +1100
>Subject: [PATCH] ctdb-common: Map ENOENT for a missing event script to ENOEXEC
>
>This handles the case where an event script is disabled by unlinking,
>while an event is being run, after the script list has been created.
>Without this change the script will fail.  With this change the script
>will be marked as DISABLED.  See the comment added by this commit for
>more details.
>
>Add a testcase to simulate the race, using an event script to disable
>subsequent ones.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15755
>RN: Avoid event failure race when disabling an event script
>
>Signed-off-by: Martin Schwenke <mschwenke@ddn.com>
>Reviewed-by: Amitay Isaacs <amitay@gmail.com>
>
>Autobuild-User(master): Amitay Isaacs <amitay@samba.org>
>Autobuild-Date(master): Thu Nov 21 01:42:09 UTC 2024 on atb-devel-224
>
>(cherry picked from commit fee31b6cb2b8f7dd111bdd9d2ff5479c31cbca37)
>---
> ctdb/common/run_event.c                       | 23 +++++++++++-
> .../share/events/data/01.dummy.script         |  4 ++
> ctdb/tests/UNIT/eventd/eventd_009.sh          | 37 +++++++++++++++++++
> 3 files changed, 62 insertions(+), 2 deletions(-)
>
>diff --git a/ctdb/common/run_event.c b/ctdb/common/run_event.c
>index d283664e2cf..30369eeff22 100644
>--- a/ctdb/common/run_event.c
>+++ b/ctdb/common/run_event.c
>@@ -268,8 +268,27 @@ static int run_event_script_status(struct run_event_script *script)
> 	if (script->result.sig > 0) {
> 		ret = -EINTR;
> 	} else if (script->result.err > 0) {
>-		if (script->result.err == EACCES) {
>-			/* Map EACCESS to ENOEXEC */
>+		if (script->result.err == EACCES ||
>+		    script->result.err == ENOENT) {
>+			/*
>+			 * Map EACCESS/ENOENT to ENOEXEC
>+			 *
>+			 * ENOENT: Disabling a standard event script
>+			 * by removing its symlink can result in
>+			 * ENOENT.  This happens when the script list
>+			 * is built while the link exists, but the
>+			 * link is removed before the attempt to run
>+			 * it.  Map it to ENOEXEC (which causes a
>+			 * script to be shown as DISABLED).  This
>+			 * makes it impossible to distinguish a
>+			 * removed symlink from a dangling
>+			 * symlink... but the latter can just be
>+			 * defined as disabled.  It should be rare
>+			 * because it shouldn't happen if event
>+			 * scripts are properly managed.  If someone
>+			 * is doing weird things then they can easily
>+			 * debug such issues by looking at the link.
>+			 */
> 			ret = -ENOEXEC;
> 		} else {
> 			ret = -script->result.err;
>diff --git a/ctdb/tests/UNIT/eventd/etc-ctdb/share/events/data/01.dummy.script b/ctdb/tests/UNIT/eventd/etc-ctdb/share/events/data/01.dummy.script
>index 9c56f5b968b..b7b5945714c 100755
>--- a/ctdb/tests/UNIT/eventd/etc-ctdb/share/events/data/01.dummy.script
>+++ b/ctdb/tests/UNIT/eventd/etc-ctdb/share/events/data/01.dummy.script
>@@ -2,5 +2,9 @@
> 
> case "$1" in
> "failure") exit 1 ;;
>+"disablehack")
>+	ctdb-event script disable data 02.disabled
>+	ctdb-event script disable data 03.notalink
>+	;;
> *) exit 0 ;;
> esac
>diff --git a/ctdb/tests/UNIT/eventd/eventd_009.sh b/ctdb/tests/UNIT/eventd/eventd_009.sh
>index 39e5cd658cc..86c7224ad23 100755
>--- a/ctdb/tests/UNIT/eventd/eventd_009.sh
>+++ b/ctdb/tests/UNIT/eventd/eventd_009.sh
>@@ -153,3 +153,40 @@ ok <<EOF
> * 03.notalink
> EOF
> simple_test script list data
>+
>+#
>+# Test disabling of scripts after the script list has been
>+# built. Normally this would be an admin racing with eventd instead of
>+# one script disabling subsequent ones.
>+#
>+
>+# First enable all scripts - this might repeat some previous stuff
>+ok_null
>+simple_test script enable data 01.dummy
>+ok_null
>+simple_test script enable data 02.disabled
>+ok_null
>+simple_test script enable data 03.notalink
>+
>+# Confirm expected state
>+ok <<EOF
>+* 01.dummy
>+* 02.disabled
>+
>+* 03.notalink
>+EOF
>+simple_test script list data
>+
>+# Now run the event that disables the subsequent scripts:
>+# - 02.disabled has its link removed
>+# - 03.notalink effectively has "chmod -x" applied
>+ok_null
>+simple_test run 10 data disablehack
>+
>+# Confirm that both subsequent scripts were disabled
>+ok <<EOF
>+01.dummy             OK         DURATION DATETIME
>+02.disabled          DISABLED  
>+03.notalink          DISABLED  
>+EOF
>+simple_test status data disablehack
>-- 
>2.45.2
>

Flags:

amitay: review+

Actions: View

Attachments on bug 15755: 18501