[PATCH 4/7] dbwrap: allow transactions on non-persistent non-clustered dbs.

Rusty Russell rusty at rustcorp.com.au
Wed Mar 27 18:21:57 MDT 2013


"Stefan (metze) Metzmacher" <metze at samba.org> writes:
> Hi Rusty,
>
>> Otherwise we can't use dbwrap for schannel_store:
>> 
>> 	transactions not supported on non-persistent database /home/rusty/samba/st/dc/private/schannel_store.tdb
>> 
>> Note that the source3/lib/dbwrap/dbwrap_ctdb.c transaction start will
>> return an error with the same message anyway.
>
> I think we should avoid using transactions instead of removing the check!

I was told to use dbwrap for the ntdb switch, but that means making
dbwrap a generic wrapper, used for non-clustered databases.  So I think
we'll hit the same issue as the other databases are converted to dbwrap.

(Volker pointed out on IRC that I should have checked git annotate
before looking at this change... indeed!  My mistake.)

Basically, if it comes from db_open() w/ CLEAR_IF_FIRST, we should make
sure they don't try to do transactions (which would fail if they *were*
using ctdb).  But direct calls to dbwrap_local_open() are OK.

Something like this (untested)?

diff --git a/lib/dbwrap/dbwrap.c b/lib/dbwrap/dbwrap.c
index f0b7a9a..b160fb8 100644
--- a/lib/dbwrap/dbwrap.c
+++ b/lib/dbwrap/dbwrap.c
@@ -452,7 +452,7 @@ int dbwrap_get_seqnum(struct db_context *db)
 
 int dbwrap_transaction_start(struct db_context *db)
 {
-	if (!db->persistent) {
+	if (!db->allow_transactions) {
 		DEBUG(1, ("transactions not supported on non-persistent "
 			  "database %s\n", db->name));
 		return -1;
@@ -494,3 +494,8 @@ const char *dbwrap_name(struct db_context *db)
 {
 	return db->name;
 }
+
+void dbwrap_no_transactions(struct db_context *db)
+{
+	db->allow_transactions = false;
+}
diff --git a/lib/dbwrap/dbwrap.h b/lib/dbwrap/dbwrap.h
index e394296..d6aaded 100644
--- a/lib/dbwrap/dbwrap.h
+++ b/lib/dbwrap/dbwrap.h
@@ -164,4 +164,6 @@ struct db_context *dbwrap_local_open(TALLOC_CTX *mem_ctx,
 				     int open_flags, mode_t mode,
 				     enum dbwrap_lock_order lock_order);
 
+/* Make sure that noone tries to do a transaction on this database. */
+void dbwrap_no_transactions(struct db_context *db);
 #endif /* __DBWRAP_H__ */
diff --git a/lib/dbwrap/dbwrap_cache.c b/lib/dbwrap/dbwrap_cache.c
index d97242e..dc7697c 100644
--- a/lib/dbwrap/dbwrap_cache.c
+++ b/lib/dbwrap/dbwrap_cache.c
@@ -210,5 +210,6 @@ struct db_context *db_open_cache(TALLOC_CTX *mem_ctx,
 	db->id = dbwrap_cache_id;
 	db->name = dbwrap_name(ctx->backing);
 	db->hash_size = dbwrap_hash_size(ctx->backing);
+	db->allow_transactions = true;
 	return db;
 }
diff --git a/lib/dbwrap/dbwrap_file.c b/lib/dbwrap/dbwrap_file.c
index a3b1737..a4e3765 100644
--- a/lib/dbwrap/dbwrap_file.c
+++ b/lib/dbwrap/dbwrap_file.c
@@ -371,6 +371,7 @@ struct db_context *db_open_file(TALLOC_CTX *mem_ctx,
 	result->traverse = db_file_traverse;
 	result->traverse_read = db_file_traverse;
 	result->persistent = ((tdb_flags & TDB_CLEAR_IF_FIRST) == 0);
+	result->allow_transactions = true;
 	result->hash_size = 0;
 	result->name = talloc_strdup(result, name);
 	if (result->name == NULL) {
diff --git a/lib/dbwrap/dbwrap_ntdb.c b/lib/dbwrap/dbwrap_ntdb.c
index 5be7b84..2ba1058 100644
--- a/lib/dbwrap/dbwrap_ntdb.c
+++ b/lib/dbwrap/dbwrap_ntdb.c
@@ -668,6 +668,7 @@ struct db_context *db_open_ntdb(TALLOC_CTX *mem_ctx,
 	result->parse_record = db_ntdb_parse;
 	result->get_seqnum = db_ntdb_get_seqnum;
 	result->persistent = ((ntdb_flags & NTDB_CLEAR_IF_FIRST) == 0);
+	result->allow_transactions = true;
 	result->transaction_start = db_ntdb_transaction_start;
 	result->transaction_start_nonblock = db_ntdb_transaction_start_nonblock;
 	result->transaction_commit = db_ntdb_transaction_commit;
diff --git a/lib/dbwrap/dbwrap_private.h b/lib/dbwrap/dbwrap_private.h
index d49a568..a13e482 100644
--- a/lib/dbwrap/dbwrap_private.h
+++ b/lib/dbwrap/dbwrap_private.h
@@ -68,6 +68,7 @@ struct db_context {
 	void *private_data;
 	enum dbwrap_lock_order lock_order;
 	bool persistent;
+	bool allow_transactions;
 	void (*stored_callback)(struct db_context *db, struct db_record *rec,
 				void *private_data);
 	void *stored_callback_private_data;
diff --git a/lib/dbwrap/dbwrap_rbt.c b/lib/dbwrap/dbwrap_rbt.c
index 3f97086..d435572 100644
--- a/lib/dbwrap/dbwrap_rbt.c
+++ b/lib/dbwrap/dbwrap_rbt.c
@@ -497,6 +497,7 @@ struct db_context *db_open_rbt(TALLOC_CTX *mem_ctx)
 	result->traverse = db_rbt_traverse;
 	result->traverse_read = db_rbt_traverse_read;
 	result->get_seqnum = db_rbt_get_seqnum;
+	result->allow_transactions = true;
 	result->transaction_start = db_rbt_trans_dummy;
 	result->transaction_commit = db_rbt_trans_dummy;
 	result->transaction_cancel = db_rbt_trans_dummy;
diff --git a/lib/dbwrap/dbwrap_tdb.c b/lib/dbwrap/dbwrap_tdb.c
index b62dcdf..37b1acd 100644
--- a/lib/dbwrap/dbwrap_tdb.c
+++ b/lib/dbwrap/dbwrap_tdb.c
@@ -465,6 +465,7 @@ struct db_context *db_open_tdb(TALLOC_CTX *mem_ctx,
 	result->parse_record = db_tdb_parse;
 	result->get_seqnum = db_tdb_get_seqnum;
 	result->persistent = ((tdb_flags & TDB_CLEAR_IF_FIRST) == 0);
+	result->allow_transactions = true;
 	result->transaction_start = db_tdb_transaction_start;
 	result->transaction_start_nonblock = db_tdb_transaction_start_nonblock;
 	result->transaction_commit = db_tdb_transaction_commit;
diff --git a/source3/lib/dbwrap/dbwrap_open.c b/source3/lib/dbwrap/dbwrap_open.c
index 515b4bf..9bbc345 100644
--- a/source3/lib/dbwrap/dbwrap_open.c
+++ b/source3/lib/dbwrap/dbwrap_open.c
@@ -124,5 +124,10 @@ struct db_context *db_open(TALLOC_CTX *mem_ctx,
 					   lock_order);
 		talloc_unlink(mem_ctx, lp_ctx);
 	}
+
+	/* Catch transactions on non-persistent dbs, even if no CTDB support */
+	if (result && (tdb_flags & TDB_CLEAR_IF_FIRST)) {
+		dbwrap_no_transactions(result);
+	}
 	return result;
 }


More information about the samba-technical mailing list