[PATCH] Convert Samba to OFD locking.

Jeremy Allison jra at samba.org
Wed May 18 21:42:20 UTC 2016


Hi Jeff, Steve and Simo,

Here is the patchset we discussed to convert
smbd to open file description locking on
modern kernels. It also includes a regression
test to ensure we stay working even on systems
that don't have the required kernels :-).

Now all we need is the fix you and I discussed
to the calls to CIFSSMBPosixLock() to convert
the lock context from current->tgid for process-associated
record locks to a handle-specific lock context
for open file description record locks.

This will put us in really good shape for
doing the OFD-POSIX locks we need in the
SMB2+ UNIX extensions protocol changes.

Please review and let me know !

Cheers,

Jeremy.
-------------- next part --------------
From aa09e48478ff0cb0f30f1af86d581c02a569e403 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 16 May 2016 16:17:12 -0700
Subject: [PATCH 01/10] s3: locking: Rename xxx_windows_lock_ref_count to
 xxx_lock_ref_count.

We will be using this to also ref count a posix lock applied
to a file handle when changing to open file description lock
semantics.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/locking/posix.c | 36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/source3/locking/posix.c b/source3/locking/posix.c
index ea42159..beab9a4 100644
--- a/source3/locking/posix.c
+++ b/source3/locking/posix.c
@@ -410,11 +410,11 @@ bool posix_locking_end(void)
 ****************************************************************************/
 
 /****************************************************************************
- Keep a reference count of the number of Windows locks open on this dev/ino
+ Keep a reference count of the number of locks open on this dev/ino
  pair. Creates entry if it doesn't exist.
 ****************************************************************************/
 
