[PATCHES] Convert gencache to dbwrap to enable mutexes

Christof Schmitt cs at samba.org
Thu Jun 19 23:33:04 MDT 2014


On Tue, Jun 17, 2014 at 01:19:32PM +0200, Stefan (metze) Metzmacher wrote:
> I don't like DBWRAP_FLAG_LOCAL_ONLY much. I think using dbwrap_local_open()
> would be better. Even if it means we have the TDB_MUTEX_LOCKING magic twice
> (maybe we can have a helper function).

See the attached patchset with the change that the mutex check is now in
a helper function that is called from dbwrap_local_open and
db_open_ctdb. gencache is converted to use dbwrap_local_open. I still
need to run some testing, but is that your preferred approach?

> While having a look at it I may found a problem with mutexes and ctdb.
> 
>         /* only pass through specific flags */
>         tdb_flags &= TDB_SEQNUM|TDB_VOLATILE;
> 
> in db_open_ctdb() seems to completely useless (or wrong).
> ctdbd_db_attach() is called before and gets the raw value of tdb_flags.
> It has only has effect for
> 
>         db_ctdb->wtdb = tdb_wrap_open(db_ctdb, db_path, hash_size,
>                                       lpcfg_tdb_flags(lp_ctx, tdb_flags),
>                                       O_RDWR, 0);
> 
> So I think we can just remove tdb_flags &= TDB_SEQNUM|TDB_VOLATILE; above
> and let lpcfg_tdb_flags() remove TDB_MUTEX_LOCKING when adding TDB_NOMMAP.

The two patches at the end of the changeset implement those changes.

Christof
-------------- next part --------------
>From ea9b5d7cfadb35edc4de027e347edc1984a8956b Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Wed, 18 Jun 2014 16:37:13 -0700
Subject: [PATCH 1/9] dbwrap_ctdb: Move error handling to end of db_open_ctdb

---
 source3/lib/dbwrap/dbwrap_ctdb.c |   31 ++++++++++++-------------------
 1 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/source3/lib/dbwrap/dbwrap_ctdb.c b/source3/lib/dbwrap/dbwrap_ctdb.c
index ca33c8f..7325e73 100644
--- a/source3/lib/dbwrap/dbwrap_ctdb.c
+++ b/source3/lib/dbwrap/dbwrap_ctdb.c
@@ -1561,7 +1561,7 @@ struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
 				enum dbwrap_lock_order lock_order,
 				uint64_t dbwrap_flags)
 {
-	struct db_context *result;
+	struct db_context *result = NULL;
 	struct db_ctdb_ctx *db_ctdb;
 	char *db_path;
 	struct ctdbd_connection *conn;
@@ -1577,21 +1577,18 @@ struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
 
 	if (!(result = talloc_zero(mem_ctx, struct db_context))) {
 		DEBUG(0, ("talloc failed\n"));
-		TALLOC_FREE(result);
-		return NULL;
+		goto free_result;
 	}
 
 	if (!(db_ctdb = talloc(result, struct db_ctdb_ctx))) {
 		DEBUG(0, ("talloc failed\n"));
-		TALLOC_FREE(result);
-		return NULL;
+		goto free_result;
 	}
 
 	result->name = talloc_strdup(result, name);
 	if (result->name == NULL) {
 		DEBUG(0, ("talloc failed\n"));
-		TALLOC_FREE(result);
-		return NULL;
+		goto free_result;
 	}
 
 	db_ctdb->transaction = NULL;
@@ -1600,14 +1597,12 @@ struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
 	conn = messaging_ctdbd_connection();
 	if (conn == NULL) {
 		DEBUG(1, ("Could not connect to ctdb\n"));
-		TALLOC_FREE(result);
-		return NULL;
+		goto free_result;
 	}
 
 	if (!NT_STATUS_IS_OK(ctdbd_db_attach(conn, name, &db_ctdb->db_id, tdb_flags))) {
 		DEBUG(0, ("ctdbd_db_attach failed for %s\n", name));
-		TALLOC_FREE(result);
-		return NULL;
+		goto free_result;
 	}
 
 	db_path = ctdbd_dbpath(conn, db_ctdb, db_ctdb->db_id);
@@ -1634,8 +1629,7 @@ struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
 	if (!NT_STATUS_IS_OK(status) || (cstatus != 0)) {
 		DEBUG(1, ("CTDB_CONTROL_SET_DB_PRIORITY failed: %s, %d\n",
 			  nt_errstr(status), cstatus));
-		TALLOC_FREE(result);
-		return NULL;
+		goto free_result;
 	}
 
 #ifdef HAVE_CTDB_WANT_READONLY_DECL
@@ -1653,8 +1647,7 @@ struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
 		if (!NT_STATUS_IS_OK(status) || (cstatus != 0)) {
 			DEBUG(1, ("CTDB_CONTROL_SET_DB_READONLY failed: "
 				  "%s, %d\n", nt_errstr(status), cstatus));
-			TALLOC_FREE(result);
-			return NULL;
+			goto free_result;
 		}
 	}
 #endif
