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