POSIX requires that utimensat/futimens must update ctime in all cases
where any change is made (it only exempts when both atime and mtime were
requested as UTIME_OMIT, where the file must exist but no change is made).
Unfortunately, when atime is specified and mtime is UTIME_OMIT, the
kernel mistakenly behaves like read(), by updating atime but not ctime.
This in turn caused a regression in coreutils 8.2, visible through 'touch -a':
http://lists.gnu.org/archive/html/bug-coreutils/2009-12/msg00171.html
Here is a simple program demonstrating the failure:
$ cat foo.c
#include <fcntl.h>
#include <unistd.h>
#include <sys/stat.h>
int
main ()
{
int fd = creat ("file", 0600);
struct stat st1, st2;
struct timespec t[2] = { { 1000000000, 0 }, { 0, UTIME_OMIT } };
fstat (fd, &st1);
sleep (1);
futimens (fd, t);
fstat (fd, &st2);
return st1.st_ctime == st2.st_ctime;
}
$ gcc -o foo foo.c -D_GNU_SOURCE
$ ./foo; echo $?
1
The exit status should have been 0.
GNU coreutils will end up working around the bug by calling fstat/[l]stat
prior to futimens/utimensat, and populating the mtime field with the
desired value rather than using UTIME_OMIT. But this is a pointless stat
call, which could be avoided if the kernel were fixed to comply with POSIX
by updating ctime even when mtime is UTIME_OMIT.
--
Don't work too hard, make some time for fun as well!
Eric Blake [email protected]
Eric Blake <[email protected]> writes:
> POSIX requires that utimensat/futimens must update ctime in all cases
> where any change is made (it only exempts when both atime and mtime were
> requested as UTIME_OMIT, where the file must exist but no change is made).
> Unfortunately, when atime is specified and mtime is UTIME_OMIT, the
> kernel mistakenly behaves like read(), by updating atime but not ctime.
> This in turn caused a regression in coreutils 8.2, visible through 'touch -a':
> http://lists.gnu.org/archive/html/bug-coreutils/2009-12/msg00171.html
>
> Here is a simple program demonstrating the failure:
> $ cat foo.c
> #include <fcntl.h>
> #include <unistd.h>
> #include <sys/stat.h>
> int
> main ()
> {
> int fd = creat ("file", 0600);
> struct stat st1, st2;
> struct timespec t[2] = { { 1000000000, 0 }, { 0, UTIME_OMIT } };
> fstat (fd, &st1);
> sleep (1);
> futimens (fd, t);
> fstat (fd, &st2);
> return st1.st_ctime == st2.st_ctime;
> }
> $ gcc -o foo foo.c -D_GNU_SOURCE
> $ ./foo; echo $?
> 1
>
> The exit status should have been 0.
>
> GNU coreutils will end up working around the bug by calling fstat/[l]stat
> prior to futimens/utimensat, and populating the mtime field with the
> desired value rather than using UTIME_OMIT. But this is a pointless stat
> call, which could be avoided if the kernel were fixed to comply with POSIX
> by updating ctime even when mtime is UTIME_OMIT.
I couldn't reproduce this with your test program on my machine (latest
linus tree). And that utime path looks like no problem, um..., can you
provide output of strace or something?
Thanks.
--
OGAWA Hirofumi <[email protected]>
According to OGAWA Hirofumi on 12/21/2009 12:31 AM:
>> This in turn caused a regression in coreutils 8.2, visible through 'touch -a':
>> http://lists.gnu.org/archive/html/bug-coreutils/2009-12/msg00171.html
>>
>> GNU coreutils will end up working around the bug by calling fstat/[l]stat
>> prior to futimens/utimensat, and populating the mtime field with the
>> desired value rather than using UTIME_OMIT. But this is a pointless stat
>> call, which could be avoided if the kernel were fixed to comply with POSIX
>> by updating ctime even when mtime is UTIME_OMIT.
>
> I couldn't reproduce this with your test program on my machine (latest
> linus tree). And that utime path looks like no problem, um..., can you
> provide output of strace or something?
$ uname -a
Linux fencepost 2.6.26-2-xen-amd64 #1 SMP Thu Nov 5 04:27:12 UTC 2009
x86_64 GNU/Linux
$ strace ./foo
execve("./foo", ["./foo"], [/* 19 vars */]) = 0
brk(0) = 0x16fc000
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0x7fa7d94eb000
access("/etc/ld.so.nohwcap", F_OK) = -1 ENOENT (No such file or
directory)
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0x7fa7d94e9000
access("/etc/ld.so.preload", R_OK) = -1 ENOENT (No such file or
directory)
open("/etc/ld.so.cache", O_RDONLY) = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=42235, ...}) = 0
mmap(NULL, 42235, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7fa7d94de000
close(3) = 0
access("/etc/ld.so.nohwcap", F_OK) = -1 ENOENT (No such file or
directory)
open("/lib/libc.so.6", O_RDONLY) = 3
read(3, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\340\342"..., 832)
= 832
fstat(3, {st_mode=S_IFREG|0755, st_size=1436976, ...}) = 0
mmap(NULL, 3543672, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0)
= 0x7fa7d8f6d000
mprotect(0x7fa7d90c5000, 2097152, PROT_NONE) = 0
mmap(0x7fa7d92c5000, 20480, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x158000) = 0x7fa7d92c5000
mmap(0x7fa7d92ca000, 17016, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7fa7d92ca000
close(3) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0x7fa7d94dd000
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0x7fa7d94dc000
arch_prctl(ARCH_SET_FS, 0x7fa7d94dc6e0) = 0
mprotect(0x7fa7d92c5000, 12288, PROT_READ) = 0
munmap(0x7fa7d94de000, 42235) = 0
creat("file", 0600) = 3
fstat(3, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
rt_sigprocmask(SIG_BLOCK, [CHLD], [], 8) = 0
rt_sigaction(SIGCHLD, NULL, {SIG_DFL}, 8) = 0
rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
nanosleep({1, 0}, {1, 0}) = 0
syscall_280(0x3, 0, 0x7fff707fb660, 0, 0, 0x7fff707fb360, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) = 0
fstat(3, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
exit_group(1) = ?
Process 13846 detached
$ stat file
File: `file'
Size: 0 Blocks: 0 IO Block: 4096 regular empty file
Device: 811h/2065d Inode: 1769273205 Links: 1
Access: (0600/-rw-------) Uid: ( 1267/ ericb) Gid: ( 1267/ ericb)
Access: 2001-09-08 21:46:40.000000000 -0400
Modify: 2009-12-21 08:06:03.425326531 -0500
Change: 2009-12-21 08:06:03.425326531 -0500
$
Note that ctime is still stuck at the same second as mtime, even though it
should be at least one second later since there was a sleep(1) in between
the file creation and the utimensat that adjusted the atime.
However, when I tried on a different machine, I did not see the failure:
$ uname -a
Linux vladim 2.6.31.6-166.fc12.i686 #1 SMP Wed Dec 9 11:14:59 EST 2009
i686 GNU/Linux
So it may be architecture-dependent. The coreutils report was against:
> Host triplet: i686-pc-linux-gnu
> coreutils: 8.2
> kernel: 2.6.32.1
but I don't have access to a machine running that new of a kernel at the
moment.
--
Don't work too hard, make some time for fun as well!
Eric Blake [email protected]
According to Eric Blake on 12/21/2009 6:12 AM:
It may also be file-system dependent. On the machine where I saw the
original failure:
> $ uname -a
> Linux fencepost 2.6.26-2-xen-amd64 #1 SMP Thu Nov 5 04:27:12 UTC 2009
> x86_64 GNU/Linux
$ df -T .
Filesystem Type 1K-blocks Used Available Use% Mounted on
/dev/sdb1 xfs 419299328 269018656 150280672 65% /srv/data
But on the machine where the test case is working:
> $ uname -a
> Linux vladim 2.6.31.6-166.fc12.i686 #1 SMP Wed Dec 9 11:14:59 EST 2009
> i686 GNU/Linux
$ df -T .
Filesystem Type 1K-blocks Used Available Use% Mounted on
/dev/sda3 ext3 34123236 25894932 6494892 80% /
--
Don't work too hard, make some time for fun as well!
Eric Blake [email protected]
Eric Blake <[email protected]> writes:
According to OGAWA Hirofumi on 12/21/2009 12:31 AM:
>>> This in turn caused a regression in coreutils 8.2, visible through 'touch -a':
>>> http://lists.gnu.org/archive/html/bug-coreutils/2009-12/msg00171.html
>>>
>>> GNU coreutils will end up working around the bug by calling fstat/[l]stat
>>> prior to futimens/utimensat, and populating the mtime field with the
>>> desired value rather than using UTIME_OMIT. But this is a pointless stat
>>> call, which could be avoided if the kernel were fixed to comply with POSIX
>>> by updating ctime even when mtime is UTIME_OMIT.
>>
>> I couldn't reproduce this with your test program on my machine (latest
>> linus tree). And that utime path looks like no problem, um..., can you
>> provide output of strace or something?
> According to Eric Blake on 12/21/2009 6:12 AM:
>
> It may also be file-system dependent. On the machine where I saw the
> original failure:
>> $ uname -a
>> Linux fencepost 2.6.26-2-xen-amd64 #1 SMP Thu Nov 5 04:27:12 UTC 2009
>> x86_64 GNU/Linux
>
> $ df -T .
> Filesystem Type 1K-blocks Used Available Use% Mounted on
> /dev/sdb1 xfs 419299328 269018656 150280672 65% /srv/data
Thanks.
This is good point. This would be xfs issue or design. xfs seems to have
own special handling of ctime.
Cc: to xfs peoples. Any idea?
> But on the machine where the test case is working:
>> $ uname -a
>> Linux vladim 2.6.31.6-166.fc12.i686 #1 SMP Wed Dec 9 11:14:59 EST 2009
>> i686 GNU/Linux
>
> $ df -T .
> Filesystem Type 1K-blocks Used Available Use% Mounted on
> /dev/sda3 ext3 34123236 25894932 6494892 80% /
--
OGAWA Hirofumi <[email protected]>
According to OGAWA Hirofumi on 12/21/2009 8:05 AM:
>> It may also be file-system dependent. On the machine where I saw the
>> original failure:
>>> $ uname -a
>>> Linux fencepost 2.6.26-2-xen-amd64 #1 SMP Thu Nov 5 04:27:12 UTC 2009
>>> x86_64 GNU/Linux
>> $ df -T .
>> Filesystem Type 1K-blocks Used Available Use% Mounted on
>> /dev/sdb1 xfs 419299328 269018656 150280672 65% /srv/data
>
> Thanks.
>
> This is good point. This would be xfs issue or design. xfs seems to have
> own special handling of ctime.
Here's another report, this time about an mtime update not happening on
ntfs-3g. http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/19336
--
Don't work too hard, make some time for fun as well!
Eric Blake [email protected]
Eric Blake <[email protected]> writes:
> According to OGAWA Hirofumi on 12/21/2009 8:05 AM:
>>> It may also be file-system dependent. On the machine where I saw the
>>> original failure:
>>>> $ uname -a
>>>> Linux fencepost 2.6.26-2-xen-amd64 #1 SMP Thu Nov 5 04:27:12 UTC 2009
>>>> x86_64 GNU/Linux
>>> $ df -T .
>>> Filesystem Type 1K-blocks Used Available Use% Mounted on
>>> /dev/sdb1 xfs 419299328 269018656 150280672 65% /srv/data
>>
>> Thanks.
>>
>> This is good point. This would be xfs issue or design. xfs seems to have
>> own special handling of ctime.
>
> Here's another report, this time about an mtime update not happening on
> ntfs-3g. http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/19336
It is likely the issue of libfuse or ntfs-3g. I don't know about ntfs-3g
people at all. So, for now, just Cc: to fuse people.
> utimensat(0, NULL, {UTIME_OMIT, UTIME_NOW}, 0) = 0
>From this, "ia_valid" will have "ATTR_CTIME | ATTR_MTIME". And the
request would pass to userland via fuse of kernel part, then it will be
handled by libfuse.
>From quick grep of libfuse and ntfs-3g (would not be latest), ntfs-3g is
using "struct fuse_operations", not "struct fuse_lowlevel_ops".
So, fuse_lib_setattr() would be the handler for it in libfuse. And, from
the following part, it seems to require the both of MTIME and ATIME to
call ntfs-3g's ->utime handler.
I don't know whether it is limitation or bug in fuse_operations.
Any ideas?
if (!err &&
(valid & (FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME)) ==
(FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME)) {
struct timespec tv[2];
tv[0].tv_sec = attr->st_atime;
tv[0].tv_nsec = ST_ATIM_NSEC(attr);
tv[1].tv_sec = attr->st_mtime;
tv[1].tv_nsec = ST_MTIM_NSEC(attr);
err = fuse_fs_utimens(f->fs, path, tv);
}
Thanks.
--
OGAWA Hirofumi <[email protected]>
Hi all,
OGAWA Hirofumi wrote:
> Eric Blake<[email protected]> writes:
>
>
> It is likely the issue of libfuse or ntfs-3g. I don't know about ntfs-3g
> people at all. So, for now, just Cc: to fuse people.
>
Which ntfs-3g version are you using ?
>> utimensat(0, NULL, {UTIME_OMIT, UTIME_NOW}, 0) = 0
>>
>
Currently ntfs-3g does not set sub-second precision.
There is also a slight problem in the fuse interface :
the time buffer is never passed as NULL, consequently
in some circumstances ntfs-3g cannot decide correctly
over permissions. A permissive action is taken in this
situation.
> > From this, "ia_valid" will have "ATTR_CTIME | ATTR_MTIME". And the
> request would pass to userland via fuse of kernel part, then it will be
> handled by libfuse.
>
> > From quick grep of libfuse and ntfs-3g (would not be latest), ntfs-3g is
> using "struct fuse_operations", not "struct fuse_lowlevel_ops".
>
With the latest ntfs-3g, currently as a release candidate,
you can (optionally) use the low level fuse interface
http://pagesperso-orange.fr/b.andre/advanced-ntfs-3g.html
use the "lowntfs-3g" driver instead of "ntfs-3g"
Hope this helps
Regards
Jean-Pierre
Jean-Pierre Andr? <[email protected]> writes:
> Hi all,
Hi,
> OGAWA Hirofumi wrote:
>> Eric Blake<[email protected]> writes:
>>
>>
>> It is likely the issue of libfuse or ntfs-3g. I don't know about ntfs-3g
>> people at all. So, for now, just Cc: to fuse people.
>>
>
> Which ntfs-3g version are you using ?
I don't know which version is used by actual user. But, well, I've got
source by "apt-get source", and the version was 1:2009.4.4-1.
Now, I've got ntfs-3g-2009.11.14AC.2.tgz from specified url.
>>> utimensat(0, NULL, {UTIME_OMIT, UTIME_NOW}, 0) = 0
>
> Currently ntfs-3g does not set sub-second precision.
>
> There is also a slight problem in the fuse interface :
> the time buffer is never passed as NULL, consequently
> in some circumstances ntfs-3g cannot decide correctly
> over permissions. A permissive action is taken in this
> situation.
>
>> > From this, "ia_valid" will have "ATTR_CTIME | ATTR_MTIME". And the
>> request would pass to userland via fuse of kernel part, then it will be
>> handled by libfuse.
>>
>> > From quick grep of libfuse and ntfs-3g (would not be latest), ntfs-3g is
>> using "struct fuse_operations", not "struct fuse_lowlevel_ops".
>>
>
> With the latest ntfs-3g, currently as a release candidate,
> you can (optionally) use the low level fuse interface
> http://pagesperso-orange.fr/b.andre/advanced-ntfs-3g.html
> use the "lowntfs-3g" driver instead of "ntfs-3g"
Well, the problem seems in fuse_lib_setattr() and ntfs_fuse_setattr()
(lowlevel op too).
The both functions is requiring "ATIME | MTIME". Doesn't it mean the
ntfs-3g can't set only MTIME like above utimensat()?
In fuse_lib_setattr(),
if (!err && (valid & (FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME)) ==
(FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME)) {
In ntfs_fuse_setattr(),
case FUSE_SET_ATTR_ATIME + FUSE_SET_ATTR_MTIME :
res = ntfs_fuse_utime(&security, ino, attr, &stbuf);
Or I'm missing something?
Thanks.
--
OGAWA Hirofumi <[email protected]>
Hi again,
OGAWA Hirofumi wrote:
> Jean-Pierre Andr?<[email protected]> writes:
>
>
>> Hi all,
>>
> Hi,
>
>
>> OGAWA Hirofumi wrote:
>>
>>> Eric Blake<[email protected]> writes:
>>>
>>>
>>> It is likely the issue of libfuse or ntfs-3g. I don't know about ntfs-3g
>>> people at all. So, for now, just Cc: to fuse people.
>>>
>>>
>> Which ntfs-3g version are you using ?
>>
> I don't know which version is used by actual user. But, well, I've got
> source by "apt-get source", and the version was 1:2009.4.4-1.
>
> Now, I've got ntfs-3g-2009.11.14AC.2.tgz from specified url.
>
>
>>>> utimensat(0, NULL, {UTIME_OMIT, UTIME_NOW}, 0) = 0
>>>>
>> Currently ntfs-3g does not set sub-second precision.
>>
>> There is also a slight problem in the fuse interface :
>> the time buffer is never passed as NULL, consequently
>> in some circumstances ntfs-3g cannot decide correctly
>> over permissions. A permissive action is taken in this
>> situation.
>>
>>
>>>> From this, "ia_valid" will have "ATTR_CTIME | ATTR_MTIME". And the
>>>>
>>> request would pass to userland via fuse of kernel part, then it will be
>>> handled by libfuse.
>>>
>>>
>>>> From quick grep of libfuse and ntfs-3g (would not be latest), ntfs-3g is
>>>>
>>> using "struct fuse_operations", not "struct fuse_lowlevel_ops".
>>>
>>>
>> With the latest ntfs-3g, currently as a release candidate,
>> you can (optionally) use the low level fuse interface
>> http://pagesperso-orange.fr/b.andre/advanced-ntfs-3g.html
>> use the "lowntfs-3g" driver instead of "ntfs-3g"
>>
> Well, the problem seems in fuse_lib_setattr() and ntfs_fuse_setattr()
> (lowlevel op too).
>
> The both functions is requiring "ATIME | MTIME". Doesn't it mean the
> ntfs-3g can't set only MTIME like above utimensat()?
>
With ntfs-3g this is not directly possible, because
the interface does not provide flags telling which
timestamps should be updated. The only way would
be fuse feeding both values (even though unchanged)
before calling ntfs-3g. This is true for all versions of
ntfs-3g.
With lowntfs-3g (release candidate only), this could
be possible.... but this is not implemented, as the
case was never found up to now. I can provide you
with a patch,... if fuse can feed in the flags selectively.
> In fuse_lib_setattr(),
>
> if (!err&& (valid& (FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME)) ==
> (FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME)) {
>
> In ntfs_fuse_setattr(),
> case FUSE_SET_ATTR_ATIME + FUSE_SET_ATTR_MTIME :
> res = ntfs_fuse_utime(&security, ino, attr,&stbuf);
>
> Or I'm missing something?
>
> Thanks.
>
Regards
Jean-Pierre
On Mon, Dec 21, 2009 at 09:37:24PM -0700, Eric Blake wrote:
> According to OGAWA Hirofumi on 12/21/2009 8:05 AM:
> >> It may also be file-system dependent. On the machine where I saw the
> >> original failure:
> >>> $ uname -a
> >>> Linux fencepost 2.6.26-2-xen-amd64 #1 SMP Thu Nov 5 04:27:12 UTC 2009
> >>> x86_64 GNU/Linux
> >> $ df -T .
> >> Filesystem Type 1K-blocks Used Available Use% Mounted on
> >> /dev/sdb1 xfs 419299328 269018656 150280672 65% /srv/data
> >
> > Thanks.
> >
> > This is good point. This would be xfs issue or design. xfs seems to have
> > own special handling of ctime.
Yeah, it looks like the change to utimesat() back in 2.6.26 for
posix conformance made ATTR_CTIME appear outside inode truncation
and XFS wasn't updated for this change in behaviour at the VFS level.
Looks simple to fix, but I'm worried about introducing other
unintended ctime modifications - is there a test suite that checks
posix compliant atime/mtime/ctime behaviour around anywhere?
Cheers,
Dave.
--
Dave Chinner
[email protected]
According to Dave Chinner on 12/22/2009 5:34 AM:
> Yeah, it looks like the change to utimesat() back in 2.6.26 for
> posix conformance made ATTR_CTIME appear outside inode truncation
> and XFS wasn't updated for this change in behaviour at the VFS level.
> Looks simple to fix, but I'm worried about introducing other
> unintended ctime modifications - is there a test suite that checks
> posix compliant atime/mtime/ctime behaviour around anywhere?
Yes - the gnulib unit test, consisting of:
http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/nap.h
http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-utimens-common.h
http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-utimens.h
http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-lutimens.h
http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-futimens.h
http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-futimens.c
http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-utimensat.c
Taken together, these files produce the executables test-futimens and
test-utimensat which can demonstrate compliance with POSIX. They are also
bundled as part of GNU coreutils (the version bundled with coreutils 8.2
didn't test for ctime compliance, and coreutils 8.3 hasn't been released
yet) if you use 'make -C gnulib-tests check'.
--
Don't work too hard, make some time for fun as well!
Eric Blake [email protected]
On Tue, 22 Dec 2009, Jean-Pierre André wrote:
> With ntfs-3g this is not directly possible, because
> the interface does not provide flags telling which
> timestamps should be updated. The only way would
> be fuse feeding both values (even though unchanged)
> before calling ntfs-3g. This is true for all versions of
> ntfs-3g.
Here's a patch for the high level lib. If the flag_utime_omit_ok flag
is set then the ->utimes() method will be supplied with arguments that
correspond to utimensat (i.e. it can have UTIME_NOW and UTIME_OMIT
values).
Thanks,
Miklos
Index: fuse/configure.in
===================================================================
--- fuse.orig/configure.in 2009-12-22 12:46:44.000000000 +0100
+++ fuse/configure.in 2009-12-22 13:16:07.000000000 +0100
@@ -53,7 +53,7 @@ fi
if test "$enable_mtab" = "no"; then
AC_DEFINE(IGNORE_MTAB, 1, [Don't update /etc/mtab])
fi
-AC_CHECK_FUNCS([fork setxattr fdatasync])
+AC_CHECK_FUNCS([fork setxattr fdatasync utimensat])
AC_CHECK_MEMBERS([struct stat.st_atim])
AC_CHECK_MEMBERS([struct stat.st_atimespec])
Index: fuse/include/fuse.h
===================================================================
--- fuse.orig/include/fuse.h 2009-12-22 12:46:44.000000000 +0100
+++ fuse/include/fuse.h 2009-12-22 13:16:07.000000000 +0100
@@ -458,9 +458,15 @@ struct fuse_operations {
unsigned int flag_nullpath_ok : 1;
/**
+ * Flag indicating that the filesystem accepts special
+ * UTIME_NOW and UTIME_OMIT values in its utimens operation.
+ */
+ unsigned int flag_utime_omit_ok : 1;
+
+ /**
* Reserved flags, don't set
*/
- unsigned int flag_reserved : 31;
+ unsigned int flag_reserved : 30;
/**
* Ioctl
Index: fuse/lib/fuse.c
===================================================================
--- fuse.orig/lib/fuse.c 2009-12-22 12:46:44.000000000 +0100
+++ fuse/lib/fuse.c 2009-12-22 13:16:07.000000000 +0100
@@ -97,6 +97,7 @@ struct fuse {
int intr_installed;
struct fuse_fs *fs;
int nullpath_ok;
+ int utime_omit_ok;
int curr_ticket;
struct lock_queue_element *lockq;
};
@@ -2119,6 +2120,29 @@ static void fuse_lib_setattr(fuse_req_t
err = fuse_fs_truncate(f->fs, path,
attr->st_size);
}
+#ifdef HAVE_UTIMENSAT
+ if (!err && f->utime_omit_ok &&
+ (valid & (FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME))) {
+ struct timespec tv[2];
+
+ tv[0].tv_sec = 0;
+ tv[1].tv_sec = 0;
+ tv[0].tv_nsec = UTIME_OMIT;
+ tv[1].tv_nsec = UTIME_OMIT;
+
+ if (valid & FUSE_SET_ATTR_ATIME_NOW)
+ tv[0].tv_nsec = UTIME_NOW;
+ else if (valid & FUSE_SET_ATTR_ATIME)
+ tv[0] = attr->st_atim;
+
+ if (valid & FUSE_SET_ATTR_MTIME_NOW)
+ tv[1].tv_nsec = UTIME_NOW;
+ else if (valid & FUSE_SET_ATTR_MTIME)
+ tv[1] = attr->st_mtim;
+
+ err = fuse_fs_utimens(f->fs, path, tv);
+ } else
+#endif
if (!err &&
(valid & (FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME)) ==
(FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME)) {
@@ -3637,6 +3661,7 @@ static int fuse_push_module(struct fuse
newfs->m = m;
f->fs = newfs;
f->nullpath_ok = newfs->op.flag_nullpath_ok && f->nullpath_ok;
+ f->utime_omit_ok = newfs->op.flag_utime_omit_ok && f->utime_omit_ok;
return 0;
}
@@ -3687,6 +3712,7 @@ struct fuse *fuse_new_common(struct fuse
fs->compat = compat;
f->fs = fs;
f->nullpath_ok = fs->op.flag_nullpath_ok;
+ f->utime_omit_ok = fs->op.flag_utime_omit_ok;
/* Oh f**k, this is ugly! */
if (!fs->op.lock) {
@@ -3743,8 +3769,10 @@ struct fuse *fuse_new_common(struct fuse
fuse_session_add_chan(f->se, ch);
- if (f->conf.debug)
+ if (f->conf.debug) {
fprintf(stderr, "nullpath_ok: %i\n", f->nullpath_ok);
+ fprintf(stderr, "utime_omit_ok: %i\n", f->utime_omit_ok);
+ }
/* Trace topmost layer by default */
f->fs->debug = f->conf.debug;
Index: fuse/example/fusexmp_fh.c
===================================================================
--- fuse.orig/example/fusexmp_fh.c 2009-06-19 12:24:25.000000000 +0200
+++ fuse/example/fusexmp_fh.c 2009-12-22 13:55:04.000000000 +0100
@@ -283,6 +283,9 @@ static int xmp_ftruncate(const char *pat
static int xmp_utimens(const char *path, const struct timespec ts[2])
{
int res;
+#ifdef HAVE_UTIMENSAT
+ res = utimensat(AT_FDCWD, path, ts, AT_SYMLINK_NOFOLLOW);
+#else
struct timeval tv[2];
tv[0].tv_sec = ts[0].tv_sec;
@@ -291,6 +294,7 @@ static int xmp_utimens(const char *path,
tv[1].tv_usec = ts[1].tv_nsec / 1000;
res = utimes(path, tv);
+#endif
if (res == -1)
return -errno;
@@ -486,6 +490,9 @@ static struct fuse_operations xmp_oper =
.lock = xmp_lock,
.flag_nullpath_ok = 1,
+#if HAVE_UTIMENSAT
+ .flag_utime_omit_ok = 1,
+#endif
};
int main(int argc, char *argv[])
Jean-Pierre Andr? <[email protected]> writes:
> Hi again,
Hi,
>> Well, the problem seems in fuse_lib_setattr() and ntfs_fuse_setattr()
>> (lowlevel op too).
>>
>> The both functions is requiring "ATIME | MTIME". Doesn't it mean the
>> ntfs-3g can't set only MTIME like above utimensat()?
>>
>
> With ntfs-3g this is not directly possible, because
> the interface does not provide flags telling which
> timestamps should be updated. The only way would
> be fuse feeding both values (even though unchanged)
> before calling ntfs-3g. This is true for all versions of
> ntfs-3g.
Yes, with fuse_operations. It is why I'm saying the issue is libfuse or
ntfs-3g.
But I noticed ntfs-3g is including libfuse-lite sources and use it with
static link (I might be wrong here, because I just looked ntfs-3g source
slightly). AFAIK, the fuse of kernel part is passing the flags of some
sort of detail always.
[BTW, the code of that part in kernel may be the following,
fs/fuse/dir.c:iattr_to_fattr(),
if (ivalid & ATTR_ATIME) {
arg->valid |= FATTR_ATIME;
arg->atime = iattr->ia_atime.tv_sec;
arg->atimensec = iattr->ia_atime.tv_nsec;
if (!(ivalid & ATTR_ATIME_SET))
arg->valid |= FATTR_ATIME_NOW;
}
if ((ivalid & ATTR_MTIME) && update_mtime(ivalid)) {
arg->valid |= FATTR_MTIME;
arg->mtime = iattr->ia_mtime.tv_sec;
arg->mtimensec = iattr->ia_mtime.tv_nsec;
if (!(ivalid & ATTR_MTIME_SET))
arg->valid |= FATTR_MTIME_NOW;
}
]
So, if libfuse-lite was fixed to supported that update request, it would
be able to do even if fuse_operations. I.e. in libfuse-lite, emulate
"ATIME | MTIME" request if "MTIME" only (pass unchanged original atime),
then call ->utime() callback. (or adds new utime2 callback with flags, or
something other solutions)
Or, if that request is known limitation of fuse_operations, I think it
would be clear state and ok. The fs needs to use lowlevel op to support
that.
> With lowntfs-3g (release candidate only), this could
> be possible.... but this is not implemented, as the
> case was never found up to now. I can provide you
> with a patch,... if fuse can feed in the flags selectively.
Yes. AFAIK, fuse of kernel part is passing FATTR_MTIME without
FATTR_ATIME to userland (i.e. FUSE_SET_ATTR_ATIME and
FUSE_SET_ATTR_MTIME in libfuse).
I think it's good to implement if it's not design decision of ntfs-3g.
[BTW, just my guess though, it would be good to use "if (vaild &
ATTR_XXX)" style, not "switch()" to support various combinations of
flags]
Thanks.
--
OGAWA Hirofumi <[email protected]>
Hi again,
OGAWA Hirofumi wrote:
> Jean-Pierre Andr?<[email protected]> writes:
>
>
>> Hi again,
>>
> Hi,
>
>
>>> Well, the problem seems in fuse_lib_setattr() and ntfs_fuse_setattr()
>>> (lowlevel op too).
>>>
>>> The both functions is requiring "ATIME | MTIME". Doesn't it mean the
>>> ntfs-3g can't set only MTIME like above utimensat()?
>>>
>>>
>> With ntfs-3g this is not directly possible, because
>> the interface does not provide flags telling which
>> timestamps should be updated. The only way would
>> be fuse feeding both values (even though unchanged)
>> before calling ntfs-3g. This is true for all versions of
>> ntfs-3g.
>>
> Yes, with fuse_operations. It is why I'm saying the issue is libfuse or
> ntfs-3g.
>
> But I noticed ntfs-3g is including libfuse-lite sources and use it with
> static link (I might be wrong here, because I just looked ntfs-3g source
> slightly). AFAIK, the fuse of kernel part is passing the flags of some
> sort of detail always.
>
Use of fuse-lite is optional, and static linking also
(both depend on your configure options),
> [BTW, the code of that part in kernel may be the following,
>
> fs/fuse/dir.c:iattr_to_fattr(),
>
>
[...]
> So, if libfuse-lite was fixed to supported that update request, it would
> be able to do even if fuse_operations. I.e. in libfuse-lite, emulate
> "ATIME | MTIME" request if "MTIME" only (pass unchanged original atime),
> then call ->utime() callback. (or adds new utime2 callback with flags, or
> something other solutions)
>
I will port to fuse-lite the patch which Miklos has
just sent.
> Or, if that request is known limitation of fuse_operations, I think it
> would be clear state and ok. The fs needs to use lowlevel op to support
> that.
>
>
>> With lowntfs-3g (release candidate only), this could
>> be possible.... but this is not implemented, as the
>> case was never found up to now. I can provide you
>> with a patch,... if fuse can feed in the flags selectively.
>>
> Yes. AFAIK, fuse of kernel part is passing FATTR_MTIME without
> FATTR_ATIME to userland (i.e. FUSE_SET_ATTR_ATIME and
> FUSE_SET_ATTR_MTIME in libfuse).
>
> I think it's good to implement if it's not design decision of ntfs-3g.
>
> [BTW, just my guess though, it would be good to use "if (vaild&
> ATTR_XXX)" style, not "switch()" to support various combinations of
> flags]
>
Might be better, ... or not. Setting both mtime
and atime is much simpler than setting each one
individually. So both methods will end up having
to process three different situations.
I suggest I port Miklos patch to fuse-lite soon,
and delay the low-level case (and microsecond
precision) until January. Does that suit your needs ?
Also : there is a common case of mis-configuration
in which gid is set in the mount options and uid
is not. As a consequence files appear as owned
by root and utime() is forbidden to plain users.
So you should first check your mount options.
And since ntfs-2009.11.14 standard ownership
and permissions are fully supported provided a
proper configuration file has been set.
Regards
Jean-Pierre
> Thanks.
>
--
JP Andr?
email [email protected]
This patch which I had in my QA queue for a while should fix all the
timestamp update issues in XFS:
Index: linux-2.6/fs/xfs/xfs_vnodeops.c
===================================================================
--- linux-2.6.orig/fs/xfs/xfs_vnodeops.c 2009-12-21 16:24:28.470990760 +0100
+++ linux-2.6/fs/xfs/xfs_vnodeops.c 2009-12-21 16:32:41.348990760 +0100
@@ -70,7 +70,6 @@ xfs_setattr(
uint commit_flags=0;
uid_t uid=0, iuid=0;
gid_t gid=0, igid=0;
- int timeflags = 0;
struct xfs_dquot *udqp, *gdqp, *olddquot1, *olddquot2;
int need_iolock = 1;
@@ -135,16 +134,13 @@ xfs_setattr(
if (flags & XFS_ATTR_NOLOCK)
need_iolock = 0;
if (!(mask & ATTR_SIZE)) {
- if ((mask != (ATTR_CTIME|ATTR_ATIME|ATTR_MTIME)) ||
- (mp->m_flags & XFS_MOUNT_WSYNC)) {
- tp = xfs_trans_alloc(mp, XFS_TRANS_SETATTR_NOT_SIZE);
- commit_flags = 0;
- if ((code = xfs_trans_reserve(tp, 0,
- XFS_ICHANGE_LOG_RES(mp), 0,
- 0, 0))) {
- lock_flags = 0;
- goto error_return;
- }
+ tp = xfs_trans_alloc(mp, XFS_TRANS_SETATTR_NOT_SIZE);
+ commit_flags = 0;
+ code = xfs_trans_reserve(tp, 0, XFS_ICHANGE_LOG_RES(mp),
+ 0, 0, 0);
+ if (code) {
+ lock_flags = 0;
+ goto error_return;
}
} else {
if (DM_EVENT_ENABLED(ip, DM_EVENT_TRUNCATE) &&
@@ -295,15 +291,23 @@ xfs_setattr(
* or we are explicitly asked to change it. This handles
* the semantic difference between truncate() and ftruncate()
* as implemented in the VFS.
- */
- if (iattr->ia_size != ip->i_size || (mask & ATTR_CTIME))
- timeflags |= XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG;
+ *
+ * The regular truncate() case without ATTR_CTIME and ATTR_MTIME
+ * is a special case where we need to update the times despite
+ * not having these flags set. For all other operations the
+ * VFS set these flags explicitly if it wants a timestamp
+ * update.
+ */
+ if (iattr->ia_size != ip->i_size &&
+ (!(mask & (ATTR_CTIME | ATTR_MTIME)))) {
+ iattr->ia_ctime = iattr->ia_mtime =
+ current_fs_time(inode->i_sb);
+ mask |= ATTR_CTIME | ATTR_MTIME;
+ }
if (iattr->ia_size > ip->i_size) {
ip->i_d.di_size = iattr->ia_size;
ip->i_size = iattr->ia_size;
- if (!(flags & XFS_ATTR_DMI))
- xfs_ichgtime(ip, XFS_ICHGTIME_CHG);
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
} else if (iattr->ia_size <= ip->i_size ||
(iattr->ia_size == 0 && ip->i_d.di_nextents)) {
@@ -374,9 +378,6 @@ xfs_setattr(
ip->i_d.di_gid = gid;
inode->i_gid = gid;
}
-
- xfs_trans_log_inode (tp, ip, XFS_ILOG_CORE);
- timeflags |= XFS_ICHGTIME_CHG;
}
/*
@@ -393,51 +394,37 @@ xfs_setattr(
inode->i_mode &= S_IFMT;
inode->i_mode |= mode & ~S_IFMT;
-
- xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
- timeflags |= XFS_ICHGTIME_CHG;
}
/*
* Change file access or modified times.
*/
- if (mask & (ATTR_ATIME|ATTR_MTIME)) {
- if (mask & ATTR_ATIME) {
- inode->i_atime = iattr->ia_atime;
- ip->i_d.di_atime.t_sec = iattr->ia_atime.tv_sec;
- ip->i_d.di_atime.t_nsec = iattr->ia_atime.tv_nsec;
- ip->i_update_core = 1;
- }
- if (mask & ATTR_MTIME) {
- inode->i_mtime = iattr->ia_mtime;
- ip->i_d.di_mtime.t_sec = iattr->ia_mtime.tv_sec;
- ip->i_d.di_mtime.t_nsec = iattr->ia_mtime.tv_nsec;
- timeflags &= ~XFS_ICHGTIME_MOD;
- timeflags |= XFS_ICHGTIME_CHG;
- }
- if (tp && (mask & (ATTR_MTIME_SET|ATTR_ATIME_SET)))
- xfs_trans_log_inode (tp, ip, XFS_ILOG_CORE);
+ if (mask & ATTR_ATIME) {
+ inode->i_atime = iattr->ia_atime;
+ ip->i_d.di_atime.t_sec = iattr->ia_atime.tv_sec;
+ ip->i_d.di_atime.t_nsec = iattr->ia_atime.tv_nsec;
+ ip->i_update_core = 1;
}
-
- /*
- * Change file inode change time only if ATTR_CTIME set
- * AND we have been called by a DMI function.
- */
-
- if ((flags & XFS_ATTR_DMI) && (mask & ATTR_CTIME)) {
+ if (mask & ATTR_CTIME) {
inode->i_ctime = iattr->ia_ctime;
ip->i_d.di_ctime.t_sec = iattr->ia_ctime.tv_sec;
ip->i_d.di_ctime.t_nsec = iattr->ia_ctime.tv_nsec;
ip->i_update_core = 1;
- timeflags &= ~XFS_ICHGTIME_CHG;
+ }
+ if (mask & ATTR_MTIME) {
+ inode->i_mtime = iattr->ia_mtime;
+ ip->i_d.di_mtime.t_sec = iattr->ia_mtime.tv_sec;
+ ip->i_d.di_mtime.t_nsec = iattr->ia_mtime.tv_nsec;
+ ip->i_update_core = 1;
}
/*
- * Send out timestamp changes that need to be set to the
- * current time. Not done when called by a DMI function.
+ * And finally, log the inode core if any attribute in it
+ * has been changed.
*/
- if (timeflags && !(flags & XFS_ATTR_DMI))
- xfs_ichgtime(ip, timeflags);
+ if (mask & (ATTR_UID|ATTR_GID|ATTR_MODE|
+ ATTR_ATIME|ATTR_CTIME|ATTR_MTIME))
+ xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
XFS_STATS_INC(xs_ig_attrchg);
@@ -452,12 +439,10 @@ xfs_setattr(
* mix so this probably isn't worth the trouble to optimize.
*/
code = 0;
- if (tp) {
- if (mp->m_flags & XFS_MOUNT_WSYNC)
- xfs_trans_set_sync(tp);
+ if (mp->m_flags & XFS_MOUNT_WSYNC)
+ xfs_trans_set_sync(tp);
- code = xfs_trans_commit(tp, commit_flags);
- }
+ code = xfs_trans_commit(tp, commit_flags);
xfs_iunlock(ip, lock_flags);
Index: linux-2.6/fs/xfs/linux-2.6/xfs_acl.c
===================================================================
--- linux-2.6.orig/fs/xfs/linux-2.6/xfs_acl.c 2009-12-21 16:31:51.838990759 +0100
+++ linux-2.6/fs/xfs/linux-2.6/xfs_acl.c 2009-12-21 16:32:17.471990759 +0100
@@ -251,8 +251,9 @@ xfs_set_mode(struct inode *inode, mode_t
if (mode != inode->i_mode) {
struct iattr iattr;
- iattr.ia_valid = ATTR_MODE;
+ iattr.ia_valid = ATTR_MODE | ATTR_CTIME;
iattr.ia_mode = mode;
+ iattr.ia_ctime = current_fs_time(inode->i_sb);
error = -xfs_setattr(XFS_I(inode), &iattr, XFS_ATTR_NOACL);
}
Jean-Pierre Andr? <[email protected]> writes:
> Hi again,
Hi,
>> Yes. AFAIK, fuse of kernel part is passing FATTR_MTIME without
>> FATTR_ATIME to userland (i.e. FUSE_SET_ATTR_ATIME and
>> FUSE_SET_ATTR_MTIME in libfuse).
>>
>> I think it's good to implement if it's not design decision of ntfs-3g.
>>
>> [BTW, just my guess though, it would be good to use "if (vaild&
>> ATTR_XXX)" style, not "switch()" to support various combinations of
>> flags]
>>
>
> Might be better, ... or not. Setting both mtime
> and atime is much simpler than setting each one
> individually. So both methods will end up having
> to process three different situations.
Well, I don't care the implementation detail. However, the combination
might not be only three. E.g. if fs was exported as network fs's
backend, so many combinations are possible. So, specified known
combination can be fragile.
(e.g. ATTR_MTIME | ATTR_SIZE, etc, etc.)
> I suggest I port Miklos patch to fuse-lite soon,
> and delay the low-level case (and microsecond
> precision) until January. Does that suit your needs ?
Thanks. Sounds good. I'm not using ntfs-3g actually, I just bridged the
bug report on lkml to others. Eric?
Thanks.
--
OGAWA Hirofumi <[email protected]>
Christoph Hellwig <[email protected]> writes:
> This patch which I had in my QA queue for a while should fix all the
> timestamp update issues in XFS:
Thanks for fixing this.
> Index: linux-2.6/fs/xfs/xfs_vnodeops.c
> ===================================================================
> --- linux-2.6.orig/fs/xfs/xfs_vnodeops.c 2009-12-21 16:24:28.470990760 +0100
> +++ linux-2.6/fs/xfs/xfs_vnodeops.c 2009-12-21 16:32:41.348990760 +0100
> @@ -70,7 +70,6 @@ xfs_setattr(
> uint commit_flags=0;
> uid_t uid=0, iuid=0;
> gid_t gid=0, igid=0;
> - int timeflags = 0;
> struct xfs_dquot *udqp, *gdqp, *olddquot1, *olddquot2;
> int need_iolock = 1;
>
> @@ -135,16 +134,13 @@ xfs_setattr(
> if (flags & XFS_ATTR_NOLOCK)
> need_iolock = 0;
> if (!(mask & ATTR_SIZE)) {
> - if ((mask != (ATTR_CTIME|ATTR_ATIME|ATTR_MTIME)) ||
> - (mp->m_flags & XFS_MOUNT_WSYNC)) {
> - tp = xfs_trans_alloc(mp, XFS_TRANS_SETATTR_NOT_SIZE);
> - commit_flags = 0;
> - if ((code = xfs_trans_reserve(tp, 0,
> - XFS_ICHANGE_LOG_RES(mp), 0,
> - 0, 0))) {
> - lock_flags = 0;
> - goto error_return;
> - }
> + tp = xfs_trans_alloc(mp, XFS_TRANS_SETATTR_NOT_SIZE);
> + commit_flags = 0;
> + code = xfs_trans_reserve(tp, 0, XFS_ICHANGE_LOG_RES(mp),
> + 0, 0, 0);
> + if (code) {
> + lock_flags = 0;
> + goto error_return;
> }
> } else {
> if (DM_EVENT_ENABLED(ip, DM_EVENT_TRUNCATE) &&
> @@ -295,15 +291,23 @@ xfs_setattr(
> * or we are explicitly asked to change it. This handles
> * the semantic difference between truncate() and ftruncate()
> * as implemented in the VFS.
> - */
> - if (iattr->ia_size != ip->i_size || (mask & ATTR_CTIME))
> - timeflags |= XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG;
> + *
> + * The regular truncate() case without ATTR_CTIME and ATTR_MTIME
> + * is a special case where we need to update the times despite
> + * not having these flags set. For all other operations the
> + * VFS set these flags explicitly if it wants a timestamp
> + * update.
> + */
> + if (iattr->ia_size != ip->i_size &&
> + (!(mask & (ATTR_CTIME | ATTR_MTIME)))) {
> + iattr->ia_ctime = iattr->ia_mtime =
> + current_fs_time(inode->i_sb);
> + mask |= ATTR_CTIME | ATTR_MTIME;
> + }
>
> if (iattr->ia_size > ip->i_size) {
> ip->i_d.di_size = iattr->ia_size;
> ip->i_size = iattr->ia_size;
> - if (!(flags & XFS_ATTR_DMI))
> - xfs_ichgtime(ip, XFS_ICHGTIME_CHG);
> xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> } else if (iattr->ia_size <= ip->i_size ||
> (iattr->ia_size == 0 && ip->i_d.di_nextents)) {
> @@ -374,9 +378,6 @@ xfs_setattr(
> ip->i_d.di_gid = gid;
> inode->i_gid = gid;
> }
> -
> - xfs_trans_log_inode (tp, ip, XFS_ILOG_CORE);
> - timeflags |= XFS_ICHGTIME_CHG;
> }
>
> /*
> @@ -393,51 +394,37 @@ xfs_setattr(
>
> inode->i_mode &= S_IFMT;
> inode->i_mode |= mode & ~S_IFMT;
> -
> - xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> - timeflags |= XFS_ICHGTIME_CHG;
> }
>
> /*
> * Change file access or modified times.
> */
> - if (mask & (ATTR_ATIME|ATTR_MTIME)) {
> - if (mask & ATTR_ATIME) {
> - inode->i_atime = iattr->ia_atime;
> - ip->i_d.di_atime.t_sec = iattr->ia_atime.tv_sec;
> - ip->i_d.di_atime.t_nsec = iattr->ia_atime.tv_nsec;
> - ip->i_update_core = 1;
> - }
> - if (mask & ATTR_MTIME) {
> - inode->i_mtime = iattr->ia_mtime;
> - ip->i_d.di_mtime.t_sec = iattr->ia_mtime.tv_sec;
> - ip->i_d.di_mtime.t_nsec = iattr->ia_mtime.tv_nsec;
> - timeflags &= ~XFS_ICHGTIME_MOD;
> - timeflags |= XFS_ICHGTIME_CHG;
> - }
> - if (tp && (mask & (ATTR_MTIME_SET|ATTR_ATIME_SET)))
> - xfs_trans_log_inode (tp, ip, XFS_ILOG_CORE);
> + if (mask & ATTR_ATIME) {
> + inode->i_atime = iattr->ia_atime;
> + ip->i_d.di_atime.t_sec = iattr->ia_atime.tv_sec;
> + ip->i_d.di_atime.t_nsec = iattr->ia_atime.tv_nsec;
> + ip->i_update_core = 1;
> }
> -
> - /*
> - * Change file inode change time only if ATTR_CTIME set
> - * AND we have been called by a DMI function.
> - */
> -
> - if ((flags & XFS_ATTR_DMI) && (mask & ATTR_CTIME)) {
> + if (mask & ATTR_CTIME) {
> inode->i_ctime = iattr->ia_ctime;
> ip->i_d.di_ctime.t_sec = iattr->ia_ctime.tv_sec;
> ip->i_d.di_ctime.t_nsec = iattr->ia_ctime.tv_nsec;
> ip->i_update_core = 1;
> - timeflags &= ~XFS_ICHGTIME_CHG;
> + }
> + if (mask & ATTR_MTIME) {
> + inode->i_mtime = iattr->ia_mtime;
> + ip->i_d.di_mtime.t_sec = iattr->ia_mtime.tv_sec;
> + ip->i_d.di_mtime.t_nsec = iattr->ia_mtime.tv_nsec;
> + ip->i_update_core = 1;
> }
>
> /*
> - * Send out timestamp changes that need to be set to the
> - * current time. Not done when called by a DMI function.
> + * And finally, log the inode core if any attribute in it
> + * has been changed.
> */
> - if (timeflags && !(flags & XFS_ATTR_DMI))
> - xfs_ichgtime(ip, timeflags);
> + if (mask & (ATTR_UID|ATTR_GID|ATTR_MODE|
> + ATTR_ATIME|ATTR_CTIME|ATTR_MTIME))
> + xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
>
> XFS_STATS_INC(xs_ig_attrchg);
>
> @@ -452,12 +439,10 @@ xfs_setattr(
> * mix so this probably isn't worth the trouble to optimize.
> */
> code = 0;
> - if (tp) {
> - if (mp->m_flags & XFS_MOUNT_WSYNC)
> - xfs_trans_set_sync(tp);
> + if (mp->m_flags & XFS_MOUNT_WSYNC)
> + xfs_trans_set_sync(tp);
>
> - code = xfs_trans_commit(tp, commit_flags);
> - }
> + code = xfs_trans_commit(tp, commit_flags);
>
> xfs_iunlock(ip, lock_flags);
>
> Index: linux-2.6/fs/xfs/linux-2.6/xfs_acl.c
> ===================================================================
> --- linux-2.6.orig/fs/xfs/linux-2.6/xfs_acl.c 2009-12-21 16:31:51.838990759 +0100
> +++ linux-2.6/fs/xfs/linux-2.6/xfs_acl.c 2009-12-21 16:32:17.471990759 +0100
> @@ -251,8 +251,9 @@ xfs_set_mode(struct inode *inode, mode_t
> if (mode != inode->i_mode) {
> struct iattr iattr;
>
> - iattr.ia_valid = ATTR_MODE;
> + iattr.ia_valid = ATTR_MODE | ATTR_CTIME;
> iattr.ia_mode = mode;
> + iattr.ia_ctime = current_fs_time(inode->i_sb);
>
> error = -xfs_setattr(XFS_I(inode), &iattr, XFS_ATTR_NOACL);
> }
--
OGAWA Hirofumi <[email protected]>
On Tue, Dec 22, 2009 at 05:42:49AM -0700, Eric Blake wrote:
> According to Dave Chinner on 12/22/2009 5:34 AM:
> > Yeah, it looks like the change to utimesat() back in 2.6.26 for
> > posix conformance made ATTR_CTIME appear outside inode truncation
> > and XFS wasn't updated for this change in behaviour at the VFS level.
> > Looks simple to fix, but I'm worried about introducing other
> > unintended ctime modifications - is there a test suite that checks
> > posix compliant atime/mtime/ctime behaviour around anywhere?
>
> Yes - the gnulib unit test, consisting of:
>
> http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/nap.h
> http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-utimens-common.h
> http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-utimens.h
> http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-lutimens.h
> http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-futimens.h
> http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-futimens.c
> http://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-utimensat.c
>
> Taken together, these files produce the executables test-futimens and
> test-utimensat which can demonstrate compliance with POSIX. They are also
> bundled as part of GNU coreutils (the version bundled with coreutils 8.2
> didn't test for ctime compliance, and coreutils 8.3 hasn't been released
> yet) if you use 'make -C gnulib-tests check'.
Ok, I'll see if I can add the last GPLv2 licensed version into xfstests.
Hi,
OGAWA Hirofumi wrote:
>> I suggest I port Miklos patch to fuse-lite soon,
>> and delay the low-level case (and microsecond
>> precision) until January. Does that suit your needs ?
>>
> Thanks. Sounds good. I'm not using ntfs-3g actually, I just bridged the
> bug report on lkml to others. Eric?
Attached is a proposed patch for selective update
of timestamps in ntfs-3g (based on the release
candidate version ntfs-3g-2009.11.14AC.2).
It includes the backport of Miklos's patch posted
yesterday to fuse-lite. As a matter of fact fuse-lite
is needed, because I do not know how to make this
compile with unpatched fuse.
Note : this does not yet include sub-second precision.
Regards
Jean-Pierre
Jean-Pierre Andr? <[email protected]> writes:
> Hi,
Hi,
> OGAWA Hirofumi wrote:
>>> I suggest I port Miklos patch to fuse-lite soon,
>>> and delay the low-level case (and microsecond
>>> precision) until January. Does that suit your needs ?
>>>
>> Thanks. Sounds good. I'm not using ntfs-3g actually, I just bridged the
>> bug report on lkml to others. Eric?
>
> Attached is a proposed patch for selective update
> of timestamps in ntfs-3g (based on the release
> candidate version ntfs-3g-2009.11.14AC.2).
>
> It includes the backport of Miklos's patch posted
> yesterday to fuse-lite. As a matter of fact fuse-lite
> is needed, because I do not know how to make this
> compile with unpatched fuse.
Thanks for fixing this.
--
OGAWA Hirofumi <[email protected]>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
According to OGAWA Hirofumi on 12/22/2009 10:58 AM:
>> I suggest I port Miklos patch to fuse-lite soon,
>> and delay the low-level case (and microsecond
>> precision) until January. Does that suit your needs ?
>
> Thanks. Sounds good. I'm not using ntfs-3g actually, I just bridged the
> bug report on lkml to others. Eric?
I'm also bridging the report from a coreutils user (now cc'd). Since I
also don't use ntfs-3g, I'm hoping that ctrn3e8 will be able to help test
whether the latest patch to ntfs-3g makes a difference in properly setting
times. To me, delaying precision while fixing UTIME_OMIT semantics is a
reasonable approach.
By the way, is there any reliable way, other than uname() and checking for
a minimum kernel version, to tell if all file systems will properly
support UTIME_OMIT? For coreutils 8.3, we will be inserting a workaround
where instead of using UTIME_OMIT, we call fstatat() in advance of
utimensat() and pass the original timestamp down. But it would be nice to
avoid the penalty of the extra stat if there were a reliable way to ensure
that, regardless of file system, the use of UTIME_OMIT will be honored.
After all, coreutils wants touch(1) to work regardless of how old the
user's kernel and file system drivers are.
- --
Don't work too hard, make some time for fun as well!
Eric Blake [email protected]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAksyEu0ACgkQ84KuGfSFAYCrzACgirIjqmS7vFOBcI8xau6jHEa0
4L0AnAjJkje+tSMF/FZkTbkohg/fhQ+i
=ngx0
-----END PGP SIGNATURE-----
Hi,
OGAWA Hirofumi wrote:
>> I suggest I port Miklos patch to fuse-lite soon,
>> and delay the low-level case (and microsecond
>> precision) until January. Does that suit your needs ?
>>
> Thanks. Sounds good. I'm not using ntfs-3g actually, I just bridged the
> bug report on lkml to others. Eric?
As my Christmas present to any real non-bridging
user, here is, ahead of schedule, the patch for the
low level fuse interface of ntfs-3g.
The good news is that for this interface, no update
of fuse is needed. So there is no compilation issue with
unpatched versions of fuse, and the attached patch
is independent of the one for the high-level interface
posted previously, except for the configuration.ac file
for which I duplicate the patch.
Regards
Jean-Pierre
Eric Blake <[email protected]> writes:
> By the way, is there any reliable way, other than uname() and checking for
> a minimum kernel version, to tell if all file systems will properly
> support UTIME_OMIT?
Um... sorry, I don't know. And it might be hard to detect efficiently if
the workaround is enough efficient like one fstat() syscall (Pass fd to
kernel. I.e. just read from cached inode).
> For coreutils 8.3, we will be inserting a workaround where instead of
> using UTIME_OMIT, we call fstatat() in advance of utimensat() and pass
> the original timestamp down. But it would be nice to avoid the
> penalty of the extra stat if there were a reliable way to ensure that,
> regardless of file system, the use of UTIME_OMIT will be honored.
> After all, coreutils wants touch(1) to work regardless of how old the
> user's kernel and file system drivers are.
Or it would depend on coreutils policy though, personally I think it's
ok that it ignores the bug as known fs bug, otherwise coreutils would
need to collect workarounds on several filesystems of several OSes.
Thanks.
--
OGAWA Hirofumi <[email protected]>