[Gluster-devel] [PATCH] vfs/glusterfs: in case atime is not passed, set it to the mtime

[Jeremy Allison]

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.

Cheers,

Jeremy.