tdb: use tdb_nest_lock() for open lock.

This never nests, so it's overkill, but it centralizes the locking into
lock.c and removes the ugly flag in the transaction code to track whether
we have the lock or not.

Note that we have a temporary hack so this places a real lock, despite
the fact that we are in a transaction.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

ccan/tdb/lock.c

@@ -683,7 +683,8 @@ bool tdb_have_extra_locks(struct tdb_context *tdb)
 	return extra;
 }
 
-/* The transaction code uses this to remove all locks. */
+/* The transaction code uses this to remove all locks.  Note that this
+   may include OPEN_LOCK. */
 void tdb_release_extra_locks(struct tdb_context *tdb)
 {
 	unsigned int i, extra = 0;

ccan/tdb/open.c

@@ -240,7 +240,7 @@ struct tdb_context *tdb_open_ex(const char *name, int hash_size, int tdb_flags,
         fcntl(tdb->fd, F_SETFD, v | FD_CLOEXEC);
 
 	/* ensure there is only one process initialising at once */
-	if (tdb->methods->brlock(tdb, F_WRLCK, OPEN_LOCK, 1, TDB_LOCK_WAIT) == -1) {
+	if (tdb_nest_lock(tdb, OPEN_LOCK, F_WRLCK, TDB_LOCK_WAIT) == -1) {
 		TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_open_ex: failed to get open lock on %s: %s\n",
 			 name, strerror(errno)));
 		goto fail;	/* errno set by tdb_brlock */
@@ -356,8 +356,9 @@ struct tdb_context *tdb_open_ex(const char *name, int hash_size, int tdb_flags,
 	/* Internal (memory-only) databases skip all the code above to
 	 * do with disk files, and resume here by releasing their
 	 * open lock and hooking into the active list. */
-	if (tdb->methods->brunlock(tdb, F_WRLCK, OPEN_LOCK, 1) == -1)
+	if (tdb_nest_unlock(tdb, OPEN_LOCK, F_WRLCK, false) == -1) {
 		goto fail;
+	}
 	tdb->next = tdbs;
 	tdbs = tdb;
 	return tdb;

ccan/tdb/transaction.c

@@ -135,9 +135,6 @@ struct tdb_transaction {
 	bool prepared;
 	tdb_off_t magic_offset;
 
-	/* set when the OPEN_LOCK has been taken */
-	bool open_lock_taken;
-
 	/* old file size before transaction */
 	tdb_len_t old_map_size;
 
@@ -418,6 +415,9 @@ static int transaction_brlock(struct tdb_context *tdb,
 			      int rw_type, tdb_off_t offset, size_t len,
 			      enum tdb_lock_flags flags)
 {
+	/* FIXME: We actually grab the open lock during a transaction. */
+	if (offset == OPEN_LOCK)
+		return tdb_brlock(tdb, rw_type, offset, len, flags);
 	return 0;
 }
 
@@ -504,11 +504,7 @@ int _tdb_transaction_cancel(struct tdb_context *tdb, int ltype)
 		}
 	}
 
-	if (tdb->transaction->open_lock_taken) {
-		tdb_brunlock(tdb, F_WRLCK, OPEN_LOCK, 1);
-		tdb->transaction->open_lock_taken = false;
-	}
-
+	/* This also removes the OPEN_LOCK, if we have it. */
 	tdb_release_extra_locks(tdb);
 
 	/* restore the normal io methods */
@@ -945,15 +941,12 @@ static int _tdb_transaction_prepare_commit(struct tdb_context *tdb)
 
 	/* get the open lock - this prevents new users attaching to the database
 	   during the commit */
-	if (tdb_brlock(tdb, F_WRLCK, OPEN_LOCK, 1, TDB_LOCK_WAIT) == -1) {
+	if (tdb_nest_lock(tdb, OPEN_LOCK, F_WRLCK, TDB_LOCK_WAIT) == -1) {
 		TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_transaction_prepare_commit: failed to get open lock\n"));
-		tdb->ecode = TDB_ERR_LOCK;
 		_tdb_transaction_cancel(tdb, F_WRLCK);
 		return -1;
 	}
 
-	tdb->transaction->open_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) {