[Gluster-devel] [PATCH v2] vfs/glusterfs: in case atime is not passed, set it to the current atime
Niels de Vos
ndevos at redhat.com
Wed Jan 15 10:17:31 UTC 2014
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
-------------- next part --------------
>From afc80e99bca7256b9765cffdb049b67847229e9f Mon Sep 17 00:00:00 2001
From: Niels de Vos <ndevos at redhat.com>
Date: Fri, 10 Jan 2014 16:26:18 +0100
Subject: [PATCH] vfs/glusterfs: in case atime is not passed, set it to the current atime
The Linux CIFS client does not pass an updated atime when a write() is
done. This causes the vfs/glusterfs module to set the atime to -1 on the
Gluster backend, resulting in an atime far in the future (year 2106).
Signed-off-by: Niels de Vos <ndevos at redhat.com>
---
source3/modules/vfs_glusterfs.c | 26 ++++++++++++++++++++++----
1 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c
index 3262f11..9bcd0cb 100644
--- a/source3/modules/vfs_glusterfs.c
+++ b/source3/modules/vfs_glusterfs.c
@@ -734,10 +734,28 @@ static int vfs_gluster_ntimes(struct vfs_handle_struct *handle,
{
struct timespec times[2];
- times[0].tv_sec = ft->atime.tv_sec;
- times[0].tv_nsec = ft->atime.tv_nsec;
- times[1].tv_sec = ft->mtime.tv_sec;
- times[1].tv_nsec = ft->mtime.tv_nsec;
+ if (null_timespec(ft->atime)) {
+ times[0].tv_sec = smb_fname->st.st_ex_atime.tv_sec;
+ times[0].tv_nsec = smb_fname->st.st_ex_atime.tv_nsec;
+ } else {
+ times[0].tv_sec = ft->atime.tv_sec;
+ times[0].tv_nsec = ft->atime.tv_nsec;
+ }
+
+ if (null_timespec(ft->mtime)) {
+ times[1].tv_sec = smb_fname->st.st_ex_mtime.tv_sec;
+ times[1].tv_nsec = smb_fname->st.st_ex_mtime.tv_nsec;
+ } else {
+ times[1].tv_sec = ft->mtime.tv_sec;
+ times[1].tv_nsec = ft->mtime.tv_nsec;
+ }
+
+ if ((timespec_compare(×[0],
+ &smb_fname->st.st_ex_atime) == 0) &&
+ (timespec_compare(×[1],
+ &smb_fname->st.st_ex_mtime) == 0)) {
+ return 0;
+ }
return glfs_utimens(handle->data, smb_fname->base_name, times);
}
--
1.7.1
More information about the Gluster-devel
mailing list