[linux-cifs-client] Re: Re: Timestamps not preserved on CIFS mount

[Günter Kukkukk]

Am Samstag, 8. März 2008 schrieb Eric B.:

> Ahhh - that's a good question.  The truth is I am not sure.  It is a Maxtor 
> Shared Storage device that is running a version of Linux as firmware, but 
> have no idea which kernel version, nor what version of samba.  Is there 
> anyway to remotely query the device to find out?  There is no telnet/ssh 
> access to the unit - just a simple web interface that doesn't give any 
> version information.
> 
> The firmware dates back to 2006-02-01, so I can be sure that it is based on 
> technology older than that, but no ideas what version it might be.
> 
> Any hints on how to find out?

Sorry, no idea atm.
Interesting coincidence, cause some days ago we discussed this failure
on irc channel #samba-os2. There the main focus was on how to improve
the vifs vfs legacy support.

Mounting an OS/2 server and doing

linux700:/mnt/server/gktest # cp -p /home/gk/test.c .
cp: Erhalten der Zeiten für ´./test.c´: Das Argument ist ungültig
(cp: Preserving times for './test.c': Invalid argument)

This works as expected, cause the needed legacy functions are not
implemented right now - which i'm working on.

Using the most recent cifs git tree, i noticed, that the current
usage of setpathinfo and setfileinfo is also not working
when running against winnt or winxp servers!
No error is returned, but the time stamps are not preserved.

When using the following command sequence, when cwd is the mount point

stat /home/gk/test.c && cp -p /home/gk/test.c . && sleep 2 && stat test.c

one can easily see, that the time stamps are not preserved.
A bit strange is the needed usuage of "sleep". When not used,
the immediate stat info seems to be fetched from some cached inode info
and/or some revalidating has not finished.... (?)
 
My current changes can be viewed from the diff below.
They are working here against winNT and winXP!  :-)

More work needs to be done, cause a similar calling sequence
is used in the cifs_unlink() function.

Cheers, Günter

--------------------
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 24eb4d3..4ae8391 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -1591,42 +1591,34 @@ int cifs_setattr(struct dentry *direntry, struct iattr *attrs)
 
 	if (set_time || set_dosattr) {
 		time_buf.CreationTime = 0;	/* do not change */
-		/* In the future we should experiment - try setting timestamps
-		   via Handle (SetFileInfo) instead of by path */
-		if (!(pTcon->ses->flags & CIFS_SES_NT4))
-			rc = CIFSSMBSetTimes(xid, pTcon, full_path, &time_buf,
-					     cifs_sb->local_nls,
-					     cifs_sb->mnt_cifs_flags &
-						CIFS_MOUNT_MAP_SPECIAL_CHR);
-		else
-			rc = -EOPNOTSUPP;
-
-		if (rc == -EOPNOTSUPP) {
-			int oplock = FALSE;
-			__u16 netfid;
-
-			cFYI(1, ("calling SetFileInfo since SetPathInfo for "
-				 "times not supported by this server"));
-			/* BB we could scan to see if we already have it open
-			   and pass in pid of opener to function */
-			rc = CIFSSMBOpen(xid, pTcon, full_path, FILE_OPEN,
-					 SYNCHRONIZE | FILE_WRITE_ATTRIBUTES,
-					 CREATE_NOT_DIR, &netfid, &oplock,
-					 NULL, cifs_sb->local_nls,
-					 cifs_sb->mnt_cifs_flags &
-						CIFS_MOUNT_MAP_SPECIAL_CHR);
-			if (rc == 0) {
-				rc = CIFSSMBSetFileTimes(xid, pTcon, &time_buf,
-							 netfid);
-				CIFSSMBClose(xid, pTcon, netfid);
-			} else {
-			/* BB For even older servers we could convert time_buf
-			   into old DOS style which uses two second
-			   granularity */
-
-			/* rc = CIFSSMBSetTimesLegacy(xid, pTcon, full_path,
-					&time_buf, cifs_sb->local_nls); */
-			}
+		/*
+		   I have spent hours to track this down with servers
+		   like winNT and winXP.
+		   CONCLUSION:
+		   When using "cp -p srcfile /mnt/tmp/" to preserve time stamps
+		   (originated from utimes()), both former used alternatives
+		   - CIFSSMBSetTimes() to use setpathinfo
+		   - CIFSSMBOpen() -> CIFSSMBSetFileTimes() -> CIFSSMBClose()
+                     to use setfileinfo
+		   do not work properly.
+		   Windows _is_ returning success on both variants, but _no_
+		   time stamps have been set on the remote server!
+		   It seems to be problematic to call these functions, when the
+		   dest file is already open.
+		   This new version is working for me.
+		   */
+		open_file = find_writable_file(cifsInode);
+		if (open_file) {
+		    cFYI(1,("cifs_setattr - open_file OK! Calling setfileinfo now!"));
+		    rc = CIFSSMBSetFileTimes(xid, pTcon, &time_buf, 
+					     open_file->netfid);
+		    atomic_dec(&open_file->wrtPending);
+		} else {
+		    cFYI(1,("cifs_setattr - calling setpathinfo now!"));
+		    rc = CIFSSMBSetTimes(xid, pTcon, full_path, &time_buf,
+				     cifs_sb->local_nls,
+				     cifs_sb->mnt_cifs_flags &
+				     CIFS_MOUNT_MAP_SPECIAL_CHR);
 		}
 		/* Even if error on time set, no sense failing the call if
 		the server would set the time to a reasonable value anyway,
-------------------
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 194 bytes
Desc: This is a digitally signed message part.
Url : http://lists.samba.org/archive/linux-cifs-client/attachments/20080308/0e166e0b/attachment.bin