From fedc1d96a8d8251237916fdf230e981bc396aa02 Mon Sep 17 00:00:00 2001 From: LOLi Date: Mon, 13 Nov 2017 18:24:26 +0100 Subject: [PATCH] Fix truncate(2) mtime and ctime handling On Linux, ftruncate(2) always changes the file timestamps, even if the file size is not changed. However, in case of a successfull truncate(2), the timestamps are updated only if the file size changes. This translates to the VFS calling the ZFS Posix Layer "setattr" function (zpl_setattr) with ATTR_MTIME and ATTR_CTIME unconditionally set on the iattr mask only when doing a ftruncate(2), while the truncate(2) is left to the filesystem implementation to be dealt with. This behaviour is consistent with POSIX:2004/SUSv3 specifications where there's no explicit requirement for file size changes to update the timestamps only for ftruncate(2): http://pubs.opengroup.org/onlinepubs/009695399/functions/truncate.html http://pubs.opengroup.org/onlinepubs/009695399/functions/ftruncate.html This has been later updated in POSIX:2008/SUSv4 where, for both truncate(2)/ftruncate(2), there's no mention of this size change requirement: http://austingroupbugs.net/view.php?id=489 http://pubs.opengroup.org/onlinepubs/9699919799/functions/truncate.html http://pubs.opengroup.org/onlinepubs/9699919799/functions/ftruncate.html Unfortunately the Linux VFS is still calling into the ZPL without ATTR_MTIME/ATTR_CTIME set in the truncate(2) case: we fix this by explicitly updating the timestamps when detecting the ATTR_SIZE bit, which is always set in do_truncate(), on the iattr mask. Reviewed-by: Brian Behlendorf Signed-off-by: loli10K Closes #6811 Closes #6819 --- module/zfs/zfs_vnops.c | 4 +- tests/runfiles/linux.run | 3 +- tests/zfs-tests/include/math.shlib | 30 +++++ .../tests/functional/truncate/.gitignore | 1 + .../tests/functional/truncate/Makefile.am | 11 +- .../tests/functional/truncate/truncate_test.c | 103 ++++++++++++++++++ .../truncate/truncate_timestamps.ksh | 74 +++++++++++++ 7 files changed, 222 insertions(+), 4 deletions(-) create mode 100644 tests/zfs-tests/tests/functional/truncate/.gitignore create mode 100644 tests/zfs-tests/tests/functional/truncate/truncate_test.c create mode 100755 tests/zfs-tests/tests/functional/truncate/truncate_timestamps.ksh diff --git a/module/zfs/zfs_vnops.c b/module/zfs/zfs_vnops.c index a88f2482e7..6a1dab5c98 100644 --- a/module/zfs/zfs_vnops.c +++ b/module/zfs/zfs_vnops.c @@ -3155,7 +3155,7 @@ top: &atime, sizeof (atime)); } - if (mask & ATTR_MTIME) { + if (mask & (ATTR_MTIME | ATTR_SIZE)) { ZFS_TIME_ENCODE(&vap->va_mtime, mtime); ZTOI(zp)->i_mtime = timespec_trunc(vap->va_mtime, ZTOI(zp)->i_sb->s_time_gran); @@ -3164,7 +3164,7 @@ top: mtime, sizeof (mtime)); } - if (mask & ATTR_CTIME) { + if (mask & (ATTR_CTIME | ATTR_SIZE)) { ZFS_TIME_ENCODE(&vap->va_ctime, ctime); ZTOI(zp)->i_ctime = timespec_trunc(vap->va_ctime, ZTOI(zp)->i_sb->s_time_gran); diff --git a/tests/runfiles/linux.run b/tests/runfiles/linux.run index 5583a25541..7d6d13b4b2 100644 --- a/tests/runfiles/linux.run +++ b/tests/runfiles/linux.run @@ -537,7 +537,8 @@ tests = ['threadsappend_001_pos'] tests = ['tmpfile_001_pos', 'tmpfile_002_pos', 'tmpfile_003_pos'] [tests/functional/truncate] -tests = ['truncate_001_pos', 'truncate_002_pos'] +tests = ['truncate_001_pos', 'truncate_002_pos', 'truncate_timestamps'] +tags = ['functional', 'truncate'] [tests/functional/upgrade] tests = [ 'upgrade_userobj_001_pos' ] diff --git a/tests/zfs-tests/include/math.shlib b/tests/zfs-tests/include/math.shlib index 4ed11fb4b7..66658cdda9 100644 --- a/tests/zfs-tests/include/math.shlib +++ b/tests/zfs-tests/include/math.shlib @@ -66,3 +66,33 @@ function to_bytes return 0 } + +# +# Verify $a is equal to $b, otherwise raise an error specifying +# the $type of values being compared +# +function verify_eq # +{ + typeset a=$1 + typeset b=$2 + typeset type=$3 + + if [[ $a -ne $b ]]; then + log_fail "Compared $type should be equal: $a != $b" + fi +} + +# +# Verify $a is not equal to $b, otherwise raise an error specifying +# the $type of values being compared +# +function verify_ne # +{ + typeset a=$1 + typeset b=$2 + typeset type=$3 + + if [[ $a -eq $b ]]; then + log_fail "Compared $type should be not equal: $a == $b" + fi +} diff --git a/tests/zfs-tests/tests/functional/truncate/.gitignore b/tests/zfs-tests/tests/functional/truncate/.gitignore new file mode 100644 index 0000000000..f28d93573c --- /dev/null +++ b/tests/zfs-tests/tests/functional/truncate/.gitignore @@ -0,0 +1 @@ +/truncate_test diff --git a/tests/zfs-tests/tests/functional/truncate/Makefile.am b/tests/zfs-tests/tests/functional/truncate/Makefile.am index 16cadf29d7..0071e8f870 100644 --- a/tests/zfs-tests/tests/functional/truncate/Makefile.am +++ b/tests/zfs-tests/tests/functional/truncate/Makefile.am @@ -1,7 +1,16 @@ +include $(top_srcdir)/config/Rules.am + pkgdatadir = $(datadir)/@PACKAGE@/zfs-tests/tests/functional/truncate + dist_pkgdata_SCRIPTS = \ setup.ksh \ cleanup.ksh \ truncate.cfg \ truncate_001_pos.ksh \ - truncate_002_pos.ksh + truncate_002_pos.ksh \ + truncate_timestamps.ksh + +pkgexecdir = $(datadir)/@PACKAGE@/zfs-tests/tests/functional/truncate + +pkgexec_PROGRAMS = truncate_test +truncate_test_SOURCES = truncate_test.c diff --git a/tests/zfs-tests/tests/functional/truncate/truncate_test.c b/tests/zfs-tests/tests/functional/truncate/truncate_test.c new file mode 100644 index 0000000000..3e277e8657 --- /dev/null +++ b/tests/zfs-tests/tests/functional/truncate/truncate_test.c @@ -0,0 +1,103 @@ +/* + * This file and its contents are supplied under the terms of the + * Common Development and Distribution License ("CDDL"), version 1.0. + * You may only use this file in accordance with the terms of version + * 1.0 of the CDDL. + * + * A full copy of the text of the CDDL should have accompanied this + * source. A copy of the CDDL is also available via the Internet at + * http://www.illumos.org/license/CDDL. + */ + +/* + * Copyright (c) 2012, 2014 by Delphix. All rights reserved. + * Copyright 2017, loli10K . All rights reserved. + */ + +#include +#include +#include +#include +#include +#include +#include + +#define FSIZE 256*1024*1024 + +static long fsize = FSIZE; +static int errflag = 0; +static char *filename = NULL; +static int ftruncflag = 0; + +static void parse_options(int argc, char *argv[]); + +static void +usage(char *execname) +{ + (void) fprintf(stderr, + "usage: %s [-s filesize] [-f] /path/to/file\n", execname); + (void) exit(1); +} + +int +main(int argc, char *argv[]) +{ + int fd; + + parse_options(argc, argv); + + if (ftruncflag) { + fd = open(filename, O_RDWR|O_CREAT, 0666); + if (fd < 0) { + perror("open"); + return (1); + } + if (ftruncate(fd, fsize) < 0) { + perror("ftruncate"); + return (1); + } + if (close(fd)) { + perror("close"); + return (1); + } + } else { + if (truncate(filename, fsize) < 0) { + perror("truncate"); + return (1); + } + } + return (0); +} + +static void +parse_options(int argc, char *argv[]) +{ + int c; + extern char *optarg; + extern int optind, optopt; + + while ((c = getopt(argc, argv, "s:f")) != -1) { + switch (c) { + case 's': + fsize = atoi(optarg); + break; + case 'f': + ftruncflag++; + break; + case ':': + (void) fprintf(stderr, + "Option -%c requires an operand\n", optopt); + errflag++; + break; + } + if (errflag) { + (void) usage(argv[0]); + } + } + + if (argc <= optind) { + (void) fprintf(stderr, "No filename specified\n"); + usage(argv[0]); + } + filename = argv[optind]; +} diff --git a/tests/zfs-tests/tests/functional/truncate/truncate_timestamps.ksh b/tests/zfs-tests/tests/functional/truncate/truncate_timestamps.ksh new file mode 100755 index 0000000000..c365c7415e --- /dev/null +++ b/tests/zfs-tests/tests/functional/truncate/truncate_timestamps.ksh @@ -0,0 +1,74 @@ +#!/bin/ksh -p +# +# This file and its contents are supplied under the terms of the +# Common Development and Distribution License ("CDDL"), version 1.0. +# You may only use this file in accordance with the terms of version +# 1.0 of the CDDL. +# +# A full copy of the text of the CDDL should have accompanied this +# source. A copy of the CDDL is also available via the Internet at +# http://www.illumos.org/license/CDDL. +# + +# +# Copyright 2017, loli10K . All rights reserved. +# + +. $STF_SUITE/tests/functional/truncate/truncate.cfg +. $STF_SUITE/include/libtest.shlib +. $STF_SUITE/include/math.shlib + +# +# DESCRIPTION: +# Ensure both truncate(2)/ftruncate(2) update target file mtime/ctime attributes +# +# STRATEGY: +# 1. Create a file +# 2. Truncate the file +# 3. Verify both mtime/ctime are updated +# 4. Rinse and repeat for both truncate(2) and ftruncate(2) with various sizes +# + +verify_runnable "both" + +function verify_truncate #