Corinna Vinschen <corinna-cygwin <at> cygwin.com> writes: > Just to be sure. This does not occur with cp -p on other filesystems > than MVFS, right? I don't see that problem on NTFS or FAT.
I can confirm that with both NTFS and FAT, the NtSetInformationFile in utimens_fs is immediate (no fd has to be closed). There may be other buggy fs out there, but we'll deal with them as reports are issued. > > Or do we teach fchmod to honor pending > > timestamp changes? > > The problem is that the calls are stateless. The fchmod call doesn't > know about a former utimens call and vice versa. Worse, to do that > really correct you would not only have to keep track of the timestamp as > set by utimens, you would also have to keep track of them in case of > write. That's a can of worms if you ask me. Agreed. Forcing the utimes to disk (as already happens for decent fs) seems like the best approach for a fix. > Maybe(!) a FlushFileBuffer call will also flush metadata changes: Nice thought, but no dice: the time never changed. > Other than that, I can only suggest a change like this one: > > Index: fhandler_disk_file.cc > =================================================================== > - NTSTATUS status = NtSetInformationFile (get_handle (), &io, &fbi, sizeof fbi, > - FileBasicInformation); > + if (pc.fs_is_mvfs () && !closeit) > + { As written, that one didn't quite do it either: $ cp -p bar bar1 cp: preserving times for `bar1': Bad file descriptor But I see why - you are using fh uninitialized. > + HANDLE fh; > + > + InitializeObjectAttributes (&attr, &ro_u_empty, pc.objcaseinsensitive (), > + fh, NULL); Yet the obvious fix of using get_handle() instead of fh doesn't help either. Before this patch, writable files were able to preserve times (because there was no fchmod in the loop), but after the patch, both writable and read-only are losing the timestamp update. The duplicated handle does just fine, but then closing the original handle undoes the change: Breakpoint 1, fhandler_base::utimens_fs (this=0x6128f004, tvp=0x22cb34) at ../../../../winsup/cygwin/fhandler_disk_file.cc:1315 1315 if (pc.fs_is_mvfs () && !closeit) Current language: auto; currently c++ (gdb) shell ls -l bar* # just created, prior to fchmod -r--r--r-- 1 eblake Domain Users 3 Jul 16 17:12 bar -rw-r--r-- 1 eblake Domain Users 3 Jul 20 10:31 bar1 (gdb) n 1320 InitializeObjectAttributes (&attr, &ro_u_empty, pc.objcaseinsensitive (), (gdb) 1323 FILE_SHARE_VALID_FLAGS, FILE_OPEN_FOR_BACKUP_INTENT); (gdb) Breakpoint 2, fhandler_base::utimens_fs (this=0x6128f004, tvp=0x22cb34) at ../../../../winsup/cygwin/fhandler_disk_file.cc:1324 1324 if (NT_SUCCESS (status)) (gdb) p status $1 = 0 (gdb) n 1327 FileBasicInformation); (gdb) Breakpoint 3, fhandler_base::utimens_fs (this=0x6128f004, tvp=0x22cb34) at ../../../../winsup/cygwin/fhandler_disk_file.cc:1328 1328 NtClose (fh); (gdb) shell ls -l bar* # changed, but fd not closed, so don't see it yet -r--r--r-- 1 eblake Domain Users 3 Jul 16 17:12 bar -rw-r--r-- 1 eblake Domain Users 3 Jul 20 10:31 bar1 (gdb) p status $2 = 0 (gdb) n Breakpoint 4, fhandler_base::utimens_fs (this=0x6128f004, tvp=0x22cb34) at ../../../../winsup/cygwin/fhandler_disk_file.cc:1334 1334 if (closeit) (gdb) p status $3 = 0 (gdb) shell ls -l bar* # all right - the time made it through -r--r--r-- 1 eblake Domain Users 3 Jul 16 17:12 bar -rw-r--r-- 1 eblake Domain Users 3 Jul 16 17:12 bar1 (gdb) b 848 Breakpoint 5 at 0x6103e89c: file ../../../../winsup/cygwin/fhandler_disk_file.cc, line 848. (gdb) b 850 Breakpoint 6 at 0x6103e8db: file ../../../../winsup/cygwin/fhandler_disk_file.cc, line 850. (gdb) c Continuing. Breakpoint 5, fhandler_disk_file::fchmod (this=0x6128f004, mode=292) at ../../../../winsup/cygwin/fhandler_disk_file.cc:848 848 status = NtSetAttributesFile (get_handle (), pc.file_attributes ()); (gdb) shell ls -l bar* # now ready to do the fchmod -r--r--r-- 1 eblake Domain Users 3 Jul 16 17:12 bar -rw-r--r-- 1 eblake Domain Users 3 Jul 16 17:12 bar1 (gdb) n Breakpoint 6, fhandler_disk_file::fchmod (this=0x6128f004, mode=292) at ../../../../winsup/cygwin/fhandler_disk_file.cc:850 850 if (!pc.has_acls ()) (gdb) shell ls -l bar* # good: the w bit is lost, but timestamps don't change -r--r--r-- 1 eblake Domain Users 3 Jul 16 17:12 bar -r--r--r-- 1 eblake Domain Users 3 Jul 16 17:12 bar1 (gdb) c Continuing. Program exited normally. (gdb) shell ls -l bar* # oh dear - closing the original fd changed time -r--r--r-- 1 eblake Domain Users 3 Jul 16 17:12 bar -r--r--r-- 1 eblake Domain Users 3 Jul 20 10:37 bar1 So, setting the timestamp via closing an alternative handle works, but then we're back to the problem that closing the original handle corrupts the timestamp (and worse, it set it to 10:37 of the handle close, instead of the 10:31 of the file create; whereas before the patch it least left the create time intact). I noticed that http://msdn.microsoft.com/en-us/library/dd852055.aspx states that "However, a driver or application can request that the file system not update one or more of these members for I/O operations that are performed on the caller's file handle by setting the appropriate members to –1." Maybe it would work to change the appropriate fields of the original handle to -1 at the same time we close the duplicate handle? Or will that inhibit correct timestamp modifications from subsequent reads on the open fd? Another thought - since we are able to see the timestamps updated as soon as we close the duplicated handle, and the fchmod didn't corrupt state (only the subsequent close), maybe we just need to special case close_fs for MVFS to also play games with handles, so that the last handle closed always has the desired times? Or maybe this is a case where we need a FlushFileBuffer call during the utimens_fs after all, in addition to closing the duplicate descriptor, so that the original handle no longer has any state that needs flushing which might inadvertently change timestamps. I guess my next step will be looking at procmon output, to see if I can make sense of why timestamps are changing as MVFS forwards operations onto the backing store fs. -- Eric Blake -- Problem reports: http://cygwin.com/problems.html FAQ: http://cygwin.com/faq/ Documentation: http://cygwin.com/docs.html Unsubscribe info: http://cygwin.com/ml/#unsubscribe-simple