Good pickup, Reviewed-by: Ira Cooper <i...@samba.org>
-Ira On Wed, Jan 15, 2014 at 5:17 AM, Niels de Vos <nde...@redhat.com> wrote: > On Tue, Jan 14, 2014 at 02:56:16PM -0800, Jeremy Allison wrote: > > On Mon, Jan 13, 2014 at 11:26:42AM +0100, Niels de Vos wrote: > > > Hello, > > > > > > we were informed that when using Samba+vfs_glusterfs a normal 'write' > to > > > a file does not correctly set the 'atime'. In fact, the 'atime' is set > > > to "2106-02-07 11:58:15.000000000 +0530" according to 'stat'. > > > > > > Tracing the network and checking the SMB/SET_FILE_INFO shows that the > > > Linux CIFS client sends the following date: > > > - May 28, 60056 07:36:10.955161500 CEST > > > > > > This date, in fact, shows as 0xffffffff (32-bit -1) in the packet > > > capture. The Linux CIFS client sets NO_CHANGE_64 (0xffffffffffffffff, > > > 64-bit -1) when it intends to not modify the 'atime' (fs/cifs/inode.c). > > > > > > Debugging the Samba/vfs_glusterfs module with systemtap, showed that > the > > > 'write' that triggers the unexpected 'atime', indeed gets forwarded to > > > the GlusterFS volume as -1. Obviously, Samba passes a -1 for the > 'atime' > > > when it should not get modified. Unfortunately the gfapi library does > > > not expose a function to set the 'mtime' only, it is always needed to > > > set the 'atime' as well (like 'utimes()'). > > > > > > The attached patch fixes setting the 'atime' to a value way in the > > > future. It resolves to setting the 'atime' to the same value as the > > > 'mtime', whenever the 'atime' is set to -1. > > > > > > Downstream bug report and detailed results of the debugging: > > > - https://bugzilla.redhat.com/1051226 > > > > Actually I have to vote NAK on this patch, as it's > > not doing the expected thing. > > > > Sending -1 means no atime change, and the current > > atime is available to you in the smb_fname->st struct > > passed into the ntimes VFS call. > > > > So I suggest you copy the code inside vfswrap_ntimes() > > in source3/modules/vfs_default.c and do: > > > > if (null_timespec(ft->atime)) { > > ft->atime= smb_fname->st.st_ex_atime; > > } > > > > if (null_timespec(ft->mtime)) { > > ft->mtime = smb_fname->st.st_ex_mtime; > > } > > > > if (!null_timespec(ft->create_time)) { > > set_create_timespec_ea(handle->conn, > > smb_fname, > > ft->create_time); > > } > > > > if ((timespec_compare(&ft->atime, > > &smb_fname->st.st_ex_atime) == 0) && > > (timespec_compare(&ft->mtime, > > &smb_fname->st.st_ex_mtime) == 0)) { > > return 0; > > } > > > > instead. > > That makes perfect sense to me. Thanks for the suggestion! The attached > patch has been tested successfully too. > > Niels >
_______________________________________________ Gluster-devel mailing list Gluster-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/gluster-devel
