Samba and Linux OFD locks
Jeremy Allison
jra at samba.org
Thu Jan 31 01:21:24 UTC 2019
On Wed, Jan 30, 2019 at 07:45:30PM +0100, Andreas Schneider wrote:
> Hello,
>
> on newer Linux distributions with glibc >= 2.20 we have OFD file locks
> available. Our autobuild hosts are too old!
>
> However if your system supports OFD file locks the
> samba3.vfs.fruit_netatalk.nt4 test is failing!
>
> The attached patch has some fixes and adds debug messages and shows a
> workaround.
>
> If you look where it fails, we want to unlock a file with fcntl and get:
>
> 3163 19:20:18.349723 fcntl(37</home/asn/workspace/projects/samba/st/nt4_dc/
> share/vfs_fruit_dir/locking_conflict.txt>, F_OFD_GETLK, {l_type=F_RDLCK,
> l_whence=SEEK_SET, l_start=0, l_len=9223372036854775807,
> l_pid=18446744073709551615}) = 0 <0.000003>
l_len and l_pid look... strange here. I don't think they
can be correct.
> In contrary to the posix locks, the OFD locks return information in 'struct
> flock' and this tells us that we are holding a read lock!
>
> test_netatalk_lock() checks if the return lock is F_UNLCK, but F_RDLCK has
> been returned and we fail. We fail because if you look a bit earlier in the
> strace we acquired an flock() on that file:
>
> 3163 19:20:18.349282 flock(37</home/asn/workspace/projects/samba/st/nt4_dc/
> share/vfs_fruit_dir/locking_conflict.txt>, LOCK_MAND|LOCK_READ) = 0 <0.000004>
>
> I think that the OFD lock returns that information and we do not handle it. I
> have no idea where this flock() is coming from. Maybe the vfs experts could
> chime in and help here.
>
>
> Thanks,
>
>
> Andreas
>
>
> --
> Andreas Schneider asn at samba.org
> Samba Team www.samba.org
> GPG-ID: 8DFF53E18F2ABC8D8F3C92237EE0FC4DCC014E3D
> From d964ce0c6fc038397155b9596103b42c365b968c Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Wed, 30 Jan 2019 18:09:52 +0100
> Subject: [PATCH 1/5] s3:vfs: Initialize pid to 0 in test_netatalk_lock()
>
> Signed-off-by: Andreas Schneider <asn at samba.org>
> ---
> source3/modules/vfs_fruit.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
> index 9f3fe24e5fc..0a67ad39d8e 100644
> --- a/source3/modules/vfs_fruit.c
> +++ b/source3/modules/vfs_fruit.c
> @@ -2647,7 +2647,7 @@ static bool test_netatalk_lock(files_struct *fsp, off_t in_offset)
> off_t offset = in_offset;
> off_t len = 1;
> int type = F_WRLCK;
> - pid_t pid;
> + pid_t pid = 0;
>
> result = SMB_VFS_GETLOCK(fsp, &offset, &len, &type, &pid);
> if (result == false) {
> --
> 2.20.1
>
>
> From 02d4c954923b0be3950660fa695cce53919c1f24 Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Wed, 30 Jan 2019 18:45:34 +0100
> Subject: [PATCH 2/5] s3:vfs: Correctly check if OFD locks should be enabled or
> not
>
> Also the smb.conf options should only be checked once and a reload of
> the config should not switch to a different locking mode.
>
> Signed-off-by: Andreas Schneider <asn at samba.org>
> ---
> source3/include/proto.h | 2 +-
> source3/lib/util.c | 7 ++-----
> source3/modules/vfs_default.c | 14 ++++----------
> source3/smbd/files.c | 9 +++++++++
> 4 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/source3/include/proto.h b/source3/include/proto.h
> index 715bc56e286..9d6192967ba 100644
> --- a/source3/include/proto.h
> +++ b/source3/include/proto.h
> @@ -360,7 +360,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);
> +int map_process_lock_to_ofd_lock(int op);
> 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 5dbd67349fa..7530ea67973 100644
> --- a/source3/lib/util.c
> +++ b/source3/lib/util.c
> @@ -1079,7 +1079,7 @@ bool fcntl_getlock(int fd, int op, off_t *poffset, off_t *pcount, int *ptype, pi
> }
>
> #if defined(HAVE_OFD_LOCKS)
> -int map_process_lock_to_ofd_lock(int op, bool *use_ofd_locks)
> +int map_process_lock_to_ofd_lock(int op)
> {
> switch (op) {
> case F_GETLK:
> @@ -1095,16 +1095,13 @@ int map_process_lock_to_ofd_lock(int op, bool *use_ofd_locks)
> 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)
> +int map_process_lock_to_ofd_lock(int op)
> {
> - *use_ofd_locks = false;
> return op;
> }
> #endif /* HAVE_OFD_LOCKS */
> diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
> index a27d33a6bea..cb5537e096e 100644
> --- a/source3/modules/vfs_default.c
> +++ b/source3/modules/vfs_default.c
> @@ -2553,11 +2553,8 @@ static bool vfswrap_lock(vfs_handle_struct *handle, files_struct *fsp, int op, o
>
> START_PROFILE(syscall_fcntl_lock);
>
> - 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);
> + if (fsp->use_ofd_locks) {
> + op = map_process_lock_to_ofd_lock(op);
> }
>
> result = fcntl_lock(fsp->fh->fd, op, offset, count, type);
> @@ -2581,11 +2578,8 @@ static bool vfswrap_getlock(vfs_handle_struct *handle, files_struct *fsp, off_t
>
> START_PROFILE(syscall_fcntl_getlock);
>
> - 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);
> + if (fsp->use_ofd_locks) {
> + op = map_process_lock_to_ofd_lock(op);
> }
>
> result = fcntl_getlock(fsp->fh->fd, op, poffset, pcount, ptype, ppid);
> diff --git a/source3/smbd/files.c b/source3/smbd/files.c
> index 397baea84cb..99b4937c99b 100644
> --- a/source3/smbd/files.c
> +++ b/source3/smbd/files.c
> @@ -51,6 +51,15 @@ NTSTATUS fsp_new(struct connection_struct *conn, TALLOC_CTX *mem_ctx,
> goto fail;
> }
>
> +#if defined(HAVE_OFD_LOCKS)
> + fsp->use_ofd_locks = true;
> + if (lp_parm_bool(SNUM(conn),
> + "smbd",
> + "force process locks",
> + false)) {
> + fsp->use_ofd_locks = false;
> + }
> +#endif
> fsp->fh->ref_count = 1;
> fsp->fh->fd = -1;
>
> --
> 2.20.1
>
>
> From e9f50120368b731ea921f1d032211ca5e96d1014 Mon Sep 17 00:00:00 2001
> From: Stefan Metzmacher <metze at samba.org>
> Date: Mon, 28 Jan 2019 11:31:05 +0100
> Subject: [PATCH 3/5] DEBUG source3/modules/vfs_fruit.c
>
> ---
> source3/lib/util.c | 2 ++
> source3/modules/vfs_fruit.c | 10 ++++++++++
> source3/smbd/files.c | 2 ++
> 3 files changed, 14 insertions(+)
>
> diff --git a/source3/lib/util.c b/source3/lib/util.c
> index 7530ea67973..48b763c7241 100644
> --- a/source3/lib/util.c
> +++ b/source3/lib/util.c
> @@ -1097,11 +1097,13 @@ int map_process_lock_to_ofd_lock(int op)
> default:
> return -1;
> }
> + DBG_ERR("XXXXXX OFD locks supported!\n");
> return op;
> }
> #else /* HAVE_OFD_LOCKS */
> int map_process_lock_to_ofd_lock(int op)
> {
> + DBG_ERR("XXXXXX OFD locks NOT supported!\n");
> return op;
> }
> #endif /* HAVE_OFD_LOCKS */
> diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
> index 0a67ad39d8e..425cb69c44f 100644
> --- a/source3/modules/vfs_fruit.c
> +++ b/source3/modules/vfs_fruit.c
> @@ -2650,6 +2650,16 @@ static bool test_netatalk_lock(files_struct *fsp, off_t in_offset)
> pid_t pid = 0;
>
> result = SMB_VFS_GETLOCK(fsp, &offset, &len, &type, &pid);
> +
> + DBG_ERR("XXXXXX in_offset[0x%016llx] offset[0x%016llx] result[%u] "
> + "len[%llu] type[%d => %d]\n",
> + (unsigned long long)in_offset,
> + (unsigned long long)offset,
> + result,
> + (unsigned long long)len,
> + F_WRLCK,
> + type);
> +
> if (result == false) {
> return true;
> }
> diff --git a/source3/smbd/files.c b/source3/smbd/files.c
> index 99b4937c99b..e4a412f23f7 100644
> --- a/source3/smbd/files.c
> +++ b/source3/smbd/files.c
> @@ -60,6 +60,8 @@ NTSTATUS fsp_new(struct connection_struct *conn, TALLOC_CTX *mem_ctx,
> fsp->use_ofd_locks = false;
> }
> #endif
> + DBG_ERR("XXXXXX OFD locks have been %s\n",
> + fsp->use_ofd_locks ? "enabled" : "disabled");
> fsp->fh->ref_count = 1;
> fsp->fh->fd = -1;
>
> --
> 2.20.1
>
>
> From 1873c4ae77c4921c2f265a313623560cc99da523 Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Wed, 30 Jan 2019 18:47:49 +0100
> Subject: [PATCH 4/5] WORKAROUND: Disabling OFD locks with netatalk fixes the
> test
>
> make -j8 test TESTS="samba3.vfs.fruit_netatalk.nt4"
> ---
> source3/modules/vfs_fruit.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
> index 425cb69c44f..e60b62c2b69 100644
> --- a/source3/modules/vfs_fruit.c
> +++ b/source3/modules/vfs_fruit.c
> @@ -2649,6 +2649,9 @@ static bool test_netatalk_lock(files_struct *fsp, off_t in_offset)
> int type = F_WRLCK;
> pid_t pid = 0;
>
> + /* Disable OFD locks for netatalk locks as they don't work */
> + fsp->use_ofd_locks = false;
> +
> result = SMB_VFS_GETLOCK(fsp, &offset, &len, &type, &pid);
>
> DBG_ERR("XXXXXX in_offset[0x%016llx] offset[0x%016llx] result[%u] "
> --
> 2.20.1
>
>
> From adc31f0302c8a5f79f43afbb17695f6bfcd0f338 Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Wed, 30 Jan 2019 19:09:50 +0100
> Subject: [PATCH 5/5] Revert "WORKAROUND: Disabling OFD locks with netatalk
> fixes the test"
>
> This reverts commit 431c67fee7f930f5a43d40031c092be6e252c2e4.
> ---
> source3/modules/vfs_fruit.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
> index e60b62c2b69..425cb69c44f 100644
> --- a/source3/modules/vfs_fruit.c
> +++ b/source3/modules/vfs_fruit.c
> @@ -2649,9 +2649,6 @@ static bool test_netatalk_lock(files_struct *fsp, off_t in_offset)
> int type = F_WRLCK;
> pid_t pid = 0;
>
> - /* Disable OFD locks for netatalk locks as they don't work */
> - fsp->use_ofd_locks = false;
> -
> result = SMB_VFS_GETLOCK(fsp, &offset, &len, &type, &pid);
>
> DBG_ERR("XXXXXX in_offset[0x%016llx] offset[0x%016llx] result[%u] "
> --
> 2.20.1
>
More information about the samba-technical
mailing list