[SCM] Samba Shared Repository - branch v4-5-test updated
Stefan Metzmacher
metze at samba.org
Tue Sep 6 10:19:11 UTC 2016
The branch, v4-5-test has been updated
via 51a6036 ctdb-tests: Add a test to ensure that CTDB works with no eventscripts
via af2386b ctdb-tests: Conditionally use temporary config file for local daemons
via 7e0846a ctdb-tests: Factor out function config_from_environment()
via 8b2e01a ctdb-daemon: Don't steal control structure before synchronous reply
via d9f5a6a ctdb-daemon: Handle failure immediately, do housekeeping later
via 41ca635 ctdb-daemon: Schedule running of callback if there are no event scripts
via 0ccfa21 dbcheck: Abandon dbcheck if we get an error during a transaction
via b005b5b dsdb: Allow missing a mandatory attribute from a dbcheck fix
from 181d050 script/release.sh: use 8 byte gpg key ids
https://git.samba.org/?p=samba.git;a=shortlog;h=v4-5-test
- Log -----------------------------------------------------------------
commit 51a6036b9539f02c29b76fb3a2ff96d02453d2cf
Author: Martin Schwenke <martin at meltin.net>
Date: Mon Aug 29 16:52:45 2016 +1000
ctdb-tests: Add a test to ensure that CTDB works with no eventscripts
This only tests something on local daemons, since the configuration
can't be easily manipulated on a real cluster.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12180
Signed-off-by: Martin Schwenke <martin at meltin.net>
Reviewed-by: Amitay Isaacs <amitay at gmail.com>
Autobuild-User(master): Amitay Isaacs <amitay at samba.org>
Autobuild-Date(master): Thu Sep 1 17:15:06 CEST 2016 on sn-devel-144
(cherry picked from commit 625f080f213d03fd4b08e1b6ff9f1415f77ee73b)
Autobuild-User(v4-5-test): Stefan Metzmacher <metze at samba.org>
Autobuild-Date(v4-5-test): Tue Sep 6 12:18:42 CEST 2016 on sn-devel-144
commit af2386bdc2f0b69acb65701f15352ea14c0f0051
Author: Martin Schwenke <martin at meltin.net>
Date: Mon Aug 29 16:49:07 2016 +1000
ctdb-tests: Conditionally use temporary config file for local daemons
If there's configuration in the environment then daemons_start()
should use a temporary configuration file with that appended.
This means that global overrides don't (harmlessly) build up in the
configuration file during each test and individual tests can override
configuration when calling daemons_start() directly.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12180
Signed-off-by: Martin Schwenke <martin at meltin.net>
Reviewed-by: Amitay Isaacs <amitay at gmail.com>
(cherry picked from commit 7885b9652fcb3b30361a8b2e0b4688c261b55065)
commit 7e0846a3b9dddc1adec11f04912cc2d3249d394c
Author: Martin Schwenke <martin at meltin.net>
Date: Mon Aug 29 16:05:33 2016 +1000
ctdb-tests: Factor out function config_from_environment()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12180
Signed-off-by: Martin Schwenke <martin at meltin.net>
Reviewed-by: Amitay Isaacs <amitay at gmail.com>
(cherry picked from commit a2bbf71ad67e5c3a6287cf62f54ff13389bf2143)
commit 8b2e01a20656620c63d84c8b02a9a13f41a0fe25
Author: Martin Schwenke <martin at meltin.net>
Date: Wed Aug 31 08:29:13 2016 +1000
ctdb-daemon: Don't steal control structure before synchronous reply
If *async_reply isn't set then the calling code will reply to the
control and free the control structure. In some places the control
structure pointer is stolen onto state before a synchronous exit due
to an error condition. The error handling then frees state and
returns an error. The calling code will access-after-free when trying
to reply to the control.
To make this easier to understand, the convention is that any
(immediate) error results in a synchronous reply to the control via an
error return code AND *async_reply not being set. In this case the
control structure pointer should never be stolen onto state. State is
never used for a synchronous reply, it is only ever used by a
callback.
Also initialise state->c to NULL so that any premature call to a
callback (e.g. in an immediate error path) is more obvious.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12180
Signed-off-by: Martin Schwenke <martin at meltin.net>
Reviewed-by: Amitay Isaacs <amitay at gmail.com>
(cherry picked from commit 9d975b860d52030a702723c70791c6a2829107c0)
commit d9f5a6ab0fba3c7ba77c93040c7e190877c3573d
Author: Martin Schwenke <martin at meltin.net>
Date: Fri Aug 26 16:38:56 2016 +1000
ctdb-daemon: Handle failure immediately, do housekeeping later
The callback should never be called before an immediate return. The
callback might reply to a control and the caller of
ctdb_event_script_callback_v() may not have assigned/stolen the
pointer to control structure into the private data. Therefore,
calling the callback can dereference an uninitialised pointer to the
control structure when attempting to reply.
An event script isn't being run until the child has been forked. So
update relevant state and set the destructor after this.
If the child can't be forked then free the state and return with an
error. The callback will not be called and the caller will process
the error correctly.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12180
Signed-off-by: Martin Schwenke <martin at meltin.net>
Reviewed-by: Amitay Isaacs <amitay at gmail.com>
(cherry picked from commit 582518c7e89b279e34147bdb0b04b73056fac048)
commit 41ca635abb14ee3fd881978d1559c284e2e4737a
Author: Martin Schwenke <martin at meltin.net>
Date: Fri Aug 26 16:29:47 2016 +1000
ctdb-daemon: Schedule running of callback if there are no event scripts
The callback should never be called before an immediate return. The
callback might reply to a control and the caller of
ctdb_event_script_callback_v() may not have assigned/stolen the
pointer to control structure into the private data. Therefore,
calling the callback can dereference an uninitialised pointer to the
control structure when attempting to reply.
ctdb_event_script_callback_v() must succeed when there are no event
scripts. On success the caller will mark the call as asynchronous and
expect the callback to be called. Given that it can't be called
before return then it needs to be scheduled.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12180
Signed-off-by: Martin Schwenke <martin at meltin.net>
Reviewed-by: Amitay Isaacs <amitay at gmail.com>
(cherry picked from commit 9076c44f35bf309b9e183bae98829f7154b93f33)
commit 0ccfa212e431238b6ca1623d4280041872d5126d
Author: Andrew Bartlett <abartlet at samba.org>
Date: Fri Aug 26 15:53:19 2016 +1200
dbcheck: Abandon dbcheck if we get an error during a transaction
Otherwise, anything that the transaction has already done to the DB will be left in the DB
even despite the failure. For example, if a fix wrote to the DB, but then failed a post-write
check, then the fix will not be unrolled.
This is because we do not have nested transactions in TDB.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12178
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
Autobuild-User(master): Stefan Metzmacher <metze at samba.org>
Autobuild-Date(master): Mon Aug 29 12:46:21 CEST 2016 on sn-devel-144
(cherry picked from commit db32a0e5ea8f652857e45480cc31ecb1ef884c1a)
commit b005b5b2991583cc9a5d7008a0c71e4933fb9ffc
Author: Andrew Bartlett <abartlet at samba.org>
Date: Fri Aug 26 15:54:35 2016 +1200
dsdb: Allow missing a mandatory attribute from a dbcheck fix
dbcheck of the rid pool (CN=RID Set) for another server will otherwise fail because
rIDNextRid is not replicated, and so it not present
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12178
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
(cherry picked from commit 9d0c869e36ba2f43fd2ed4cd090b48102d499bc8)
-----------------------------------------------------------------------
Summary of changes:
ctdb/server/ctdb_takeover.c | 11 ++-
ctdb/server/eventscript.c | 87 ++++++++++++++++++----
ctdb/tests/simple/28_zero_eventscripts.sh | 45 +++++++++++
ctdb/tests/simple/scripts/local_daemons.bash | 33 ++++++--
python/samba/dbchecker.py | 7 ++
source4/dsdb/samdb/ldb_modules/objectclass_attrs.c | 9 ++-
testprogs/blackbox/dbcheck-oldrelease.sh | 10 +++
7 files changed, 179 insertions(+), 23 deletions(-)
create mode 100755 ctdb/tests/simple/28_zero_eventscripts.sh
Changeset truncated at 500 lines:
diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c
index ede635e..d10ffef 100644
--- a/ctdb/server/ctdb_takeover.c
+++ b/ctdb/server/ctdb_takeover.c
@@ -522,7 +522,7 @@ static int32_t ctdb_do_takeip(struct ctdb_context *ctdb,
state = talloc(vnn, struct ctdb_do_takeip_state);
CTDB_NO_MEMORY(ctdb, state);
- state->c = talloc_steal(ctdb, c);
+ state->c = NULL;
state->vnn = vnn;
vnn->update_in_flight = true;
@@ -551,6 +551,7 @@ static int32_t ctdb_do_takeip(struct ctdb_context *ctdb,
return -1;
}
+ state->c = talloc_steal(ctdb, c);
return 0;
}
@@ -659,7 +660,7 @@ static int32_t ctdb_do_updateip(struct ctdb_context *ctdb,
state = talloc(vnn, struct ctdb_do_updateip_state);
CTDB_NO_MEMORY(ctdb, state);
- state->c = talloc_steal(ctdb, c);
+ state->c = NULL;
state->old = old;
state->vnn = vnn;
@@ -691,6 +692,7 @@ static int32_t ctdb_do_updateip(struct ctdb_context *ctdb,
return -1;
}
+ state->c = talloc_steal(ctdb, c);
return 0;
}
@@ -1003,8 +1005,8 @@ int32_t ctdb_control_release_ip(struct ctdb_context *ctdb,
return -1;
}
- state->c = talloc_steal(state, c);
- state->addr = talloc(state, ctdb_sock_addr);
+ state->c = NULL;
+ state->addr = talloc(state, ctdb_sock_addr);
if (state->addr == NULL) {
ctdb_set_error(ctdb, "Out of memory at %s:%d",
__FILE__, __LINE__);
@@ -1037,6 +1039,7 @@ int32_t ctdb_control_release_ip(struct ctdb_context *ctdb,
/* tell the control that we will be reply asynchronously */
*async_reply = true;
+ state->c = talloc_steal(state, c);
return 0;
}
diff --git a/ctdb/server/eventscript.c b/ctdb/server/eventscript.c
index bd5bc0d..86d37d9 100644
--- a/ctdb/server/eventscript.c
+++ b/ctdb/server/eventscript.c
@@ -699,6 +699,62 @@ static int remove_callback(struct event_script_callback *callback)
return 0;
}
+struct schedule_callback_state {
+ struct ctdb_context *ctdb;
+ void (*callback)(struct ctdb_context *, int, void *);
+ void *private_data;
+ int status;
+ struct tevent_immediate *im;
+};
+
+static void schedule_callback_handler(struct tevent_context *ctx,
+ struct tevent_immediate *im,
+ void *private_data)
+{
+ struct schedule_callback_state *state =
+ talloc_get_type_abort(private_data,
+ struct schedule_callback_state);
+
+ if (state->callback != NULL) {
+ state->callback(state->ctdb, state->status,
+ state->private_data);
+ }
+ talloc_free(state);
+}
+
+static int
+schedule_callback_immediate(struct ctdb_context *ctdb,
+ void (*callback)(struct ctdb_context *,
+ int, void *),
+ void *private_data,
+ int status)
+{
+ struct schedule_callback_state *state;
+ struct tevent_immediate *im;
+
+ state = talloc_zero(ctdb, struct schedule_callback_state);
+ if (state == NULL) {
+ DEBUG(DEBUG_ERR, (__location__ " out of memory\n"));
+ return -1;
+ }
+ im = tevent_create_immediate(state);
+ if (im == NULL) {
+ DEBUG(DEBUG_ERR, (__location__ " out of memory\n"));
+ talloc_free(state);
+ return -1;
+ }
+
+ state->ctdb = ctdb;
+ state->callback = callback;
+ state->private_data = private_data;
+ state->status = status;
+ state->im = im;
+
+ tevent_schedule_immediate(im, ctdb->ev,
+ schedule_callback_handler, state);
+ return 0;
+}
+
/*
run the event script in the background, calling the callback when
finished
@@ -815,28 +871,33 @@ static int ctdb_event_script_callback_v(struct ctdb_context *ctdb,
state->current = 0;
state->child = 0;
- if (call == CTDB_EVENT_MONITOR) {
- ctdb->current_monitor = state;
- }
-
- talloc_set_destructor(state, event_script_destructor);
-
- ctdb->active_events++;
-
/* Nothing to do? */
if (state->scripts->num_scripts == 0) {
- callback(ctdb, 0, private_data);
+ int ret = schedule_callback_immediate(ctdb, callback,
+ private_data, 0);
talloc_free(state);
+ if (ret != 0) {
+ DEBUG(DEBUG_ERR,
+ ("Unable to schedule callback for 0 scripts\n"));
+ return 1;
+ }
return 0;
}
state->scripts->scripts[0].status = fork_child_for_script(ctdb, state);
if (state->scripts->scripts[0].status != 0) {
- /* Callback is called from destructor, with fail result. */
talloc_free(state);
- return 0;
+ return -1;
}
+ if (call == CTDB_EVENT_MONITOR) {
+ ctdb->current_monitor = state;
+ }
+
+ ctdb->active_events++;
+
+ talloc_set_destructor(state, event_script_destructor);
+
if (!timeval_is_zero(&state->timeout)) {
tevent_add_timer(ctdb->ev, state,
timeval_current_ofs(state->timeout.tv_sec,
@@ -1015,7 +1076,7 @@ int32_t ctdb_run_eventscripts(struct ctdb_context *ctdb,
state = talloc(ctdb->event_script_ctx, struct eventscript_callback_state);
CTDB_NO_MEMORY(ctdb, state);
- state->c = talloc_steal(state, c);
+ state->c = NULL;
DEBUG(DEBUG_NOTICE,("Running eventscripts with arguments %s\n", indata.dptr));
@@ -1031,7 +1092,7 @@ int32_t ctdb_run_eventscripts(struct ctdb_context *ctdb,
/* tell ctdb_control.c that we will be replying asynchronously */
*async_reply = true;
-
+ state->c = talloc_steal(state, c);
return 0;
}
diff --git a/ctdb/tests/simple/28_zero_eventscripts.sh b/ctdb/tests/simple/28_zero_eventscripts.sh
new file mode 100755
index 0000000..7c03ae4
--- /dev/null
+++ b/ctdb/tests/simple/28_zero_eventscripts.sh
@@ -0,0 +1,45 @@
+#!/bin/bash
+
+test_info()
+{
+ cat <<EOF
+Check that CTDB operated correctly if there are 0 event scripts
+
+This test only does anything with local daemons. On a real cluster it
+has no way of updating configuration.
+EOF
+}
+
+. "${TEST_SCRIPTS_DIR}/integration.bash"
+
+ctdb_test_init "$@"
+
+set -e
+
+cluster_is_healthy
+
+if [ -z "$TEST_LOCAL_DAEMONS" ] ; then
+ echo "SKIPPING this test - only runs against local daemons"
+ exit 0
+fi
+
+# Reset configuration
+ctdb_restart_when_done
+
+daemons_stop
+
+echo "Starting CTDB with an empty eventscript directory..."
+empty_dir=$(mktemp -d --tmpdir="$TEST_VAR_DIR")
+ctdb_test_exit_hook_add "rmdir $empty_dir"
+CTDB_EVENT_SCRIPT_DIR="$empty_dir" daemons_start
+
+wait_until_ready
+
+# If this fails to find processes then the tests fails, so look at
+# full command-line so this will work with valgrind. Note that the
+# output could be generated with pgrep's -a option but it doesn't
+# exist in older versions.
+ps -p $(pgrep -f '\<ctdbd\>' | xargs | sed -e 's| |,|g') -o args ww
+
+echo
+echo "Good, that seems to work!"
diff --git a/ctdb/tests/simple/scripts/local_daemons.bash b/ctdb/tests/simple/scripts/local_daemons.bash
index ecb64f9..fb1e7e1 100644
--- a/ctdb/tests/simple/scripts/local_daemons.bash
+++ b/ctdb/tests/simple/scripts/local_daemons.bash
@@ -22,6 +22,15 @@ export CTDB_NODES="${TEST_VAR_DIR}/nodes.txt"
#######################################
+config_from_environment ()
+{
+ # Override from the environment. This would be easier if env was
+ # guaranteed to quote its output so it could be reused.
+ env |
+ grep '^CTDB_' |
+ sed -e 's@=\([^"]\)@="\1@' -e 's@[^"]$@&"@' -e 's@="$@&"@'
+}
+
setup_ctdb ()
{
mkdir -p "${TEST_VAR_DIR}/test.db/persistent"
@@ -99,11 +108,9 @@ CTDB_SOCKET="${TEST_VAR_DIR}/sock.$pnn"
CTDB_NOSETSCHED=yes
EOF
- # Override from the environment. This would be easier if env was
- # guaranteed to quote its output so it could be reused.
- env |
- grep '^CTDB_' |
- sed -e 's@=\([^"]\)@="\1@' -e 's@[^"]$@&"@' -e 's@="$@&"@' >>"$conf"
+ # Append any configuration variables set in environment to
+ # configuration file so they affect CTDB after each restart.
+ config_from_environment >>"$conf"
done
}
@@ -116,9 +123,25 @@ daemons_start ()
local pidfile="${TEST_VAR_DIR}/ctdbd.${pnn}.pid"
local conf="${TEST_VAR_DIR}/ctdbd.${pnn}.conf"
+ # If there is any CTDB configuration in the environment then
+ # append it to the regular configuration in a temporary
+ # configuration file and use it just this once.
+ local tmp_conf=""
+ local env_conf=$(config_from_environment)
+ if [ -n "$env_conf" ] ; then
+ tmp_conf=$(mktemp --tmpdir="$TEST_VAR_DIR")
+ cat "$conf" >"$tmp_conf"
+ echo "$env_conf" >>"$tmp_conf"
+ conf="$tmp_conf"
+ fi
+
CTDBD="${VALGRIND} ctdbd --sloppy-start --nopublicipcheck" \
CTDBD_CONF="$conf" \
ctdbd_wrapper "$pidfile" start
+
+ if [ -n "$tmp_conf" ] ; then
+ rm -f "$tmp_conf"
+ fi
done
}
diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index e904b4a..9b0784b 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -31,6 +31,7 @@ from samba.common import dsdb_Dn
from samba.dcerpc import security
from samba.descriptor import get_wellknown_sds, get_diff_sds
from samba.auth import system_session, admin_session
+from samba.netcmd import CommandError
class dbcheck(object):
@@ -324,6 +325,8 @@ systemFlags: -1946157056%s""" % (dn, guid_suffix),
controls = controls + ["local_oid:%s:0" % dsdb.DSDB_CONTROL_DBCHECK]
self.samdb.delete(dn, controls=controls)
except Exception, err:
+ if self.in_transaction:
+ raise CommandError("%s : %s" % (msg, err))
self.report("%s : %s" % (msg, err))
return False
return True
@@ -336,6 +339,8 @@ systemFlags: -1946157056%s""" % (dn, guid_suffix),
controls = controls + ["local_oid:%s:0" % dsdb.DSDB_CONTROL_DBCHECK]
self.samdb.modify(m, controls=controls, validate=validate)
except Exception, err:
+ if self.in_transaction:
+ raise CommandError("%s : %s" % (msg, err))
self.report("%s : %s" % (msg, err))
return False
return True
@@ -353,6 +358,8 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
controls = controls + ["local_oid:%s:0" % dsdb.DSDB_CONTROL_DBCHECK]
self.samdb.rename(from_dn, to_dn, controls=controls)
except Exception, err:
+ if self.in_transaction:
+ raise CommandError("%s : %s" % (msg, err))
self.report("%s : %s" % (msg, err))
return False
return True
diff --git a/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c b/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c
index f739c40..c83c2e9 100644
--- a/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c
+++ b/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c
@@ -419,8 +419,15 @@ static int attr_handler2(struct oc_context *ac)
}
}
+ /*
+ * We skip this check under dbcheck to allow fixing of other
+ * attributes even if an attribute is missing. This matters
+ * for CN=RID Set as the required attribute rIDNextRid is not
+ * replicated.
+ */
if (found_must_contain[0] != NULL &&
- ldb_msg_check_string_attribute(msg, "isDeleted", "TRUE") == 0) {
+ ldb_msg_check_string_attribute(msg, "isDeleted", "TRUE") == 0 &&
+ ldb_request_get_control(ac->req, DSDB_CONTROL_DBCHECK) == NULL) {
ldb_asprintf_errstring(ldb, "objectclass_attrs: at least one mandatory attribute ('%s') on entry '%s' wasn't specified!",
found_must_contain[0],
ldb_dn_get_linearized(msg->dn));
diff --git a/testprogs/blackbox/dbcheck-oldrelease.sh b/testprogs/blackbox/dbcheck-oldrelease.sh
index 1efea67..dd176cf 100755
--- a/testprogs/blackbox/dbcheck-oldrelease.sh
+++ b/testprogs/blackbox/dbcheck-oldrelease.sh
@@ -186,6 +186,15 @@ check_expected_before_values() {
}
# This should 'fail', because it returns the number of modified records
+dbcheck_objectclass() {
+ if [ x$RELEASE = x"release-4-1-6-partial-object" ]; then
+ $PYTHON $BINDIR/samba-tool dbcheck --cross-ncs --fix --yes -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb --attrs=objectclass $@
+ else
+ return 1
+ fi
+}
+
+# This should 'fail', because it returns the number of modified records
dbcheck() {
$PYTHON $BINDIR/samba-tool dbcheck --cross-ncs --fix --yes -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb $@
}
@@ -363,6 +372,7 @@ if [ -d $release_dir ]; then
testit "reindex" reindex
testit "current_version_mod" do_current_version_mod
testit "check_expected_before_values" check_expected_before_values
+ testit_expect_failure "dbcheck_objectclass" dbcheck_objectclass
testit_expect_failure "dbcheck" dbcheck
testit "check_expected_after_values" check_expected_after_values
testit "check_forced_duplicate_values" check_forced_duplicate_values
--
Samba Shared Repository
More information about the samba-cvs
mailing list