This is the mail archive of the cygwin mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: MVFS results


On Jul 20 16:58, Eric Blake wrote:
> Corinna Vinschen <corinna-cygwin <at> cygwin.com> writes:
> > +      InitializeObjectAttributes (&attr, &ro_u_empty, pc.objcaseinsensitive 
> (),
> > +				  fh, NULL);
> 
> Yet the obvious fix of using get_handle() instead of fh doesn't help either.  

Ouch, right.  Did it show that I just typed away and didn't test it?

> 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:
> [...]
> 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).

Oh boy, how broken is that?

> 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?

That's how I understand setting the members to -1.  As far as one can
surmise from the behaviour of MVFS as seen up to here, this would result
in some other unwanted behaviour.  I'm on the verge of giving up on MVFS.

> 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?

I don't like the sound of it.  YA solution which requires to store state
information.  Storing the file attributes in fhandler is already not
quite correct because they can so easily be changed by another piece of
code, another application.

> 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.

That should be handled by the filesystem, in theory.

> 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.

The easiest solution is probably not to use fchmod on MVFS, but to do
the same trick in fchmod as in utimens:

Index: fhandler_disk_file.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/fhandler_disk_file.cc,v
retrieving revision 1.302
diff -u -p -r1.302 fhandler_disk_file.cc
--- fhandler_disk_file.cc	16 Jul 2009 15:28:57 -0000	1.302
+++ fhandler_disk_file.cc	20 Jul 2009 17:45:08 -0000
@@ -845,7 +845,23 @@ fhandler_disk_file::fchmod (mode_t mode)
   if (S_ISSOCK (mode))
     pc |= (DWORD) FILE_ATTRIBUTE_SYSTEM;
 
-  status = NtSetAttributesFile (get_handle (), pc.file_attributes ());
+  if (pc.fs_is_mvfs () && !oret)
+    {
+      OBJECT_ATTRIBUTES attr;
+      HANDLE fh;
+
+      InitializeObjectAttributes (&attr, &ro_u_empty, pc.objcaseinsensitive (),
+				  get_handle (), NULL);
+      status = NtOpenFile (&fh, FILE_WRITE_ATTRIBUTES, &attr, &io,
+			   FILE_SHARE_VALID_FLAGS, FILE_OPEN_FOR_BACKUP_INTENT);
+      if (NT_SUCCESS (status))
+	{
+	  status = NtSetAttributesFile (fh, pc.file_attributes ());
+	  NtClose (fh);
+	}
+    }
+  else
+    status = NtSetAttributesFile (get_handle (), pc.file_attributes ());
   /* Correct NTFS security attributes have higher priority */
   if (!pc.has_acls ())
     {


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

--
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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]