Mail Archives: cygwin/2009/07/20/12:59:17
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
- Raw text -