-static void increment_windows_lock_ref_count(files_struct *fsp)
+static void increment_lock_ref_count(files_struct *fsp)
 {
 	struct lock_ref_count_key tmp;
 	int32_t lock_ref_count = 0;
@@ -427,15 +427,11 @@ static void increment_windows_lock_ref_count(files_struct *fsp)
 	SMB_ASSERT(NT_STATUS_IS_OK(status));
 	SMB_ASSERT(lock_ref_count < INT32_MAX);
 
-	DEBUG(10,("increment_windows_lock_ref_count for file now %s = %d\n",
+	DEBUG(10,("lock_ref_count for file %s = %d\n",
 		  fsp_str_dbg(fsp), (int)lock_ref_count));
 }
 
-/****************************************************************************
- Bulk delete - subtract as many locks as we've just deleted.
-****************************************************************************/
-
-static void decrement_windows_lock_ref_count(files_struct *fsp)
+static void decrement_lock_ref_count(files_struct *fsp)
 {
 	struct lock_ref_count_key tmp;
 	int32_t lock_ref_count = 0;
@@ -448,7 +444,7 @@ static void decrement_windows_lock_ref_count(files_struct *fsp)
 	SMB_ASSERT(NT_STATUS_IS_OK(status));
 	SMB_ASSERT(lock_ref_count >= 0);
 
-	DEBUG(10,("reduce_windows_lock_ref_count for file now %s = %d\n",
+	DEBUG(10,("lock_ref_count for file %s = %d\n",
 		  fsp_str_dbg(fsp), (int)lock_ref_count));
 }
 
@@ -456,7 +452,7 @@ static void decrement_windows_lock_ref_count(files_struct *fsp)
  Fetch the lock ref count.
 ****************************************************************************/
 
-static int32_t get_windows_lock_ref_count(files_struct *fsp)
+static int32_t get_lock_ref_count(files_struct *fsp)
 {
 	struct lock_ref_count_key tmp;
 	NTSTATUS status;
@@ -468,7 +464,7 @@ static int32_t get_windows_lock_ref_count(files_struct *fsp)
 
 	if (!NT_STATUS_IS_OK(status) &&
 	    !NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
-		DEBUG(0, ("get_windows_lock_ref_count: Error fetching "
+		DEBUG(0, ("Error fetching "
 			  "lock ref count for file %s: %s\n",
 			  fsp_str_dbg(fsp), nt_errstr(status)));
 	}
@@ -479,7 +475,7 @@ static int32_t get_windows_lock_ref_count(files_struct *fsp)
  Delete a lock_ref_count entry.
 ****************************************************************************/
 
-static void delete_windows_lock_ref_count(files_struct *fsp)
+static void delete_lock_ref_count(files_struct *fsp)
 {
 	struct lock_ref_count_key tmp;
 
@@ -488,7 +484,7 @@ static void delete_windows_lock_ref_count(files_struct *fsp)
 	dbwrap_delete(posix_pending_close_db,
 		      locking_ref_count_key_fsp(fsp, &tmp));
 
-	DEBUG(10,("delete_windows_lock_ref_count for file %s\n",
+	DEBUG(10,("delete_lock_ref_count for file %s\n",
 		  fsp_str_dbg(fsp)));
 }
 
@@ -604,7 +600,7 @@ int fd_close_posix(struct files_struct *fsp)
 		return close(fsp->fh->fd);
 	}
 
-	if (get_windows_lock_ref_count(fsp)) {
+	if (get_lock_ref_count(fsp)) {
 
 		/*
 		 * There are outstanding locks on this dev/inode pair on
@@ -644,7 +640,7 @@ int fd_close_posix(struct files_struct *fsp)
 	TALLOC_FREE(fd_array);
 
 	/* Don't need a lock ref count on this dev/ino anymore. */
-	delete_windows_lock_ref_count(fsp);
+	delete_lock_ref_count(fsp);
 
 	/*
 	 * Finally close the fd associated with this fsp.
@@ -948,7 +944,7 @@ bool set_posix_lock_windows_flavour(files_struct *fsp,
 	 */
 
 	if(!posix_lock_in_range(&offset, &count, u_offset, u_count)) {
-		increment_windows_lock_ref_count(fsp);
+		increment_lock_ref_count(fsp);
 		return True;
 	}
 
@@ -1053,8 +1049,8 @@ bool set_posix_lock_windows_flavour(files_struct *fsp,
 			posix_fcntl_lock(fsp,F_SETLK,offset,count,F_UNLCK);
 		}
 	} else {
-		/* Remember the number of Windows locks we have on this dev/ino pair. */
-		increment_windows_lock_ref_count(fsp);
+		/* Remember the number of locks we have on this dev/ino pair. */
+		increment_lock_ref_count(fsp);
 	}
 
 	talloc_destroy(l_ctx);
@@ -1085,8 +1081,8 @@ bool release_posix_lock_windows_flavour(files_struct *fsp,
 		  "count = %ju\n", fsp_str_dbg(fsp),
 		  (uintmax_t)u_offset, (uintmax_t)u_count));
 
-	/* Remember the number of Windows locks we have on this dev/ino pair. */
-	decrement_windows_lock_ref_count(fsp);
+	/* Remember the number of locks we have on this dev/ino pair. */
+	decrement_lock_ref_count(fsp);
 
 	/*
 	 * If the requested lock won't fit in the POSIX range, we will
-- 
2.8.0.rc3.226.g39d4020


From 45f4be6e65fb45e234505ebf44cd405973972df6 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 16 May 2016 16:37:47 -0700
Subject: [PATCH 02/10] s3: locking: Add some const.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/locking/posix.c | 23 ++++++++++++-----------
 source3/locking/proto.h |  2 +-
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/source3/locking/posix.c b/source3/locking/posix.c
index beab9a4..1773b22 100644
--- a/source3/locking/posix.c
+++ b/source3/locking/posix.c
@@ -346,7 +346,7 @@ struct lock_ref_count_key {
  Form a static locking key for a dev/inode pair for the lock ref count
 ******************************************************************/
 
-static TDB_DATA locking_ref_count_key_fsp(files_struct *fsp,
+static TDB_DATA locking_ref_count_key_fsp(const files_struct *fsp,
 					  struct lock_ref_count_key *tmp)
 {
 	ZERO_STRUCTP(tmp);
@@ -359,9 +359,9 @@ static TDB_DATA locking_ref_count_key_fsp(files_struct *fsp,
  Convenience function to get an fd_array key from an fsp.
 ******************************************************************/
 
-static TDB_DATA fd_array_key_fsp(files_struct *fsp)
+static TDB_DATA fd_array_key_fsp(const files_struct *fsp)
 {
-	return make_tdb_data((uint8_t *)&fsp->file_id, sizeof(fsp->file_id));
+	return make_tdb_data((const uint8_t *)&fsp->file_id, sizeof(fsp->file_id));
 }
 
 /*******************************************************************
@@ -414,7 +414,7 @@ bool posix_locking_end(void)
  pair. Creates entry if it doesn't exist.
 ****************************************************************************/
 
-static void increment_lock_ref_count(files_struct *fsp)
+static void increment_lock_ref_count(const files_struct *fsp)
 {
 	struct lock_ref_count_key tmp;
 	int32_t lock_ref_count = 0;
@@ -431,7 +431,7 @@ static void increment_lock_ref_count(files_struct *fsp)
 		  fsp_str_dbg(fsp), (int)lock_ref_count));
 }
 
-static void decrement_lock_ref_count(files_struct *fsp)
+static void decrement_lock_ref_count(const files_struct *fsp)
 {
 	struct lock_ref_count_key tmp;
 	int32_t lock_ref_count = 0;
@@ -452,7 +452,7 @@ static void decrement_lock_ref_count(files_struct *fsp)
  Fetch the lock ref count.
 ****************************************************************************/
 
-static int32_t get_lock_ref_count(files_struct *fsp)
+static int32_t get_lock_ref_count(const files_struct *fsp)
 {
 	struct lock_ref_count_key tmp;
 	NTSTATUS status;
@@ -475,7 +475,7 @@ static int32_t get_lock_ref_count(files_struct *fsp)
  Delete a lock_ref_count entry.
 ****************************************************************************/
 
-static void delete_lock_ref_count(files_struct *fsp)
+static void delete_lock_ref_count(const files_struct *fsp)
 {
 	struct lock_ref_count_key tmp;
 
@@ -492,7 +492,7 @@ static void delete_lock_ref_count(files_struct *fsp)
  Add an fd to the pending close tdb.
 ****************************************************************************/
 
-static void add_fd_to_close_entry(files_struct *fsp)
+static void add_fd_to_close_entry(const files_struct *fsp)
 {
 	struct db_record *rec;
 	int *fds;
@@ -532,7 +532,7 @@ static void add_fd_to_close_entry(files_struct *fsp)
  Remove all fd entries for a specific dev/inode pair from the tdb.
 ****************************************************************************/
 
-static void delete_close_entries(files_struct *fsp)
+static void delete_close_entries(const files_struct *fsp)
 {
 	struct db_record *rec;
 
@@ -551,7 +551,8 @@ static void delete_close_entries(files_struct *fsp)
 ****************************************************************************/
 
 static size_t get_posix_pending_close_entries(TALLOC_CTX *mem_ctx,
-					      files_struct *fsp, int **entries)
+					const files_struct *fsp,
+					int **entries)
 {
 	TDB_DATA dbuf;
 	NTSTATUS status;
@@ -582,7 +583,7 @@ static size_t get_posix_pending_close_entries(TALLOC_CTX *mem_ctx,
  to delete all locks on this fsp before this function is called.
 ****************************************************************************/
 
-int fd_close_posix(struct files_struct *fsp)
+int fd_close_posix(const struct files_struct *fsp)
 {
 	int saved_errno = 0;
 	int ret;
diff --git a/source3/locking/proto.h b/source3/locking/proto.h
index 8ff1c7c..1eabd6a 100644
--- a/source3/locking/proto.h
+++ b/source3/locking/proto.h
@@ -219,7 +219,7 @@ bool is_posix_locked(files_struct *fsp,
 			enum brl_flavour lock_flav);
 bool posix_locking_init(bool read_only);
 bool posix_locking_end(void);
-int fd_close_posix(struct files_struct *fsp);
+int fd_close_posix(const struct files_struct *fsp);
 bool set_posix_lock_windows_flavour(files_struct *fsp,
 			uint64_t u_offset,
 			uint64_t u_count,
-- 
2.8.0.rc3.226.g39d4020


From b9e8212309ea02b4a0f9d627aedc51bd0b27f19c Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 17 May 2016 12:49:36 -0700
Subject: [PATCH 03/10] s3: locking: Add a const struct lock_context * paramter
 to set_posix_lock_posix_flavour()

We will need this to implement open file description record locks.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/locking/brlock.c | 1 +
 source3/locking/posix.c  | 1 +
 source3/locking/proto.h  | 1 +
 3 files changed, 3 insertions(+)

diff --git a/source3/locking/brlock.c b/source3/locking/brlock.c
index e8c8d89..70096d6 100644
--- a/source3/locking/brlock.c
+++ b/source3/locking/brlock.c
@@ -923,6 +923,7 @@ static NTSTATUS brl_lock_posix(struct messaging_context *msg_ctx,
 				plock->start,
 				plock->size,
 				plock->lock_type,
+				&plock->context,
 				&errno_ret)) {
 
 			/* We don't know who blocked us. */
diff --git a/source3/locking/posix.c b/source3/locking/posix.c
index 1773b22..743eaa1 100644
--- a/source3/locking/posix.c
+++ b/source3/locking/posix.c
@@ -1192,6 +1192,7 @@ bool set_posix_lock_posix_flavour(files_struct *fsp,
 			uint64_t u_offset,
 			uint64_t u_count,
 			enum brl_type lock_type,
+			const struct lock_context *lock_ctx,
 			int *errno_ret)
 {
 	off_t offset;
diff --git a/source3/locking/proto.h b/source3/locking/proto.h
index 1eabd6a..13499cf 100644
--- a/source3/locking/proto.h
+++ b/source3/locking/proto.h
@@ -239,6 +239,7 @@ bool set_posix_lock_posix_flavour(files_struct *fsp,
 			uint64_t u_offset,
 			uint64_t u_count,
 			enum brl_type lock_type,
+			const struct lock_context *lock_ctx,
 			int *errno_ret);
 bool release_posix_lock_posix_flavour(files_struct *fsp,
 				uint64_t u_offset,
-- 
2.8.0.rc3.226.g39d4020


From fa7ad42f95ea52b134fa0af8c37314310b446c5e Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 16 May 2016 16:59:30 -0700
Subject: [PATCH 04/10] s3: locking: Convert on the wire behavior of POSIX
 (UNIX extensions) locks from process-associated locks to open file
 description locks.

This means locks are associated with the SMB handle
they were created on, not the inode. In all other ways
they behave like UNIX extensions fcntl (process-associated)
locks. Torture test to follow.

When a handle is closed all locks attached to that handle
are closed, not all locks on the underlying inode. In
this respect they now behave like Windows locks.

The key to this in the UNIX extensions locking codepath is modifying
the reference count only when a new locking context is seen
on any lock request, and decrementing the reference count
when the last instance of a locking context is seen on any
unlock request. For SMB2+ the persistent part of a file handle
is used as the locking context so this behavior becomes
natural.

This is a behavior change but after consultation with
Jeff Layton and Steve French the only client that implements
UNIX extensions POSIX locks - the cifsfs client - already
expects these locks to behave like open file description
(ofd) locks. With our previous behavior Linux ofd-locks
fail against smbd.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/locking/posix.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/source3/locking/posix.c b/source3/locking/posix.c
index 743eaa1..0c729ad 100644
--- a/source3/locking/posix.c
+++ b/source3/locking/posix.c
@@ -1180,6 +1180,86 @@ bool release_posix_lock_windows_flavour(files_struct *fsp,
 ****************************************************************************/
 
 /****************************************************************************
+ We only increment the lock ref count when we see a POSIX lock on a context
+ that doesn't already have them.
+****************************************************************************/
+
+static void increment_posix_lock_count(const files_struct *fsp,
+					uint64_t smblctx)
+{
+	NTSTATUS status;
+	TDB_DATA ctx_key;
+	TDB_DATA val = { 0 };
+
+	ctx_key.dptr = (uint8_t *)&smblctx;
+	ctx_key.dsize = sizeof(smblctx);
+
+	/*
+	 * Don't increment if we already have any POSIX flavor
+	 * locks on this context.
+	 */
+	if (dbwrap_exists(posix_pending_close_db, ctx_key)) {
+		return;
+	}
+
+	/* Remember that we have POSIX flavor locks on this context. */
+	status = dbwrap_store(posix_pending_close_db, ctx_key, val, 0);
+	SMB_ASSERT(NT_STATUS_IS_OK(status));
+
+	increment_lock_ref_count(fsp);
+
+	DEBUG(10,("posix_locks set for file %s\n",
+		fsp_str_dbg(fsp)));
+}
+
+static void decrement_posix_lock_count(const files_struct *fsp, uint64_t smblctx)
+{
+	NTSTATUS status;
+	TDB_DATA ctx_key;
+
+	ctx_key.dptr = (uint8_t *)&smblctx;
+	ctx_key.dsize = sizeof(smblctx);
+
+	status = dbwrap_delete(posix_pending_close_db, ctx_key);
+	SMB_ASSERT(NT_STATUS_IS_OK(status));
+
+	decrement_lock_ref_count(fsp);
+
+	DEBUG(10,("posix_locks deleted for file %s\n",
+		fsp_str_dbg(fsp)));
+}
+
+/****************************************************************************
+ Return true if any locks exist on the given lock context.
+****************************************************************************/
+
+static bool locks_exist_on_context(const struct lock_struct *plocks,
+				int num_locks,
+				const struct lock_context *lock_ctx)
+{
+	int i;
+
+	for (i=0; i < num_locks; i++) {
+		const struct lock_struct *lock = &plocks[i];
+
+		/* Ignore all but read/write locks. */
+		if (lock->lock_type != READ_LOCK && lock->lock_type != WRITE_LOCK) {
+			continue;
+		}
+
+		/* Ignore locks not owned by this process. */
+		if (!serverid_equal(&lock->context.pid, &lock_ctx->pid)) {
+			continue;
+		}
+
+		if (lock_ctx->smblctx == lock->context.smblctx) {
+			return true;
+		}
+	}
+	return false;
+}
+
+/****************************************************************************
  POSIX function to acquire a lock. Returns True if the
  lock could be granted, False if not.
  As POSIX locks don't stack or conflict (they just overwrite)
@@ -1210,6 +1290,7 @@ bool set_posix_lock_posix_flavour(files_struct *fsp,
 	 */
 
 	if(!posix_lock_in_range(&offset, &count, u_offset, u_count)) {
+		increment_posix_lock_count(fsp, lock_ctx->smblctx);
 		return True;
 	}
 
@@ -1219,6 +1300,7 @@ bool set_posix_lock_posix_flavour(files_struct *fsp,
 			posix_lock_type_name(posix_lock_type), (intmax_t)offset, (intmax_t)count, strerror(errno) ));
 		return False;
 	}
+	increment_posix_lock_count(fsp, lock_ctx->smblctx);
 	return True;
 }
 
@@ -1255,6 +1337,9 @@ bool release_posix_lock_posix_flavour(files_struct *fsp,
 	 */
 
 	if(!posix_lock_in_range(&offset, &count, u_offset, u_count)) {
+		if (!locks_exist_on_context(plocks, num_locks, lock_ctx)) {
+			decrement_posix_lock_count(fsp, lock_ctx->smblctx);
+		}
 		return True;
 	}
 
@@ -1308,6 +1393,9 @@ bool release_posix_lock_posix_flavour(files_struct *fsp,
 		}
 	}
 
+	if (!locks_exist_on_context(plocks, num_locks, lock_ctx)) {
+		decrement_posix_lock_count(fsp, lock_ctx->smblctx);
+	}
 	talloc_destroy(ul_ctx);
 	return ret;
 }
-- 
2.8.0.rc3.226.g39d4020


From 69df2df08d8f032abfa02b168d0112033b1667cc Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 13 May 2016 15:09:54 +0100
Subject: [PATCH 05/10] s3: torture: Add POSIX-OFD-LOCK test.

Ensures that we *always* expose ofd-lock behavior to clients.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 selftest/knownfail        |   1 +
 source3/selftest/tests.py |   2 +-
 source3/torture/torture.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 122 insertions(+), 1 deletion(-)

diff --git a/selftest/knownfail b/selftest/knownfail
index c9f4fb0..59aba84 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -18,6 +18,7 @@
 ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).POSIX-APPEND # Fails against the s4 ntvfs server
 ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).POSIX-SYMLINK-ACL # Fails against the s4 ntvfs server
 ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).POSIX-SYMLINK-EA # Fails against the s4 ntvfs server
+^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).POSIX-OFD-LOCK # Fails against the s4 ntvfs server
 ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).NTTRANS-FSCTL # Fails against the s4 ntvfs server
 ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).SMB2-NEGPROT # Fails against the s4 ntvfs server
 ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).BAD-NBT-SESSION # Fails against the s4 ntvfs server
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 4ecb3f1..ddea77c 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -78,7 +78,7 @@ tests = ["RW1", "RW2", "RW3"]
 for t in tests:
     plantestsuite("samba3.smbtorture_s3.vfs_aio_fork(simpleserver).%s" % t, "simpleserver", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), t, '//$SERVER_IP/vfs_aio_fork', '$USERNAME', '$PASSWORD', smbtorture3, "", "-l $LOCAL_PATH"])
 
-posix_tests = ["POSIX", "POSIX-APPEND", "POSIX-SYMLINK-ACL", "POSIX-SYMLINK-EA"]
+posix_tests = ["POSIX", "POSIX-APPEND", "POSIX-SYMLINK-ACL", "POSIX-SYMLINK-EA", "POSIX-OFD-LOCK" ]
 
 for t in posix_tests:
     plantestsuite("samba3.smbtorture_s3.plain(nt4_dc).%s" % t, "nt4_dc", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), t, '//$SERVER_IP/posix_share', '$USERNAME', '$PASSWORD', smbtorture3, "", "-l $LOCAL_PATH"])
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index d6489c8..ea0fc01 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -6191,6 +6191,125 @@ static bool run_ea_symlink_test(int dummy)
 	return correct;
 }
 
+/*
+  Test POSIX locks are OFD-locks.
+ */
+static bool run_posix_ofd_lock_test(int dummy)
+{
+	static struct cli_state *cli;
+	const char *fname = "posix_file";
+	uint16_t fnum1 = (uint16_t)-1;
+	uint16_t fnum2 = (uint16_t)-1;
+	bool correct = false;
+	NTSTATUS status;
+	TALLOC_CTX *frame = NULL;
+
+	frame = talloc_stackframe();
+
+	printf("Starting POSIX ofd-lock test\n");
+
+	if (!torture_open_connection(&cli, 0)) {
+		TALLOC_FREE(frame);
+		return false;
+	}
+
+	smbXcli_conn_set_sockopt(cli->conn, sockops);
+
+	status = torture_setup_unix_extensions(cli);
+	if (!NT_STATUS_IS_OK(status)) {
+		TALLOC_FREE(frame);
+		return false;
+	}
+
+	cli_setatr(cli, fname, 0, 0);
+	cli_posix_unlink(cli, fname);
+
+	/* Open the file twice. */
+	status = cli_posix_open(cli, fname, O_RDWR|O_CREAT|O_EXCL,
+				0600, &fnum1);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("First POSIX open of %s failed\n", fname);
+		goto out;
+	}
+
+	status = cli_posix_open(cli, fname, O_RDWR, 0, &fnum2);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("First POSIX open of %s failed\n", fname);
+		goto out;
+	}
+
+	/* Set a 0-50 lock on fnum1. */
+	status = cli_posix_lock(cli, fnum1, 0, 50, false, WRITE_LOCK);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("POSIX lock (1) failed %s\n", nt_errstr(status));
+		goto out;
+	}
+
+	/* Set a 60-100 lock on fnum2. */
+	status = cli_posix_lock(cli, fnum2, 60, 100, false, WRITE_LOCK);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("POSIX lock (2) failed %s\n", nt_errstr(status));
+		goto out;
+	}
+
+	/* close fnum1 - 0-50 lock should go away. */
+	status = cli_close(cli, fnum1);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("close failed (%s)\n",
+			nt_errstr(status));
+		goto out;
+	}
+	fnum1 = (uint16_t)-1;
+
+	/* Change the lock context. */
+	cli_setpid(cli, cli_getpid(cli) + 1);
+
+	/* Re-open fnum1. */
+	status = cli_posix_open(cli, fname, O_RDWR, 0, &fnum1);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("Third POSIX open of %s failed\n", fname);
+		goto out;
+	}
+
+	/* 60-100 lock should still be there. */
+	status = cli_posix_lock(cli, fnum1, 60, 100, false, WRITE_LOCK);
+	if (!NT_STATUS_EQUAL(status, NT_STATUS_FILE_LOCK_CONFLICT)) {
+		printf("POSIX lock 60-100 not there %s\n", nt_errstr(status));
+		goto out;
+	}
+
+	/* 0-50 lock should be gone. */
+	status = cli_posix_lock(cli, fnum1, 0, 50, false, WRITE_LOCK);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("POSIX lock 0-50 failed %s\n", nt_errstr(status));
+		goto out;
+	}
+
+	printf("POSIX OFD lock test passed\n");
+	correct = true;
+
+  out:
+
+	if (fnum1 != (uint16_t)-1) {
+		cli_close(cli, fnum1);
+		fnum1 = (uint16_t)-1;
+	}
+	if (fnum2 != (uint16_t)-1) {
+		cli_close(cli, fnum2);
+		fnum2 = (uint16_t)-1;
+	}
+
+	cli_setatr(cli, fname, 0, 0);
+	cli_posix_unlink(cli, fname);
+
+	if (!torture_close_connection(cli)) {
+		correct = false;
+	}
+
+	TALLOC_FREE(frame);
+	return correct;
+}
+
 static uint32_t open_attrs_table[] = {
 		FILE_ATTRIBUTE_NORMAL,
 		FILE_ATTRIBUTE_ARCHIVE,
@@ -10020,6 +10139,7 @@ static struct {
 	{"POSIX-APPEND", run_posix_append, 0},
 	{"POSIX-SYMLINK-ACL", run_acl_symlink_test, 0},
 	{"POSIX-SYMLINK-EA", run_ea_symlink_test, 0},
+	{"POSIX-OFD-LOCK", run_posix_ofd_lock_test, 0},
 	{"CASE-INSENSITIVE-CREATE", run_case_insensitive_create, 0},
 	{"ASYNC-ECHO", run_async_echo, 0},
 	{ "UID-REGRESSION-TEST", run_uid_regression_test, 0},
-- 
2.8.0.rc3.226.g39d4020


From 1e4090cce4c94e4a1dcb8f9cbb0622267eb2ceee Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 12 May 2016 20:57:36 +0200
Subject: [PATCH 06/10] s3: lib: Add 'int op' parameter to fcntl_getlock().

Will allow us to move to open file description locks
from process-associated locks.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/include/proto.h       | 2 +-
 source3/lib/util.c            | 8 ++++----
 source3/modules/vfs_default.c | 3 ++-
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/source3/include/proto.h b/source3/include/proto.h
index c032b9e..15e0095 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -385,7 +385,7 @@ bool is_in_path(const char *name, name_compare_entry *namelist, bool case_sensit
 void set_namearray(name_compare_entry **ppname_array, const char *namelist);
 void free_namearray(name_compare_entry *name_array);
 bool fcntl_lock(int fd, int op, off_t offset, off_t count, int type);
-bool fcntl_getlock(int fd, off_t *poffset, off_t *pcount, int *ptype, pid_t *ppid);
+bool fcntl_getlock(int fd, int op, off_t *poffset, off_t *pcount, int *ptype, pid_t *ppid);
 bool is_myname(const char *s);
 void ra_lanman_string( const char *native_lanman );
 const char *get_remote_arch_str(void);
diff --git a/source3/lib/util.c b/source3/lib/util.c
index f64f6b5..36a2750 100644
--- a/source3/lib/util.c
+++ b/source3/lib/util.c
@@ -1150,13 +1150,13 @@ void set_namearray(name_compare_entry **ppname_array, const char *namelist_in)
  F_UNLCK in *ptype if the region is unlocked). False if the call failed.
 ****************************************************************************/
 
-bool fcntl_getlock(int fd, off_t *poffset, off_t *pcount, int *ptype, pid_t *ppid)
+bool fcntl_getlock(int fd, int op, off_t *poffset, off_t *pcount, int *ptype, pid_t *ppid)
 {
 	struct flock lock;
 	int ret;
 
-	DEBUG(8,("fcntl_getlock fd=%d offset=%.0f count=%.0f type=%d\n",
-		    fd,(double)*poffset,(double)*pcount,*ptype));
+	DEBUG(8,("fcntl_getlock fd=%d op=%d offset=%.0f count=%.0f type=%d\n",
+		    fd,op,(double)*poffset,(double)*pcount,*ptype));
 
 	lock.l_type = *ptype;
 	lock.l_whence = SEEK_SET;
@@ -1164,7 +1164,7 @@ bool fcntl_getlock(int fd, off_t *poffset, off_t *pcount, int *ptype, pid_t *ppi
 	lock.l_len = *pcount;
 	lock.l_pid = 0;
 
-	ret = sys_fcntl_ptr(fd,F_GETLK,&lock);
+	ret = sys_fcntl_ptr(fd,op,&lock);
 
 	if (ret == -1) {
 		int sav = errno;
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index 8ee1635..087fb44 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -2119,9 +2119,10 @@ static int vfswrap_kernel_flock(vfs_handle_struct *handle, files_struct *fsp,
 static bool vfswrap_getlock(vfs_handle_struct *handle, files_struct *fsp, off_t *poffset, off_t *pcount, int *ptype, pid_t *ppid)
 {
 	bool result;
+	int op = F_GETLK;
 
 	START_PROFILE(syscall_fcntl_getlock);
-	result =  fcntl_getlock(fsp->fh->fd, poffset, pcount, ptype, ppid);
+	result = fcntl_getlock(fsp->fh->fd, op, poffset, pcount, ptype, ppid);
 	END_PROFILE(syscall_fcntl_getlock);
 	return result;
 }
-- 
2.8.0.rc3.226.g39d4020


From df8690261c0db0a1468028620ec97db6c24dc4ad Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 12 May 2016 21:03:57 +0200
Subject: [PATCH 07/10] s3: VFS: Add bool use_ofd_locks member to struct
 files_struct.

Not yet used. We will set this if we translate a process-associated
lock operation to a open file description lock operation.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/include/vfs.h   | 2 ++
 source3/locking/posix.c | 9 +++++----
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/source3/include/vfs.h b/source3/include/vfs.h
index 9360802..13ce539 100644
--- a/source3/include/vfs.h
+++ b/source3/include/vfs.h
@@ -192,6 +192,7 @@
 		const struct smb_filename * */
 /* Version 35 - Add uint32_t flags to struct smb_filename */
 /* Version 35 - Add get/set/fget/fset dos attribute functions. */
+/* Version 35 - Add bool use_ofd_locks to struct files_struct */
 
 #define SMB_VFS_INTERFACE_VERSION 35
 
@@ -285,6 +286,7 @@ typedef struct files_struct {
 	bool backup_intent; /* Handle was successfully opened with backup intent
 				and opener has privilege to do so. */
 	bool aapl_copyfile_supported;
+	bool use_ofd_locks; /* Are we using open file description locks ? */
 	struct smb_filename *fsp_name;
 	uint32_t name_hash;		/* Jenkins hash of full pathname. */
 	uint64_t mid;			/* Mid of the operation that created us. */
diff --git a/source3/locking/posix.c b/source3/locking/posix.c
index 0c729ad..f3a89fd 100644
--- a/source3/locking/posix.c
+++ b/source3/locking/posix.c
@@ -591,12 +591,13 @@ int fd_close_posix(const struct files_struct *fsp)
 	size_t count, i;
 
 	if (!lp_locking(fsp->conn->params) ||
-	    !lp_posix_locking(fsp->conn->params))
+	    !lp_posix_locking(fsp->conn->params) ||
+	    fsp->use_ofd_locks)
 	{
 		/*
-		 * No locking or POSIX to worry about or we want POSIX semantics
-		 * which will lose all locks on all fd's open on this dev/inode,
-		 * just close.
+		 * No locking or POSIX to worry about or we are using POSIX
+		 * open file description lock semantics which only removes
+		 * locks on the file descriptor we're closing. Just close.
 		 */
 		return close(fsp->fh->fd);
 	}
-- 
2.8.0.rc3.226.g39d4020


From 6fe12e1c9c2c97c3eef61fe5aa2922ea45e2e9fe Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 12 May 2016 21:14:28 +0200
Subject: [PATCH 08/10] s3: lib: util: Add map_process_lock_to_ofd_lock()
 utility function.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/include/proto.h |  1 +
 source3/lib/util.c      | 31 +++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/source3/include/proto.h b/source3/include/proto.h
index 15e0095..36a5499 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -386,6 +386,7 @@ void set_namearray(name_compare_entry **ppname_array, const char *namelist);
 void free_namearray(name_compare_entry *name_array);
 bool fcntl_lock(int fd, int op, off_t offset, off_t count, int type);
 bool fcntl_getlock(int fd, int op, off_t *poffset, off_t *pcount, int *ptype, pid_t *ppid);
+int map_process_lock_to_ofd_lock(int op, bool *use_ofd_locks);
 bool is_myname(const char *s);
 void ra_lanman_string( const char *native_lanman );
 const char *get_remote_arch_str(void);
diff --git a/source3/lib/util.c b/source3/lib/util.c
index 36a2750..ad33624 100644
--- a/source3/lib/util.c
+++ b/source3/lib/util.c
@@ -1184,6 +1184,37 @@ bool fcntl_getlock(int fd, int op, off_t *poffset, off_t *pcount, int *ptype, pi
 	return True;
 }
 
+#if defined(HAVE_OFD_LOCKS)
+int map_process_lock_to_ofd_lock(int op, bool *use_ofd_locks)
+{
+	switch (op) {
+	case F_GETLK:
+	case F_OFD_GETLK:
+		op = F_OFD_GETLK;
+		break;
+	case F_SETLK:
+	case F_OFD_SETLK:
+		op = F_OFD_SETLK;
+		break;
+	case F_SETLKW:
+	case F_OFD_SETLKW:
+		op = F_OFD_SETLKW;
+		break;
+	default:
+		*use_ofd_locks = false;
+		return -1;
+	}
+	*use_ofd_locks = true;
+	return op;
+}
+#else /* HAVE_OFD_LOCKS */
+int map_process_lock_to_ofd_lock(int op, bool *use_ofd_locks)
+{
+	*use_ofd_locks = false;
+	return op;
+}
+#endif /* HAVE_OFD_LOCKS */
+
 #undef DBGC_CLASS
 #define DBGC_CLASS DBGC_ALL
 
-- 
2.8.0.rc3.226.g39d4020


From 648331d136309d418de048977f898b6d836050e2 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 12 May 2016 21:17:21 +0200
Subject: [PATCH 09/10] s3: VFS: Map process-associated lock operation to open
 file description lock operation.

Only in the default VFS. Gpfs, Ceph, Gluster and other modern
backend VFS filesystems might want to do the same.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/modules/vfs_default.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index 087fb44..b298593 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -2101,6 +2101,8 @@ static bool vfswrap_lock(vfs_handle_struct *handle, files_struct *fsp, int op, o
 {
 	bool result;
 
+	op = map_process_lock_to_ofd_lock(op, &fsp->use_ofd_locks);
+
 	START_PROFILE(syscall_fcntl_lock);
 	result =  fcntl_lock(fsp->fh->fd, op, offset, count, type);
 	END_PROFILE(syscall_fcntl_lock);
@@ -2119,7 +2121,7 @@ static int vfswrap_kernel_flock(vfs_handle_struct *handle, files_struct *fsp,
 static bool vfswrap_getlock(vfs_handle_struct *handle, files_struct *fsp, off_t *poffset, off_t *pcount, int *ptype, pid_t *ppid)
 {
 	bool result;
-	int op = F_GETLK;
+	int op = map_process_lock_to_ofd_lock(F_GETLK, &fsp->use_ofd_locks);
 
 	START_PROFILE(syscall_fcntl_getlock);
 	result = fcntl_getlock(fsp->fh->fd, op, poffset, pcount, ptype, ppid);
-- 
2.8.0.rc3.226.g39d4020


From eb7f30b606a006cca80f4de990bb1c9e620a7f97 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 12 May 2016 21:46:50 +0200
Subject: [PATCH 10/10] s3: wscript: Add checks for open file description
 locks.

We now use these if available.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/wscript | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/source3/wscript b/source3/wscript
index 3b6f8a4..51c1542 100644
--- a/source3/wscript
+++ b/source3/wscript
@@ -1089,6 +1089,22 @@ syscall(SYS_setgroups32, 0, NULL);
                 execute=True,
                 msg='Checking whether fcntl locking is available')
 
+    conf.CHECK_CODE('''
+           struct flock lck = {
+              .l_whence = SEEK_SET,
+              .l_type = F_WRLCK,
+              .l_start = 0,
+              .l_len = 1,
+              .l_pid = 0,
+            };
+            int ret = fcntl(0, F_OFD_SETLK, &lck);
+            int ret1 = fcntl(0, F_OFD_SETLKW, &lck);
+            int ret2 = fcntl(0, F_OFD_GETLK, &lck);
+            ''',
+            'HAVE_OFD_LOCKS',
+            msg="Checking whether fcntl lock supports open file description locks",
+            headers='unistd.h sys/types.h sys/stat.h fcntl.h')
+
 # glibc up to 2.3.6 had dangerously broken posix_fallocate(). DON'T USE IT.
     if not conf.CHECK_CODE('''
 #define _XOPEN_SOURCE 600
-- 
2.8.0.rc3.226.g39d4020



More information about the samba-technical mailing list