[linux-cifs-client] Re: timestamp problems with cp -p

[Guenter Kukkukk]

Am Donnerstag, 13. März 2008 schrieb Steve French:
> >  kukks: smfrench: the needed "sleep 1" in "stat localfile && cp -p localfile /mnt && sleep 1 && stat /mnt/localfile" is also a bit strange
> The 1 second timeout is needed since the local cache of the inode
> timestamps is kept for 1 second ... (and the write and setattr were in
> the correct order locally but the incorrect order remotely - so when
> we revalidated the cached inode it was wrong - the local copy of the
> timestamps would probably be fine as long as the file was open and
> oplocked)
> 
> > kukks: smfrench: i had already posted another solution on the cifs mailing list for inode.c ---> cifs_setattr(), which is not patching cifs_unlink atm, where the same sequence for setpathinfo/setfileinfo is used.
> 
> What is this issue? utimes itself fails to Windows when file open? I
> see http://rafb.net/p/0bzb2Y13.html but I don't recognize the high
> level problem.  Will trace it to Windows tomorrow.

about a week ago i noticed, that 'cp -p ...' was not working, but
did my tests only against winNT and winXP.
Wrote that patch (see below) - and it worked for me...

Later i also did a test against samba3_2 using cifs.ko _not_ including
my patch - and noticed the ACL problem, we were talking about on irc.
So i used the "noacl" mount option to skip that acl settings, to just
test the time stamps problem - which was showing up, too.
Was just wondering that days about the necessity of "sleep 1", which i know
better now!  :-)

As a last step i was testing against samba3_2 _including_ my cifs.ko patch -
and it was working for me!

As others noted recently, a different approach is needed at all...

--
Cause the pastebin might vanish very soon, i just add that changes here again:

------------------------------
diff against a version some days ago:
 
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,