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

[Niels de Vos]

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

Thanks,
Niels
-------------- next part --------------
>From 98aa32d0f356ddd0b3d59d856e9f5ceecc110535 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 mtime

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).

It is very unfortunate that libgfapi does not have a function that can
be used to only set the mtime (or atime for that matter).

On the other hand, there is little sense in updating the mtime without
modifying the atime too. The practical solution is to use the mtime for
the atime as well, but only in the case the atime->tv_sec is set to -1.

Signed-off-by: Niels de Vos <ndevos at redhat.com>
Reviewed-by: Ira Cooper <ira at samba.org>
---
 source3/modules/vfs_glusterfs.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c
index 3262f11..3cb7e1d 100644
--- a/source3/modules/vfs_glusterfs.c
+++ b/source3/modules/vfs_glusterfs.c
@@ -734,8 +734,16 @@ 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;
+	if (ft->atime.tv_sec != -1) {
+		times[0].tv_sec = ft->atime.tv_sec;
+		times[0].tv_nsec = ft->atime.tv_nsec;
+	} else {
+		/* actually atime should not get set, but there is no glfs_*
+		   function that can be used to only set the mtime */
+		times[0].tv_sec = ft->mtime.tv_sec;
+		times[0].tv_nsec = ft->mtime.tv_nsec;
+	}
+
 	times[1].tv_sec = ft->mtime.tv_sec;
 	times[1].tv_nsec = ft->mtime.tv_nsec;
 
-- 
1.7.1