@@ -1671,8 +1664,7 @@ struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
 	talloc_unlink(db_path, lp_ctx);
 	if (db_ctdb->wtdb == NULL) {
 		DEBUG(0, ("Could not open tdb %s: %s\n", db_path, strerror(errno)));
-		TALLOC_FREE(result);
-		return NULL;
+		goto free_result;
 	}
 	talloc_free(db_path);
 
@@ -1681,8 +1673,7 @@ struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
 						    ctdb_conn_msg_ctx(conn));
 		if (db_ctdb->lock_ctx == NULL) {
 			DEBUG(0, ("g_lock_ctx_init failed\n"));
-			TALLOC_FREE(result);
-			return NULL;
+			goto free_result;
 		}
 	}
 
@@ -1710,5 +1701,7 @@ struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
 	DEBUG(3,("db_open_ctdb: opened database '%s' with dbid 0x%x\n",
 		 name, db_ctdb->db_id));
 
+free_result:
+	TALLOC_FREE(result);
 	return result;
 }
-- 
1.7.1


>From 6f3a4b0e5aa32638ea2661ebd1e9b1f34413087c Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Wed, 18 Jun 2014 16:55:20 -0700
Subject: [PATCH 2/9] dbwrap: Move loadparm_init_s3 in db_open_ctdb

---
 source3/lib/dbwrap/dbwrap_ctdb.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/source3/lib/dbwrap/dbwrap_ctdb.c b/source3/lib/dbwrap/dbwrap_ctdb.c
index 7325e73..b1b2036 100644
--- a/source3/lib/dbwrap/dbwrap_ctdb.c
+++ b/source3/lib/dbwrap/dbwrap_ctdb.c
@@ -1600,9 +1600,11 @@ struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
 		goto free_result;
 	}
 
+	lp_ctx = loadparm_init_s3(mem_ctx, loadparm_s3_helpers());
+
 	if (!NT_STATUS_IS_OK(ctdbd_db_attach(conn, name, &db_ctdb->db_id, tdb_flags))) {
 		DEBUG(0, ("ctdbd_db_attach failed for %s\n", name));
-		goto free_result;
+		goto lp_ctx_unlink;
 	}
 
 	db_path = ctdbd_dbpath(conn, db_ctdb, db_ctdb->db_id);
@@ -1629,7 +1631,7 @@ struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
 	if (!NT_STATUS_IS_OK(status) || (cstatus != 0)) {
 		DEBUG(1, ("CTDB_CONTROL_SET_DB_PRIORITY failed: %s, %d\n",
 			  nt_errstr(status), cstatus));
-		goto free_result;
+		goto lp_ctx_unlink;
 	}
 
 #ifdef HAVE_CTDB_WANT_READONLY_DECL
@@ -1647,13 +1649,11 @@ struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
 		if (!NT_STATUS_IS_OK(status) || (cstatus != 0)) {
 			DEBUG(1, ("CTDB_CONTROL_SET_DB_READONLY failed: "
 				  "%s, %d\n", nt_errstr(status), cstatus));
-			goto free_result;
+			goto lp_ctx_unlink;
 		}
 	}
 #endif
 
-	lp_ctx = loadparm_init_s3(db_path, loadparm_s3_helpers());
-
 	if (hash_size == 0) {
 		hash_size = lpcfg_tdb_hash_size(lp_ctx, db_path);
 	}
@@ -1661,10 +1661,9 @@ struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
 	db_ctdb->wtdb = tdb_wrap_open(db_ctdb, db_path, hash_size,
 				      lpcfg_tdb_flags(lp_ctx, tdb_flags),
 				      O_RDWR, 0);
-	talloc_unlink(db_path, lp_ctx);
 	if (db_ctdb->wtdb == NULL) {
 		DEBUG(0, ("Could not open tdb %s: %s\n", db_path, strerror(errno)));
-		goto free_result;
+		goto lp_ctx_unlink;
 	}
 	talloc_free(db_path);
 
@@ -1673,7 +1672,7 @@ struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
 						    ctdb_conn_msg_ctx(conn));
 		if (db_ctdb->lock_ctx == NULL) {
 			DEBUG(0, ("g_lock_ctx_init failed\n"));
-			goto free_result;
+			goto lp_ctx_unlink;
 		}
 	}
 
@@ -1701,6 +1700,9 @@ struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
 	DEBUG(3,("db_open_ctdb: opened database '%s' with dbid 0x%x\n",
 		 name, db_ctdb->db_id));
 
+lp_ctx_unlink:
+	talloc_unlink(db_path, lp_ctx);
+
 free_result:
 	TALLOC_FREE(result);
 	return result;
-- 
1.7.1


>From 7ed0e5018cdba4e0204d443389a3ab0f6bfab5c7 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Wed, 18 Jun 2014 16:19:00 -0700
Subject: [PATCH 3/9] dbwrap: Also check for mutex usage in dbwrap_local_open

