Write times fix for S3
Jeremy Allison
jra at samba.org
Sat Sep 6 02:00:36 GMT 2008
Ok, here's the fix for the write times breakage
with the new tests in S4 smbtorture.
The key is keeping in the share mode struct
the "old_file_time" as the real write time,
set by all the write and allocation calls,
and the "changed_write_time" as the "sticky"
write time - set by the SET_FILE_TIME calls.
We can set them independently (although I
kept the optimization of not setting the
"old_file_time" is a "changed_write_time"
was already set, as we'll never see it.
This allows us to update the write time
immediately on the SMBwrite truncate case,
SET_END_OF_FILE and SET_ALLOCATION_SIZE calls,
whilst still have the 2 second delay on the
"normal" SMBwrite, SMBwriteX calls.
I think in a subsequent patch I'd like to
change the name of these from "old_file_time"
to "write_time" and "changed_write_time" to
"sticky_write_time" to make this clearer.
I think I also fixed a bug in Metze's original
code in that once a write timestamp had been
set from a "normal" SMBwriteX call the fsp->update_write_time_triggered
variable was set and then never reset - thus
meaning the write timestamp would never get
updated again on subsequent SMBwriteX's.
The new code checks the update_write_time_event
event instead, and doesn't update is there's
an event already scheduled.
Metze especially, please check this over for
your understanding.
Jeremy.
-------------- next part --------------
diff --git a/source/include/proto.h b/source/include/proto.h
index 291afac..e94a217 100644
--- a/source/include/proto.h
+++ b/source/include/proto.h
@@ -5306,8 +5306,8 @@ void set_delete_on_close_token(struct share_mode_lock *lck, UNIX_USER_TOKEN *tok
void set_delete_on_close_lck(struct share_mode_lock *lck, bool delete_on_close, UNIX_USER_TOKEN *tok);
bool set_delete_on_close(files_struct *fsp, bool delete_on_close, UNIX_USER_TOKEN *tok);
bool set_allow_initial_delete_on_close(struct share_mode_lock *lck, files_struct *fsp, bool delete_on_close);
-bool set_write_time(struct file_id fileid, struct timespec write_time,
- bool overwrite);
+bool set_sticky_write_time(struct file_id fileid, struct timespec write_time);
+bool set_write_time(struct file_id fileid, struct timespec write_time);
int share_mode_forall(void (*fn)(const struct share_mode_entry *, const char *,
const char *, void *),
void *private_data);
@@ -9616,11 +9616,10 @@ int file_set_dosmode(connection_struct *conn, const char *fname,
const char *parent_dir,
bool newfile);
int file_ntimes(connection_struct *conn, const char *fname, const struct timespec ts[2]);
-bool set_write_time_path(connection_struct *conn, const char *fname,
- struct file_id fileid, const struct timespec mtime,
- bool overwrite);
-bool set_write_time_fsp(struct files_struct *fsp, const struct timespec mtime,
- bool overwrite);
+bool set_sticky_write_time_path(connection_struct *conn, const char *fname,
+ struct file_id fileid, const struct timespec mtime);
+bool set_sticky_write_time_fsp(struct files_struct *fsp, const struct timespec mtime);
+bool update_write_time(struct files_struct *fsp);
/* The following definitions come from smbd/error.c */
@@ -9665,6 +9664,7 @@ bool directory_has_default_acl(connection_struct *conn, const char *fname);
ssize_t read_file(files_struct *fsp,char *data,SMB_OFF_T pos,size_t n);
void trigger_write_time_update(struct files_struct *fsp);
+void trigger_write_time_update_immediate(struct files_struct *fsp);
ssize_t write_file(struct smb_request *req,
files_struct *fsp,
const char *data,
diff --git a/source/include/smb.h b/source/include/smb.h
index c8c4f8c..d450eb5 100644
--- a/source/include/smb.h
+++ b/source/include/smb.h
@@ -456,7 +456,6 @@ typedef struct files_struct {
uint32 access_mask; /* NTCreateX access bits (FILE_READ_DATA etc.) */
uint32 share_access; /* NTCreateX share constants (FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE). */
- bool update_write_time_triggered;
struct timed_event *update_write_time_event;
bool update_write_time_on_close;
struct timespec close_write_time;
diff --git a/source/locking/locking.c b/source/locking/locking.c
index ca61a88..f1f9278 100644
--- a/source/locking/locking.c
+++ b/source/locking/locking.c
@@ -1405,22 +1405,21 @@ bool set_allow_initial_delete_on_close(struct share_mode_lock *lck, files_struct
return True;
}
-bool set_write_time(struct file_id fileid, struct timespec write_time,
- bool overwrite)
+bool set_sticky_write_time(struct file_id fileid, struct timespec write_time)
{
struct share_mode_lock *lck;
- DEBUG(5,("set_write_time: %s overwrite=%d id=%s\n",
+ DEBUG(5,("set_sticky_write_time: %s id=%s\n",
timestring(debug_ctx(),
convert_timespec_to_time_t(write_time)),
- overwrite, file_id_string_tos(&fileid)));
+ file_id_string_tos(&fileid)));
lck = get_share_mode_lock(NULL, fileid, NULL, NULL, NULL);
if (lck == NULL) {
return False;
}
- if (overwrite || null_timespec(lck->changed_write_time)) {
+ if (timespec_compare(&lck->changed_write_time, &write_time) != 0) {
lck->modified = True;
lck->changed_write_time = write_time;
}
@@ -1429,6 +1428,30 @@ bool set_write_time(struct file_id fileid, struct timespec write_time,
return True;
}
+bool set_write_time(struct file_id fileid, struct timespec write_time)
+{
+ struct share_mode_lock *lck;
+
+ DEBUG(5,("set_sticky_write_time: %s id=%s\n",
+ timestring(debug_ctx(),
+ convert_timespec_to_time_t(write_time)),
+ file_id_string_tos(&fileid)));
+
+ lck = get_share_mode_lock(NULL, fileid, NULL, NULL, NULL);
+ if (lck == NULL) {
+ return False;
+ }
+
+ if (timespec_compare(&lck->old_write_time, &write_time) != 0) {
+ lck->modified = True;
+ lck->old_write_time = write_time;
+ }
+
+ TALLOC_FREE(lck);
+ return True;
+}
+
+
struct forall_state {
void (*fn)(const struct share_mode_entry *entry,
const char *sharepath,
diff --git a/source/smbd/dosmode.c b/source/smbd/dosmode.c
index 0ac3873..88c6a51 100644
--- a/source/smbd/dosmode.c
+++ b/source/smbd/dosmode.c
@@ -616,39 +616,51 @@ int file_ntimes(connection_struct *conn, const char *fname, const struct timespe
return ret;
}
-/*******************************************************************
- Change a filetime - possibly allowing DOS semantics.
-*******************************************************************/
+/******************************************************************
+ Force a "sticky" write time on a pathname. This will always be
+ returned on all future write time queries and set on close.
+******************************************************************/
-bool set_write_time_path(connection_struct *conn, const char *fname,
- struct file_id fileid, const struct timespec mtime,
- bool overwrite)
+bool set_sticky_write_time_path(connection_struct *conn, const char *fname,
+ struct file_id fileid, const struct timespec mtime)
{
if (null_timespec(mtime)) {
return true;
}
- if (!set_write_time(fileid, mtime, overwrite)) {
+ if (!set_sticky_write_time(fileid, mtime)) {
return false;
}
- /* in the overwrite case the caller should trigger the notify */
- if (!overwrite) {
- notify_fname(conn, NOTIFY_ACTION_MODIFIED,
- FILE_NOTIFY_CHANGE_LAST_WRITE, fname);
- }
-
return true;
}
-bool set_write_time_fsp(struct files_struct *fsp, const struct timespec mtime,
- bool overwrite)
+/******************************************************************
+ Force a "sticky" write time on an fsp. This will always be
+ returned on all future write time queries and set on close.
+******************************************************************/
+
+bool set_sticky_write_time_fsp(struct files_struct *fsp, const struct timespec mtime)
{
- if (overwrite) {
- fsp->write_time_forced = true;
- TALLOC_FREE(fsp->update_write_time_event);
+ fsp->write_time_forced = true;
+ TALLOC_FREE(fsp->update_write_time_event);
+
+ return set_sticky_write_time_path(fsp->conn, fsp->fsp_name,
+ fsp->file_id, mtime);
+}
+
+/******************************************************************
+ Update a write time immediately, without the 2 second delay.
+******************************************************************/
+
+bool update_write_time(struct files_struct *fsp)
+{
+ if (!set_write_time(fsp->file_id, timespec_current())) {
+ return false;
}
- return set_write_time_path(fsp->conn, fsp->fsp_name, fsp->file_id,
- mtime, overwrite);
+ notify_fname(fsp->conn, NOTIFY_ACTION_MODIFIED,
+ FILE_NOTIFY_CHANGE_LAST_WRITE, fsp->fsp_name);
+
+ return true;
}
diff --git a/source/smbd/fileio.c b/source/smbd/fileio.c
index 64cea7f..63850f2 100644
--- a/source/smbd/fileio.c
+++ b/source/smbd/fileio.c
@@ -176,28 +176,38 @@ static void update_write_time_handler(struct event_context *ctx,
const struct timeval *now,
void *private_data)
{
- files_struct *fsp = (files_struct *)private_data;
+ files_struct *fsp = (files_struct *)private_data;
- /* Remove the timed event handler. */
- TALLOC_FREE(fsp->update_write_time_event);
- DEBUG(5, ("Update write time on %s\n", fsp->fsp_name));
+ /* Remove the timed event handler. */
+ TALLOC_FREE(fsp->update_write_time_event);
+ DEBUG(5, ("Update write time on %s\n", fsp->fsp_name));
- /* change the write time if not already changed by someoneelse */
- set_write_time_fsp(fsp, timespec_current(), false);
+ /* change the write time if not already changed by someone else */
+ update_write_time(fsp);
}
+/*********************************************************
+ Schedule a write time update for WRITE_TIME_UPDATE_USEC_DELAY
+ in the future.
+*********************************************************/
+
void trigger_write_time_update(struct files_struct *fsp)
{
int delay;
if (fsp->write_time_forced) {
+ /* No point - "sticky" write times
+ * in effect.
+ */
return;
}
- if (fsp->update_write_time_triggered) {
+ if (fsp->update_write_time_event) {
+ /*
+ * No point - an event is already scheduled.
+ */
return;
}
- fsp->update_write_time_triggered = true;
delay = lp_parm_int(SNUM(fsp->conn),
"smbd", "writetimeupdatedelay",
@@ -212,6 +222,27 @@ void trigger_write_time_update(struct files_struct *fsp)
update_write_time_handler, fsp);
}
+void trigger_write_time_update_immediate(struct files_struct *fsp)
+{
+ if (fsp->write_time_forced) {
+ /*
+ * No point - "sticky" write times
+ * in effect.
+ */
+ return;
+ }
+
+ if (fsp->update_write_time_event) {
+ /*
+ * No point - an event is already scheduled.
+ */
+ return;
+ }
+
+ fsp->update_write_time_on_close = true;
+ update_write_time(fsp);
+}
+
/****************************************************************************
Write to a file.
****************************************************************************/
diff --git a/source/smbd/reply.c b/source/smbd/reply.c
index 16f8a5b..6933533 100644
--- a/source/smbd/reply.c
+++ b/source/smbd/reply.c
@@ -3810,9 +3810,11 @@ void reply_write(struct smb_request *req)
END_PROFILE(SMBwrite);
return;
}
- } else
+ trigger_write_time_update_immediate(fsp);
+ } else {
nwritten = write_file(req,fsp,data,startpos,numtowrite);
-
+ }
+
status = sync_file(conn, fsp, False);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(5,("reply_write: sync_file for %s returned %s\n",
diff --git a/source/smbd/trans2.c b/source/smbd/trans2.c
index 881b29c..8d839b6 100644
--- a/source/smbd/trans2.c
+++ b/source/smbd/trans2.c
@@ -4900,11 +4900,11 @@ NTSTATUS smb_set_file_time(connection_struct *conn,
time_to_asc(convert_timespec_to_time_t(ts[1])) ));
if (fsp != NULL) {
- set_write_time_fsp(fsp, ts[1], true);
+ set_sticky_write_time_fsp(fsp, ts[1]);
} else {
- set_write_time_path(conn, fname,
+ set_sticky_write_time_path(conn, fname,
vfs_file_id_from_sbuf(conn, psbuf),
- ts[1], true);
+ ts[1]);
}
}
@@ -4988,6 +4988,7 @@ static NTSTATUS smb_set_file_size(connection_struct *conn,
if (vfs_set_filelen(fsp, size) == -1) {
return map_nt_error_from_unix(errno);
}
+ trigger_write_time_update_immediate(fsp);
return NT_STATUS_OK;
}
@@ -5011,6 +5012,7 @@ static NTSTATUS smb_set_file_size(connection_struct *conn,
return status;
}
+ trigger_write_time_update_immediate(new_fsp);
close_file(new_fsp,NORMAL_CLOSE);
return NT_STATUS_OK;
}
@@ -5737,7 +5739,7 @@ static NTSTATUS smb_set_file_allocation_info(connection_struct *conn,
* This is equivalent to a write. Ensure it's seen immediately
* if there are no pending writes.
*/
- trigger_write_time_update(fsp);
+ trigger_write_time_update_immediate(fsp);
return NT_STATUS_OK;
}
@@ -5771,7 +5773,7 @@ static NTSTATUS smb_set_file_allocation_info(connection_struct *conn,
* This is equivalent to a write. Ensure it's seen immediately
* if there are no pending writes.
*/
- trigger_write_time_update(new_fsp);
+ trigger_write_time_update_immediate(new_fsp);
close_file(new_fsp,NORMAL_CLOSE);
return NT_STATUS_OK;
More information about the samba-technical
mailing list