[PATCH] Convert Samba to OFD locking - version 2
Jeremy Allison
jra at samba.org
Thu May 19 21:12:48 UTC 2016
OK, here is version 2 with Uri and Simo's changes
incorporated. Compile-time checks now run the test
code rather than just compiling, and there's a
tuneable "smbd:force process locks" that will force
the old-style process specific locks. Advice to
use it gets printed in the log if fcntl returns
EINVAL.
Please review and push if happy !
Thanks,
Jeremy.
-------------- next part --------------
From 5fdd7c0e675a03e38988c16263bbf73a3f2e7ae2 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 e201bababc14c748d25fcd28a83f2c0c27ac3a59 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 918c5d99476d66382b7cd4243a7e190f37c2bd7f 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 c68e340da241760fc93900bd8add11709784be5f 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 cc979b2db54eebc474d7a1b32cd317879f7126a0 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 35d3baa5562ff177fc8490fe2a879d7e29f7ae39 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 03aef12826ff33f9dbc11bc062587cf12ad06ba3 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 a849443..dca6cef 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
@@ -287,6 +288,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 db5b2008f978630dd6e984f0a5aada07bdbed8bb 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 b141fad212f1ca488d75a420266c4f4c68929d5c 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.
Allow tuneable "smbd:force process locks = true" to turn
off OFD locks if in use and the kernel doesn't support them.
Display debug message showing admins what to do in this case.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/locking/posix.c | 16 +++++++++++++---
source3/modules/vfs_default.c | 14 ++++++++++++++
2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/source3/locking/posix.c b/source3/locking/posix.c
index f3a89fd..432637a 100644
--- a/source3/locking/posix.c
+++ b/source3/locking/posix.c
@@ -198,12 +198,22 @@ static bool posix_fcntl_lock(files_struct *fsp, int op, off_t offset, off_t coun
if (!ret && ((errno == EFBIG) || (errno == ENOLCK) || (errno == EINVAL))) {
- DEBUG(0, ("posix_fcntl_lock: WARNING: lock request at offset "
+ if ((errno == EINVAL) &&
+ (op != F_GETLK &&
+ op != F_SETLK &&
+ op != F_SETLKW)) {
+ DEBUG(0,("WARNING: OFD locks in use and no kernel "
+ "support. Try setting "
+ "'smbd:force process locks = true' "
+ "in smb.conf\n"));
+ } else {
+ DEBUG(0, ("WARNING: lock request at offset "
"%ju, length %ju returned\n",
(uintmax_t)offset, (uintmax_t)count));
- DEBUGADD(0, ("an %s error. This can happen when using 64 bit "
+ DEBUGADD(0, ("an %s error. This can happen when using 64 bit "
"lock offsets\n", strerror(errno)));
- DEBUGADD(0, ("on 32 bit NFS mounted file systems.\n"));
+ DEBUGADD(0, ("on 32 bit NFS mounted file systems.\n"));
+ }
/*
* If the offset is > 0x7FFFFFFF then this will cause problems on
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index 087fb44..4485737 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -2101,6 +2101,13 @@ static bool vfswrap_lock(vfs_handle_struct *handle, files_struct *fsp, int op, o
{
bool result;
+ if (fsp->use_ofd_locks || !lp_parm_bool(SNUM(fsp->conn),
+ "smbd",
+ "force process locks",
+ false)) {
+ 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);
@@ -2121,6 +2128,13 @@ static bool vfswrap_getlock(vfs_handle_struct *handle, files_struct *fsp, off_t
bool result;
int op = F_GETLK;
+ if (fsp->use_ofd_locks || !lp_parm_bool(SNUM(fsp->conn),
+ "smbd",
+ "force process locks",
+ false)) {
+ op = map_process_lock_to_ofd_lock(op, &fsp->use_ofd_locks);
+ }
+
START_PROFILE(syscall_fcntl_getlock);
result = fcntl_getlock(fsp->fh->fd, op, poffset, pcount, ptype, ppid);
END_PROFILE(syscall_fcntl_getlock);
--
2.8.0.rc3.226.g39d4020
From bfa4ee18e38220045c79416b24a34ee38972e25b 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.
Compiles and runs code that checks for working
F_OFD_GETLK, F_OFD_SETLK, F_OFD_SETLKW.
We now use these if available.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/wscript | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/source3/wscript b/source3/wscript
index 3b6f8a4..497b673 100644
--- a/source3/wscript
+++ b/source3/wscript
@@ -1089,6 +1089,66 @@ syscall(SYS_setgroups32, 0, NULL);
execute=True,
msg='Checking whether fcntl locking is available')
+ conf.CHECK_CODE('''
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+
+#define DATA "ofdtest.fcntl"
+
+int main() {
+ struct flock lck = {
+ .l_whence = SEEK_SET,
+ .l_type = F_WRLCK,
+ .l_start = 0,
+ .l_len = 1,
+ .l_pid = 0,
+ };
+ int ret;
+ int fd1;
+ int fd2;
+ char *testdir = getenv("TESTDIR");
+
+ if (testdir) {
+ if (chdir(testdir) != 0) {
+ goto err;
+ }
+ }
+
+ unlink(DATA);
+ fd1 = open(DATA, O_RDWR|O_CREAT|O_EXCL, 0600);
+ fd2 = open(DATA, O_RDWR);
+ if (fd1 == -1 || fd2 == -1) {
+ goto err;
+ }
+ ret = fcntl(fd1,F_OFD_SETLKW,&lck);
+ if (ret == -1) {
+ goto err;
+ }
+ ret = fcntl(fd2,F_OFD_SETLK,&lck);
+ if (ret != -1) {
+ goto err;
+ }
+ if (errno != EAGAIN) {
+ goto err;
+ }
+ ret = fcntl(fd2,F_OFD_GETLK,&lck);
+ if (ret == -1) {
+ goto err;
+ }
+ unlink(DATA);
+ exit(0);
+err:
+ unlink(DATA);
+ exit(1);
+}''',
+ 'HAVE_OFD_LOCKS',
+ addmain=False,
+ execute=True,
+ msg="Checking whether fcntl lock supports open file description locks")
+
# 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