Move check for mutex usage to helper function and call that for local
and clustered tdbs.
---
 lib/dbwrap/dbwrap.c              |   31 +++++++++++++++++++++++++++++++
 lib/dbwrap/dbwrap.h              |    2 ++
 lib/dbwrap/dbwrap_local_open.c   |    2 ++
 source3/lib/dbwrap/dbwrap_ctdb.c |    2 ++
 source3/lib/dbwrap/dbwrap_open.c |   19 -------------------
 5 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/lib/dbwrap/dbwrap.c b/lib/dbwrap/dbwrap.c
index d75c714..fc898d1 100644
--- a/lib/dbwrap/dbwrap.c
+++ b/lib/dbwrap/dbwrap.c
@@ -23,6 +23,7 @@
 #include "dbwrap/dbwrap.h"
 #include "dbwrap/dbwrap_private.h"
 #include "lib/util/util_tdb.h"
+#include "lib/param/param.h"
 
 /*
  * Fall back using fetch if no genuine exists operation is provided
@@ -468,3 +469,33 @@ const char *dbwrap_name(struct db_context *db)
 {
 	return db->name;
 }
+
+void dbwrap_check_mutex(struct loadparm_context *lp_ctx,
+			const char *name, int *tdb_flags)
+{
+	const char *base;
+	bool try_mutex = false;
+	struct loadparm_service *lp_srv;
+
+	if (!(*tdb_flags & TDB_CLEAR_IF_FIRST)) {
+		return;
+	}
+
+	base = strrchr_m(name, '/');
+	if (base != NULL) {
+		base += 1;
+	} else {
+		base = name;
+	}
+
+	lp_srv = lpcfg_default_service(lp_ctx);
+
+	try_mutex = lpcfg_parm_bool(lp_ctx, lp_srv, "dbwrap_tdb_mutexes", "*",
+				    try_mutex);
+	try_mutex = lpcfg_parm_bool(lp_ctx, lp_srv, "dbwrap_tdb_mutexes", base,
+				    try_mutex);
+
+	if (try_mutex && tdb_runtime_check_for_robust_mutexes()) {
+		*tdb_flags |= TDB_MUTEX_LOCKING;
+	}
+}
diff --git a/lib/dbwrap/dbwrap.h b/lib/dbwrap/dbwrap.h
index d289243..5aa5fbc 100644
--- a/lib/dbwrap/dbwrap.h
+++ b/lib/dbwrap/dbwrap.h
@@ -88,6 +88,8 @@ int dbwrap_transaction_cancel(struct db_context *db);
 void dbwrap_db_id(struct db_context *db, const uint8_t **id, size_t *idlen);
 bool dbwrap_is_persistent(struct db_context *db);
 const char *dbwrap_name(struct db_context *db);
+void dbwrap_check_mutex(struct loadparm_context *lp_ctx,
+			const char *name, int *tdb_flags);
 
 /* The following definitions come from lib/dbwrap_util.c  */
 
