Re: [patch v2] fix truncate inode time modification breakage

[Nick Piggin <npiggin@suse.de>]

On Thu, Jun 03, 2010 at 11:28:25AM +0200, Miklos Szeredi wrote:
> On Thu, 3 Jun 2010, Nick Piggin wrote:
> > > Fuse philosophy is: each operation itself has to update times on files
> > > if necessary.  So it basically moves the responsibility to update
> > > [amc]time from the VFS into the filesystem.
> > > 
> > > This means the only place fuse is interested in ATTR_ATIME or
> > > ATTR_MTIME is for the utime* syscalls.
> > 
> > OK, makes sense. I wonder why you do ATTR_ATIME changes, though,
> > rather than just getting those too back from the filesystem?
> 
> It does that, through setting S_NOATIME and invalidating attributes
> after each op.

Oh ok.

 
> > > It also means that fuse always ignores ATTR_CTIME which is never set
> > > explicitly.
> > > 
> > > So I believe the current fuse code is correct.
> > 
> > After my patch to pass ATTR_MTIME|ATTR_CTIME from truncate(2), it is
> > not (because above won't return false, ie. it will change the mtime
> > for truncate).
> > 
> > Why not avoid all mtime updates except MTIME_SET?
> 
> Because utimes(NULL) will supply only ATTR_ATIME | ATTR_MTIME.

Good answer.

 
> I guess fuse should do something like this:
> 
> 	if (valid ^ (ATTR_MTIME | ATTR_CTIME | ATTR_SIZE) == 0)
> 		valid = ATTR_SIZE;

You'll have to be careful, truncate will pass other things down like
mode to get rid of suid.

If you just wanted to ignore mtime changes on truncate, then masking
it off would be the way to go I think.

if (valid & ATTR_SIZE)
  valid &= ~ATTR_SIZE;

Would you also want to do the same thing with suid kill bits from
truncate, then? Mask off ATTR_MODE and just read it back from the
server too?