[SCM] CTDB repository - branch 1.0.112 updated - ctdb-1.0.111-18-g04e40de

[Ronnie Sahlberg]

The branch, 1.0.112 has been updated
       via  04e40deac8d0c7edf907135ae81ac961c23135c3 (commit)
       via  72d6ae64ab5ece7645b02054d617b71e231d4741 (commit)
      from  3e2b1839a9f8419eeeb7f22ea5925f6c42f32a65 (commit)

http://gitweb.samba.org/?p=sahlberg/ctdb.git;a=shortlog;h=1.0.112


- Log -----------------------------------------------------------------
commit 04e40deac8d0c7edf907135ae81ac961c23135c3
Author: Ronnie Sahlberg <ronniesahlberg at gmail.com>
Date:   Tue Feb 2 08:03:37 2010 +1100

    Version 1.0.112-5

commit 72d6ae64ab5ece7645b02054d617b71e231d4741
Author: Volker Lendecke <vl at samba.org>
Date:   Fri Jan 29 18:21:09 2010 +0100

    tdb: fix an early release of the global lock that can cause data corruption
    
    There was a bug in tdb where the
    
                    tdb_brlock(tdb, GLOBAL_LOCK, F_UNLCK, F_SETLKW, 0, 1);
    
    (ending the transaction-"mutex") was done before the
    
                            /* remove the recovery marker */
    
    This means that when a transaction is committed there is a window where another
    opener of the file sees the transaction marker while the transaction committer
    is still fully functional and working on it. This led to transaction being
    rolled back by that second opener of the file while transaction_commit() gave
    no error to the caller.
    
    This patch moves the F_UNLCK to after the recovery marker was removed, closing
    this window.

-----------------------------------------------------------------------

Summary of changes:
 lib/tdb/common/transaction.c |   15 ++++++++++-----
 packaging/RPM/ctdb.spec.in   |    4 +++-
 2 files changed, 13 insertions(+), 6 deletions(-)


Changeset truncated at 500 lines:

diff --git a/lib/tdb/common/transaction.c b/lib/tdb/common/transaction.c
index 20f2bfc..b8988ea 100644
--- a/lib/tdb/common/transaction.c
+++ b/lib/tdb/common/transaction.c
@@ -135,6 +135,9 @@ struct tdb_transaction {
 	bool prepared;
 	tdb_off_t magic_offset;
 
+	/* set when the GLOBAL_LOCK has been taken */
+	bool global_lock_taken;
+
 	/* old file size before transaction */
 	tdb_len_t old_map_size;
 
@@ -603,6 +606,11 @@ int _tdb_transaction_cancel(struct tdb_context *tdb)
 		}
 	}
 
+	if (tdb->transaction->global_lock_taken) {
+		tdb_brlock(tdb, GLOBAL_LOCK, F_UNLCK, F_SETLKW, 0, 1);
+		tdb->transaction->global_lock_taken = false;
+	}
+
 	/* remove any global lock created during the transaction */
 	if (tdb->global_lock.count != 0) {
 		tdb_brlock(tdb, FREELIST_TOP, F_UNLCK, F_SETLKW, 0, 4*tdb->header.hash_size);
@@ -947,11 +955,12 @@ static int _tdb_transaction_prepare_commit(struct tdb_context *tdb)
 		return -1;
 	}
 
+	tdb->transaction->global_lock_taken = true;
+
 	if (!(tdb->flags & TDB_NOSYNC)) {
 		/* write the recovery data to the end of the file */
 		if (transaction_setup_recovery(tdb, &tdb->transaction->magic_offset) == -1) {
 			TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_transaction_prepare_commit: failed to setup recovery data\n"));
-			tdb_brlock(tdb, GLOBAL_LOCK, F_UNLCK, F_SETLKW, 0, 1);
 			_tdb_transaction_cancel(tdb);
 			return -1;
 		}
@@ -966,7 +975,6 @@ static int _tdb_transaction_prepare_commit(struct tdb_context *tdb)
 					     tdb->transaction->old_map_size) == -1) {
 			tdb->ecode = TDB_ERR_IO;
 			TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_transaction_prepare_commit: expansion failed\n"));
-			tdb_brlock(tdb, GLOBAL_LOCK, F_UNLCK, F_SETLKW, 0, 1);
 			_tdb_transaction_cancel(tdb);
 			return -1;
 		}
@@ -1056,7 +1064,6 @@ int tdb_transaction_commit(struct tdb_context *tdb)
 			tdb_transaction_recover(tdb); 
 
 			_tdb_transaction_cancel(tdb);
-			tdb_brlock(tdb, GLOBAL_LOCK, F_UNLCK, F_SETLKW, 0, 1);
 
 			TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_transaction_commit: write failed\n"));
 			return -1;
@@ -1072,8 +1079,6 @@ int tdb_transaction_commit(struct tdb_context *tdb)
 		return -1;
 	}
 
-	tdb_brlock(tdb, GLOBAL_LOCK, F_UNLCK, F_SETLKW, 0, 1);
-
 	/*
 	  TODO: maybe write to some dummy hdr field, or write to magic
 	  offset without mmap, before the last sync, instead of the
diff --git a/packaging/RPM/ctdb.spec.in b/packaging/RPM/ctdb.spec.in
index 93f5e22..9b74d49 100644
--- a/packaging/RPM/ctdb.spec.in
+++ b/packaging/RPM/ctdb.spec.in
@@ -5,7 +5,7 @@ Vendor: Samba Team
 Packager: Samba Team <samba at samba.org>
 Name: ctdb
 Version: 1.0.112
-Release: 4
+Release: 5
 Epoch: 0
 License: GNU GPL version 3
 Group: System Environment/Daemons
@@ -123,6 +123,8 @@ rm -rf $RPM_BUILD_ROOT
 %{_docdir}/ctdb/tests/bin/ctdb_transaction
 
 %changelog
+* Tue Feb 2 2010 : Version 1.0.112-5
+ - Patch from Volker to fix a TDB transaction rolling back issue.
 * Wed Jan 21 2010 : Version 1.0.112-4
  - Update onnode with more flexible ways to define the path to the nodes file
 * Wed Jan 20 2010 : Version 1.0.112-3


-- 
CTDB repository