diff --git a/lib/dbwrap/dbwrap_local_open.c b/lib/dbwrap/dbwrap_local_open.c
index 6e40139..e7a5a26 100644
--- a/lib/dbwrap/dbwrap_local_open.c
+++ b/lib/dbwrap/dbwrap_local_open.c
@@ -169,6 +169,8 @@ struct db_context *dbwrap_local_open(TALLOC_CTX *mem_ctx,
 	const char *ntdbname, *tdbname;
 	struct db_context *db = NULL;
 
+	dbwrap_check_mutex(lp_ctx, name, &tdb_flags);
+
 	/* Get both .ntdb and .tdb variants of the name. */
 	if (!name) {
 		tdbname = ntdbname = "unnamed database";
diff --git a/source3/lib/dbwrap/dbwrap_ctdb.c b/source3/lib/dbwrap/dbwrap_ctdb.c
index b1b2036..5fdd91a 100644
--- a/source3/lib/dbwrap/dbwrap_ctdb.c
+++ b/source3/lib/dbwrap/dbwrap_ctdb.c
@@ -1602,6 +1602,8 @@ struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
 
 	lp_ctx = loadparm_init_s3(mem_ctx, loadparm_s3_helpers());
 
+	dbwrap_check_mutex(lp_ctx, name, &tdb_flags);
+
 	if (!NT_STATUS_IS_OK(ctdbd_db_attach(conn, name, &db_ctdb->db_id, tdb_flags))) {
 		DEBUG(0, ("ctdbd_db_attach failed for %s\n", name));
 		goto lp_ctx_unlink;
diff --git a/source3/lib/dbwrap/dbwrap_open.c b/source3/lib/dbwrap/dbwrap_open.c
index 64f484e..81f20b0 100644
--- a/source3/lib/dbwrap/dbwrap_open.c
+++ b/source3/lib/dbwrap/dbwrap_open.c
@@ -93,25 +93,6 @@ struct db_context *db_open(TALLOC_CTX *mem_ctx,
 		}
 	}
 
-	if (tdb_flags & TDB_CLEAR_IF_FIRST) {
-		const char *base;
-		bool try_mutex = false;
-
-		base = strrchr_m(name, '/');
-		if (base != NULL) {
-			base += 1;
-		} else {
-			base = name;
-		}
-
-		try_mutex = lp_parm_bool(-1, "dbwrap_tdb_mutexes", "*", try_mutex);
-		try_mutex = lp_parm_bool(-1, "dbwrap_tdb_mutexes", base, try_mutex);
-
-		if (try_mutex && tdb_runtime_check_for_robust_mutexes()) {
-			tdb_flags |= TDB_MUTEX_LOCKING;
-		}
-	}
-
 	sockname = lp_ctdbd_socket();
 
 	if (lp_clustering()) {
-- 
1.7.1


>From 2fa9355856a138e05dd65fd92dc87259168b62d1 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Fri, 6 Jun 2014 13:48:08 -0700
Subject: [PATCH 4/9] gencache: Make loadparm_context available in gencache_init

This will be used by dbwrap_local_open.
---
 source3/lib/gencache.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/source3/lib/gencache.c b/source3/lib/gencache.c
index 0fb1fd8..32c569a 100644
--- a/source3/lib/gencache.c
+++ b/source3/lib/gencache.c
@@ -26,6 +26,7 @@
 #include "system/glob.h"
 #include "util_tdb.h"
 #include "memcache.h"
+#include "lib/param/param.h"
 
 #undef  DBGC_CLASS
 #define DBGC_CLASS DBGC_TDB
@@ -60,10 +61,13 @@ static bool gencache_init(void)
 {
 	char* cache_fname = NULL;
 	int open_flags = O_RDWR|O_CREAT;
+	struct loadparm_context *lp_ctx;
 
 	/* skip file open if it's already opened */
 	if (cache) return True;
 
+	lp_ctx = loadparm_init_s3(NULL, loadparm_s3_helpers());
+
 	cache_fname = cache_path("gencache.tdb");
 
 	DEBUG(5, ("Opening cache file at %s\n", cache_fname));
@@ -104,7 +108,7 @@ static bool gencache_init(void)
 
 	if (!cache) {
 		DEBUG(5, ("Attempt to open gencache.tdb has failed.\n"));
-		return False;
+		goto out;
 	}
 
 	cache_fname = lock_path("gencache_notrans.tdb");
@@ -122,10 +126,11 @@ static bool gencache_init(void)
 			  strerror(errno)));
 		tdb_close(cache);
 		cache = NULL;
-		return false;
+		goto out;
 	}
-
-	return True;
+out:
+	talloc_unlink(NULL, lp_ctx);
+	return cache != NULL;
 }
 
 static TDB_DATA last_stabilize_key(void)
-- 
1.7.1


>From 8d7172fff9d67de56a6bc8e729a149e9d40de5dc Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Mon, 9 Jun 2014 12:16:25 -0700
Subject: [PATCH 5/9] gencache: Simplify return handling in gencache_set_data_blob

---
 source3/lib/gencache.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/source3/lib/gencache.c b/source3/lib/gencache.c
