From: Ralph Boehme <slow@samba.org>
Date: Mon, 8 Aug 2016 14:58:51 +0000 (+0200)
Subject: dbwrap_ctdb: treat empty records in ltdb as non-existing
X-Git-Tag: samba-4.3.12~67
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=68d13b1d50e19594503f40872413ecdacab24fbe;p=thirdparty%2Fsamba.git

dbwrap_ctdb: treat empty records in ltdb as non-existing

When fetching records from remote ctdb nodes via ctdbd_parse() or in
db_ctdb_traverse(), we already check for tombstone records and skip
them. This was originally also done for the ltdb checks.

See also bug: https://bugzilla.samba.org/show_bug.cgi?id=10008
(commit 1cae59ce112ccb51b45357a52b902f80fce1eef1).

Commit 925625b52886d40b50fc631bad8bdc81970f7598 reverted part of the
patch of bug 10008 due to a deadlock it introduced.

This patch re-introduces the consistent treatment of empty records in
the ltdb but avoids the deadlock by correctly signalling
NT_STATUS_NOT_FOUND if an empty record is found authoritatively in
the ltdb and not calling ctdb in this case.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12005

Pair-Programmed-With: Michael Adam <obnox@samba.org>

Signed-off-by: Ralph Boehme <slow@samba.org>
Signed-off-by: Michael Adam <obnox@samba.org>

Autobuild-User(master): Michael Adam <obnox@samba.org>
Autobuild-Date(master): Tue Aug  9 04:38:44 CEST 2016 on sn-devel-144

(backported from commit 25df582739918b7afd4e5497eaffe279e2d92cd1)

Autobuild-User(v4-3-test): Karolin Seeger <kseeger@samba.org>
Autobuild-Date(v4-3-test): Wed Aug 10 14:58:12 CEST 2016 on sn-devel-104
---

diff --git a/source3/lib/dbwrap/dbwrap_ctdb.c b/source3/lib/dbwrap/dbwrap_ctdb.c
index f37bfd82596..64cc7c74987 100644
--- a/source3/lib/dbwrap/dbwrap_ctdb.c
+++ b/source3/lib/dbwrap/dbwrap_ctdb.c
@@ -1213,6 +1213,7 @@ struct db_ctdb_parse_record_state {
 	void *private_data;
 	bool ask_for_readonly_copy;
 	bool done;
+	bool empty_record;
 };
 
 static void db_ctdb_parse_record_parser(
@@ -1232,7 +1233,16 @@ static void db_ctdb_parse_record_parser_nonpersistent(
 		(struct db_ctdb_parse_record_state *)private_data;
 
 	if (db_ctdb_can_use_local_hdr(header, true)) {
-		state->parser(key, data, state->private_data);
+		/*
+		 * A record consisting only of the ctdb header can be
+		 * a validly created empty record or a tombstone
+		 * record of a deleted record (not vacuumed yet). Mark
+		 * it accordingly.
+		 */
+		state->empty_record = (data.dsize == 0);
+		if (!state->empty_record) {
+			state->parser(key, data, state->private_data);
+		}
 		state->done = true;
 	} else {
 		/*
@@ -1257,6 +1267,7 @@ static NTSTATUS db_ctdb_parse_record(struct db_context *db, TDB_DATA key,
 
 	state.parser = parser;
 	state.private_data = private_data;
+	state.empty_record = false;
 
 	if (ctx->transaction != NULL) {
 		struct db_ctdb_transaction_handle *h = ctx->transaction;
@@ -1288,6 +1299,20 @@ static NTSTATUS db_ctdb_parse_record(struct db_context *db, TDB_DATA key,
 	status = db_ctdb_ltdb_parse(
 		ctx, key, db_ctdb_parse_record_parser_nonpersistent, &state);
 	if (NT_STATUS_IS_OK(status) && state.done) {
+		if (state.empty_record) {
+			/*
+			 * We know authoritatively, that this is an empty
+			 * record. Since ctdb does not distinguish between empty
+			 * and deleted records, this can be a record stored as
+			 * empty or a not-yet-vacuumed tombstone record of a
+			 * deleted record. Now Samba right now can live without
+			 * empty records, so we can safely report this record
+			 * as non-existing.
+			 *
+			 * See bugs 10008 and 12005.
+			 */
+			return NT_STATUS_NOT_FOUND;
+		}
 		return NT_STATUS_OK;
 	}