[PATCH] file change time

Ralph Böhme slow at samba.org
Sun Sep 27 01:52:11 UTC 2015


On Sat, Sep 26, 2015 at 12:32:09AM -0700, Ralph Böhme wrote:
> On Fri, Sep 25, 2015 at 10:46:23PM -0700, Jeremy Allison wrote:
> > On Fri, Sep 25, 2015 at 10:42:27PM -0700, Jeremy Allison wrote:
> > > On Fri, Sep 25, 2015 at 10:14:08PM -0700, Jeremy Allison wrote:
> > > > On Fri, Sep 25, 2015 at 07:12:22PM -0700, slow at samba.org wrote:
> > > > > Hi,
> > > > > 
> > > > > just came across this one. Looks like we don't handle ctime properly,
> > > > > patch with test attached.
> > > > > 
> > > > > The test passes with Windows but fails with Samba without the fix.
> > > > 
> > > > Looks completely correct to me ! Mod times can be tricky though,
> > > > so I hope it passes a full make test :-).
> > > 
> > > Hmmm. Doing a full make test with this I get:
> > > 
> > > [293(1167)/1872 at 22m14s] samba3.base.delaywrite(ad_dc)
> > > smbtorture 4.4.0pre1-DEVELOPERBUILD
> > > Using seed 1443246040
> > > 
> > > Running test_finfo_after_write
> > > UNEXPECTED(failure): samba3.base.delaywrite.finfo update on close(ad_dc)
> > > REASON: Exception: Exception: ../source4/torture/basic/delaywrite.c:1228: change_time changed
> > > 
> > > Now delaywrite is notoriously flaky, but this may show
> > > that the update on this field isn't as simple as it
> > > looks.
> > 
> > Can you try running test_finfo_after_write on a
> > Windows VM and seeing if this test is still a
> > valid one ?
> 
> will do.

what about this one? Added logic so ctime is set to mtime in
update_stat_ex_mtime() which is called when setting mtime in a stat_ex
to the value from a share mode lock. This one passes all delaywrite
tests.

-Ralph

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de,mailto:kontakt@sernet.de
-------------- next part --------------
From 3fa42c30d858abde88a4ea572e42526f6a78aea5 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sat, 26 Sep 2015 01:59:05 +0200
Subject: [PATCH] smbd: change time is the newer of ctime and mtime

According to [MS-FSSA] 2.1.1.3 the last changed time for a file is
defined as

  LastChangeTime: The time that identifies when the file metadata or
  contents were last changed ...

but we currently use the filesystem mtime. With this change we correctly
return an updated change time when a file is renamed.

Use the existing logic for write time and set ctime to the stored mtime
from a locking record. I'm sure there are corner cases that would require
adding the full logic for ctime we have for mtime, but this is still an
improvement.

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/lib/system.c   | 2 ++
 source3/smbd/dosmode.c | 7 ++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/source3/lib/system.c b/source3/lib/system.c
index e54b946..e2e36d3 100644
--- a/source3/lib/system.c
+++ b/source3/lib/system.c
@@ -325,6 +325,8 @@ void update_stat_ex_mtime(struct stat_ex *dst,
 				struct timespec write_ts)
 {
 	dst->st_ex_mtime = write_ts;
+	/* Keep ctime in sync with mtime */
+	dst->st_ex_ctime = write_ts;
 
 	/* We may have to recalculate btime. */
 	if (dst->st_ex_calculated_birthtime) {
diff --git a/source3/smbd/dosmode.c b/source3/smbd/dosmode.c
index 72acd4e..bf6fbd4 100644
--- a/source3/smbd/dosmode.c
+++ b/source3/smbd/dosmode.c
@@ -1098,7 +1098,12 @@ struct timespec get_change_timespec(connection_struct *conn,
 				struct files_struct *fsp,
 				const struct smb_filename *smb_fname)
 {
-	return smb_fname->st.st_ex_mtime;
+	if (timespec_compare(&smb_fname->st.st_ex_mtime,
+			     &smb_fname->st.st_ex_ctime) > 0)
+	{
+		return smb_fname->st.st_ex_mtime;
+	}
+	return smb_fname->st.st_ex_ctime;
 }
 
 /****************************************************************************
-- 
2.1.0



More information about the samba-technical mailing list