index 32c569a..f8dc987 100644
--- a/source3/lib/gencache.c
+++ b/source3/lib/gencache.c
@@ -322,7 +322,7 @@ bool gencache_set_data_blob(const char *keystr, const DATA_BLOB *blob,
 	if (writecount > lp_parm_int(-1, "gencache", "stabilize_count", 100)) {
 		gencache_stabilize();
 		writecount = 0;
-		goto done;
+		return true;
 	}
 
 	/*
@@ -343,8 +343,7 @@ bool gencache_set_data_blob(const char *keystr, const DATA_BLOB *blob,
 		gencache_stabilize();
 	}
 
-done:
-	return ret == 0;
+	return true;
 }
 
 /**
-- 
1.7.1


>From c22e9d745cf0334c1ac72cde992fd0a70c36af0c Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Mon, 9 Jun 2014 12:21:06 -0700
Subject: [PATCH 6/9] gencache: Always free data from stabilize record in gencache_set_data_blob

This is unlikely, but better be safe than to leak memory.
---
 source3/lib/gencache.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/source3/lib/gencache.c b/source3/lib/gencache.c
index f8dc987..1a60273 100644
--- a/source3/lib/gencache.c
+++ b/source3/lib/gencache.c
@@ -332,9 +332,10 @@ bool gencache_set_data_blob(const char *keystr, const DATA_BLOB *blob,
 
 	last_stabilize = 0;
 	databuf = tdb_fetch_compat(cache_notrans, last_stabilize_key());
-	if ((databuf.dptr != NULL)
-	    && (databuf.dptr[databuf.dsize-1] == '\0')) {
-		last_stabilize = atoi((char *)databuf.dptr);
+	if (databuf.dptr != NULL) {
+		if (databuf.dptr[databuf.dsize-1] == '\0') {
+			last_stabilize = atoi((char *)databuf.dptr);
+		}
 		SAFE_FREE(databuf.dptr);
 	}
 	if ((last_stabilize
-- 
1.7.1


>From 50b8bbc60b37422f85a01068ea2e3c1ed890523d Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Fri, 13 Jun 2014 10:41:23 -0700
Subject: [PATCH 7/9] gencache: Convert to dbwrap interface

---
 source3/lib/gencache.c |  196 ++++++++++++++++++++++++++----------------------
 1 files changed, 106 insertions(+), 90 deletions(-)

diff --git a/source3/lib/gencache.c b/source3/lib/gencache.c
index 1a60273..5d46451 100644
--- a/source3/lib/gencache.c
+++ b/source3/lib/gencache.c
@@ -25,6 +25,7 @@
 #include "system/filesys.h"
 #include "system/glob.h"
 #include "util_tdb.h"
+#include "dbwrap/dbwrap.h"
 #include "memcache.h"
 #include "lib/param/param.h"
 
@@ -37,8 +38,8 @@
 #define BLOB_TYPE "DATA_BLOB"
 #define BLOB_TYPE_LEN 9
 
-static struct tdb_context *cache;
-static struct tdb_context *cache_notrans;
+static struct db_context *cache;
+static struct db_context *cache_notrans;
 static int cache_notrans_seqnum;
 
 /**
@@ -72,12 +73,15 @@ static bool gencache_init(void)
 
 	DEBUG(5, ("Opening cache file at %s\n", cache_fname));
 
-	cache = tdb_open_log(cache_fname, 0, TDB_DEFAULT|TDB_INCOMPATIBLE_HASH, open_flags, 0644);
+	cache = dbwrap_local_open(NULL, lp_ctx, cache_fname, 0,
+				  TDB_DEFAULT|TDB_INCOMPATIBLE_HASH, open_flags,
+				  0644, DBWRAP_LOCK_ORDER_NONE,
+				  DBWRAP_FLAG_NONE);
 	if (cache) {
 		int ret;
-		ret = tdb_check(cache, NULL, NULL);
+		ret = dbwrap_check(cache);
 		if (ret != 0) {
-			tdb_close(cache);
+			TALLOC_FREE(cache);
 
 			/*
 			 * Retry with CLEAR_IF_FIRST.
@@ -89,18 +93,23 @@ static bool gencache_init(void)
 			 * CLEAR_IF_FIRST databases, so lets use it here to
 			 * clean up a broken database.
 			 */
-			cache = tdb_open_log(cache_fname, 0,
-					     TDB_DEFAULT|
-					     TDB_INCOMPATIBLE_HASH|
-					     TDB_CLEAR_IF_FIRST,
-					     open_flags, 0644);
+			cache = dbwrap_local_open(NULL, lp_ctx, cache_fname, 0,
+						  TDB_DEFAULT|
+						  TDB_INCOMPATIBLE_HASH|
+						  TDB_CLEAR_IF_FIRST,
+						  open_flags, 0644,
+						  DBWRAP_LOCK_ORDER_NONE,
+						  DBWRAP_FLAG_NONE);
 		}
 	}
 
 	if (!cache && (errno == EACCES)) {
 		open_flags = O_RDONLY;
-		cache = tdb_open_log(cache_fname, 0, TDB_DEFAULT|TDB_INCOMPATIBLE_HASH, open_flags,
-				     0644);
+		cache = dbwrap_local_open(NULL, lp_ctx, cache_fname, 0,
+					  TDB_DEFAULT|TDB_INCOMPATIBLE_HASH,
+					  open_flags, 0644,
+					  DBWRAP_LOCK_ORDER_NONE,
+					  DBWRAP_FLAG_NONE);
 		if (cache) {
 			DEBUG(5, ("gencache_init: Opening cache file %s read-only.\n", cache_fname));
 		}
@@ -115,17 +124,18 @@ static bool gencache_init(void)
 
 	DEBUG(5, ("Opening cache file at %s\n", cache_fname));
 
-	cache_notrans = tdb_open_log(cache_fname, 0,
-				     TDB_CLEAR_IF_FIRST|
-				     TDB_INCOMPATIBLE_HASH|
-				     TDB_SEQNUM|
-				     TDB_NOSYNC,
-				     open_flags, 0644);
+	cache_notrans = dbwrap_local_open(NULL, lp_ctx, cache_fname, 0,
+					  TDB_CLEAR_IF_FIRST|
+					  TDB_INCOMPATIBLE_HASH|
+					  TDB_SEQNUM|
+					  TDB_NOSYNC,
+					  open_flags, 0644,
+					  DBWRAP_LOCK_ORDER_NONE,
+					  DBWRAP_FLAG_NONE);
 	if (cache_notrans == NULL) {
 		DEBUG(5, ("Opening %s failed: %s\n", cache_fname,
 			  strerror(errno)));
-		tdb_close(cache);
-		cache = NULL;
+		TALLOC_FREE(cache);
 		goto out;
 	}
 out:
@@ -260,7 +270,7 @@ static bool gencache_have_val(const char *keystr, const DATA_BLOB *data,
 bool gencache_set_data_blob(const char *keystr, const DATA_BLOB *blob,
 			    time_t timeout)
 {
-	int ret;
+	NTSTATUS status;
 	TDB_DATA databuf;
 	char* val;
 	time_t last_stabilize;
@@ -303,13 +313,13 @@ bool gencache_set_data_blob(const char *keystr, const DATA_BLOB *blob,
 		   (int)(timeout - time(NULL)), 
 		   timeout > time(NULL) ? "ahead" : "in the past"));
 
-	ret = tdb_store_bystring(
-		cache_notrans, keystr,
-		make_tdb_data((uint8_t *)val, talloc_array_length(val)),
-		0);
+	status = dbwrap_store_bystring(cache_notrans, keystr,
+				       make_tdb_data((uint8_t *)val,
+						     talloc_array_length(val)),
+				       0);
 	TALLOC_FREE(val);
 
-	if (ret != 0) {
+	if (!NT_STATUS_IS_OK(status)) {
 		return false;
 	}
 
@@ -331,12 +341,13 @@ bool gencache_set_data_blob(const char *keystr, const DATA_BLOB *blob,
 	 */
 
 	last_stabilize = 0;
-	databuf = tdb_fetch_compat(cache_notrans, last_stabilize_key());
-	if (databuf.dptr != NULL) {
+	status = dbwrap_fetch(cache_notrans, talloc_tos(),
+			      last_stabilize_key(), &databuf);
+	if (NT_STATUS_IS_OK(status)) {
 		if (databuf.dptr[databuf.dsize-1] == '\0') {
 			last_stabilize = atoi((char *)databuf.dptr);
 		}
-		SAFE_FREE(databuf.dptr);
+		TALLOC_FREE(databuf.dptr);
 	}
 	if ((last_stabilize
 	     + lp_parm_int(-1, "gencache", "stabilize_interval", 300))
@@ -424,7 +435,7 @@ struct gencache_parse_state {
 	bool is_memcache;
 };
 
-static int gencache_parse_fn(TDB_DATA key, TDB_DATA data, void *private_data)
+static void gencache_parse_fn(TDB_DATA key, TDB_DATA data, void *private_data)
 {
 	struct gencache_parse_state *state;
 	DATA_BLOB blob;
@@ -433,11 +444,11 @@ static int gencache_parse_fn(TDB_DATA key, TDB_DATA data, void *private_data)
 	bool ret;
 
 	if (data.dptr == NULL) {
-		return -1;
+		return;
 	}
 	ret = gencache_pull_timeout((char *)data.dptr, &t, &endptr);
 	if (!ret) {
-		return -1;
+		return;
 	}
 	state = (struct gencache_parse_state *)private_data;
 	blob = data_blob_const(
@@ -449,8 +460,6 @@ static int gencache_parse_fn(TDB_DATA key, TDB_DATA data, void *private_data)
 			     data_blob_const(key.dptr, key.dsize),
 			     data_blob_const(data.dptr, data.dsize));
 	}
-
-	return 0;
 }
 
 bool gencache_parse(const char *keystr,
@@ -461,7 +470,7 @@ bool gencache_parse(const char *keystr,
 	struct gencache_parse_state state;
 	TDB_DATA key = string_term_tdb_data(keystr);
 	DATA_BLOB memcache_val;
-	int ret;
+	NTSTATUS status;
 
 	if (keystr == NULL) {
 		return false;
@@ -483,7 +492,7 @@ bool gencache_parse(const char *keystr,
 		 * Make sure that nobody has changed the gencache behind our
 		 * back.
 		 */
-		int current_seqnum = tdb_get_seqnum(cache_notrans);
+		int current_seqnum = dbwrap_get_seqnum(cache_notrans);
 		if (current_seqnum == cache_notrans_seqnum) {
 			/*
 			 * Ok, our memcache is still current, use it without
@@ -501,12 +510,17 @@ bool gencache_parse(const char *keystr,
 
 	state.is_memcache = false;
 
-	ret = tdb_parse_record(cache_notrans, key, gencache_parse_fn, &state);
-	if (ret == 0) {
+	status = dbwrap_parse_record(cache_notrans, key, gencache_parse_fn,
+				     &state);
+	if (NT_STATUS_IS_OK(status)) {
 		return true;
 	}
-	ret = tdb_parse_record(cache, key, gencache_parse_fn, &state);
-	return (ret == 0);
+	status = dbwrap_parse_record(cache, key, gencache_parse_fn, &state);
+	if (NT_STATUS_IS_OK(status)) {
+		return true;
+	}
+
+	return false;
 }
 
 struct gencache_get_data_blob_state {
@@ -602,8 +616,7 @@ struct stabilize_state {
 	bool written;
 	bool error;
 };
-static int stabilize_fn(struct tdb_context *tdb, TDB_DATA key, TDB_DATA val,
-			void *priv);
+static int stabilize_fn(struct db_record *rec, void *priv);
 
 /**
  * Stabilize gencache
@@ -615,6 +628,7 @@ static int stabilize_fn(struct tdb_context *tdb, TDB_DATA key, TDB_DATA val,
 bool gencache_stabilize(void)
 {
 	struct stabilize_state state;
+	NTSTATUS status;
 	int res;
 	char *now;
 
@@ -622,77 +636,77 @@ bool gencache_stabilize(void)
 		return false;
 	}
 
-	res = tdb_transaction_start_nonblock(cache);
-	if (res != 0) {
-		if (tdb_error(cache) == TDB_ERR_NOLOCK)
-		{
-			/*
-			 * Someone else already does the stabilize,
-			 * this does not have to be done twice
-			 */
-			return true;
-		}
+	status = dbwrap_transaction_start_nonblock(cache);
+	if (NT_STATUS_EQUAL(status, NT_STATUS_FILE_LOCK_CONFLICT)) {
+		/*
+		 * Someone else already does the stabilize,
+		 * this does not have to be done twice
+		 */
+		return true;
+	}
 
+	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(10, ("Could not start transaction on gencache.tdb: "
-			   "%s\n", tdb_errorstr_compat(cache)));
+			   "%s\n", nt_errstr(status)));
 		return false;
 	}
-	res = tdb_transaction_start(cache_notrans);
-	if (res != 0) {
-		tdb_transaction_cancel(cache);
+	res = dbwrap_transaction_start(cache_notrans);
+	if (!NT_STATUS_IS_OK(status)) {
+		dbwrap_transaction_cancel(cache);
 		DEBUG(10, ("Could not start transaction on "
-			   "gencache_notrans.tdb: %s\n",
-			   tdb_errorstr_compat(cache_notrans)));
+			   "gencache_notrans.tdb\n"));
 		return false;
 	}
 
 	state.error = false;
 	state.written = false;
 
-	res = tdb_traverse(cache_notrans, stabilize_fn, &state);
-	if ((res < 0) || state.error) {
-		tdb_transaction_cancel(cache_notrans);
-		tdb_transaction_cancel(cache);
+	status = dbwrap_traverse(cache_notrans, stabilize_fn, &state, 0);
+
+	if (!NT_STATUS_IS_OK(status) || state.error) {
+		dbwrap_transaction_cancel(cache_notrans);
+		dbwrap_transaction_cancel(cache);
 		return false;
 	}
 
 	if (!state.written) {
-		tdb_transaction_cancel(cache_notrans);
-		tdb_transaction_cancel(cache);
+		dbwrap_transaction_cancel(cache_notrans);
+		dbwrap_transaction_cancel(cache);
 		return true;
 	}
 
-	res = tdb_transaction_commit(cache);
+	res = dbwrap_transaction_commit(cache);
 	if (res != 0) {
-		DEBUG(10, ("tdb_transaction_commit on gencache.tdb failed: "
-			   "%s\n", tdb_errorstr_compat(cache)));
-		tdb_transaction_cancel(cache_notrans);
+		DEBUG(10, ("dbwrap_transaction_commit on gencache.tdb "
+			   "failed\n"));
+		dbwrap_transaction_cancel(cache_notrans);
 		return false;
 	}
 
-	res = tdb_transaction_commit(cache_notrans);
+	res = dbwrap_transaction_commit(cache_notrans);
 	if (res != 0) {
-		DEBUG(10, ("tdb_transaction_commit on gencache.tdb failed: "
-			   "%s\n", tdb_errorstr_compat(cache)));
+		DEBUG(10, ("dbwrap_transaction_commit on gencache_notrans.tdb "
+			   "failed\n"));
 		return false;
 	}
 
 	now = talloc_asprintf(talloc_tos(), "%d", (int)time(NULL));
 	if (now != NULL) {
-		tdb_store(cache_notrans, last_stabilize_key(),
-			  string_term_tdb_data(now), 0);
+		dbwrap_store(cache_notrans, last_stabilize_key(),
+			     string_term_tdb_data(now), 0);
 		TALLOC_FREE(now);
 	}
 
 	return true;
 }
 
-static int stabilize_fn(struct tdb_context *tdb, TDB_DATA key, TDB_DATA val,
-			void *priv)
+static int stabilize_fn(struct db_record *rec, void *priv)
 {
 	struct stabilize_state *state = (struct stabilize_state *)priv;
-	int res;
+	NTSTATUS status;
 	time_t timeout;
+	TDB_DATA key = dbwrap_record_get_key(rec);
+	TDB_DATA val = dbwrap_record_get_value(rec);
 
 	if (tdb_data_cmp(key, last_stabilize_key()) == 0) {
 		return 0;
@@ -703,29 +717,30 @@ static int stabilize_fn(struct tdb_context *tdb, TDB_DATA key, TDB_DATA val,
 		return 0;
 	}
 	if ((timeout < time(NULL)) || (val.dsize == 0)) {
-		res = tdb_delete(cache, key);
-		if ((res != 0) && (tdb_error(cache) == TDB_ERR_NOEXIST)) {
-			res = 0;
+		status = dbwrap_delete(cache, key);
+		if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
+			status = NT_STATUS_OK;
 		} else {
 			state->written = true;
 		}
 	} else {
-		res = tdb_store(cache, key, val, 0);
-		if (res == 0) {
+		status = dbwrap_store(cache, key, val, 0);
+		if (NT_STATUS_IS_OK(status)) {
 			state->written = true;
 		}
 	}
 
-	if (res != 0) {
+	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(10, ("Transfer to gencache.tdb failed: %s\n",
-			   tdb_errorstr_compat(cache)));
+			   nt_errstr(status)));
 		state->error = true;
 		return -1;
 	}
 
-	if (tdb_delete(cache_notrans, key) != 0) {
+	status = dbwrap_delete(cache_notrans, key);
+	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(10, ("tdb_delete from gencache_notrans.tdb failed: "
-			   "%s\n", tdb_errorstr_compat(cache_notrans)));
+			   "%s\n", nt_errstr(status)));
 		state->error = true;
 		return -1;
 	}
@@ -802,8 +817,7 @@ struct gencache_iterate_blobs_state {
 	bool in_persistent;
 };
 
-static int gencache_iterate_blobs_fn(struct tdb_context *tdb, TDB_DATA key,
-				     TDB_DATA data, void *priv)
+static int gencache_iterate_blobs_fn(struct db_record *rec, void *priv)
 {
 	struct gencache_iterate_blobs_state *state =
 		(struct gencache_iterate_blobs_state *)priv;
@@ -811,11 +825,13 @@ static int gencache_iterate_blobs_fn(struct tdb_context *tdb, TDB_DATA key,
 	char *free_key = NULL;
 	time_t timeout;
 	char *endptr;
+	TDB_DATA key = dbwrap_record_get_key(rec);
+	TDB_DATA data = dbwrap_record_get_value(rec);
 
 	if (tdb_data_cmp(key, last_stabilize_key()) == 0) {
 		return 0;
 	}
-	if (state->in_persistent && tdb_exists(cache_notrans, key)) {
+	if (state->in_persistent && dbwrap_exists(cache_notrans, key)) {
 		return 0;
 	}
 
@@ -870,10 +886,10 @@ void gencache_iterate_blobs(void (*fn)(const char *key, DATA_BLOB value,
 	state.private_data = private_data;
 
 	state.in_persistent = false;
-	tdb_traverse(cache_notrans, gencache_iterate_blobs_fn, &state);
+	dbwrap_traverse(cache_notrans, gencache_iterate_blobs_fn, &state, NULL);
 
 	state.in_persistent = true;
-	tdb_traverse(cache, gencache_iterate_blobs_fn, &state);
+	dbwrap_traverse(cache, gencache_iterate_blobs_fn, &state, NULL);
 }
 
 /**
-- 
1.7.1


>From 107d0bc1f00797440617c9ddd167bf6a2ab829b1 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 19 Jun 2014 22:22:54 -0700
Subject: [PATCH 8/9] param: Disable mutexes in tdbs when mmap is disabled

---
 lib/param/loadparm.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
index 5a0ef88..7e31f89 100644
--- a/lib/param/loadparm.c
+++ b/lib/param/loadparm.c
@@ -2982,6 +2982,7 @@ int lpcfg_tdb_flags(struct loadparm_context *lp_ctx, int tdb_flags)
 {
 	if (!lpcfg_use_mmap(lp_ctx)) {
 		tdb_flags |= TDB_NOMMAP;
+		tdb_flags &= TDB_MUTEX_LOCKING;
 	}
 	return tdb_flags;
 }
-- 
1.7.1


>From 098727112b3aefa5533f0b73d068ae50827f6839 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 19 Jun 2014 22:23:53 -0700
Subject: [PATCH 9/9] dbwrap: Remove limitation of tdb flags

---
 source3/lib/dbwrap/dbwrap_ctdb.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/source3/lib/dbwrap/dbwrap_ctdb.c b/source3/lib/dbwrap/dbwrap_ctdb.c
index 5fdd91a..f4da1d6 100644
--- a/source3/lib/dbwrap/dbwrap_ctdb.c
+++ b/source3/lib/dbwrap/dbwrap_ctdb.c
@@ -1614,9 +1614,6 @@ struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
 	result->persistent = ((tdb_flags & TDB_CLEAR_IF_FIRST) == 0);
 	result->lock_order = lock_order;
 
-	/* only pass through specific flags */
-	tdb_flags &= TDB_SEQNUM|TDB_VOLATILE;
-
 	/* honor permissions if user has specified O_CREAT */
 	if (open_flags & O_CREAT) {
 		chmod(db_path, mode);
-- 
1.7.1



More information about the samba-technical mailing list