gluster-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Gluster-devel] [PATCH v2] vfs/glusterfs: in case atime is not passed, s


From: Niels de Vos
Subject: [Gluster-devel] [PATCH v2] vfs/glusterfs: in case atime is not passed, set it to the current atime
Date: Wed, 15 Jan 2014 11:17:31 +0100
User-agent: Mutt/1.5.20 (2009-12-10)

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

Attachment: vfs-glusterfs-in-case-atime-is-not-passed-set-it-to-the-current-atime.patch
Description: Text document


reply via email to

[Prev in Thread] Current Thread [Next in Thread]