Samba and Linux OFD locks
Jeff Layton
jlayton at samba.org
Thu Jan 31 14:49:02 UTC 2019
On Wed, 2019-01-30 at 17:21 -0800, Jeremy Allison via samba-technical
wrote:
> 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.
>
l_pid is correct -- it's -1.
Because OFD locks aren't associated with a process, we can't reasonably
report l_pid. The file description could be shared across processes, so
we can't reliably say what pid "owns" it.
fwiw: the -1 comes from BSD where flock and fcntl locks can conflict,
and BSD always reports -1 for l_pid when the conflicting lock is a flock
lock.
> > 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
> >
>
>
--
Jeff Layton <jlayton at samba.org>
More information about the samba-technical
mailing list