All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/10] Add tmpfs filesystem support
@ 2013-12-10 20:11 Junho Ryu
  2013-12-10 20:11 ` [PATCH 01/10] xfstests: Add tmpfs support Junho Ryu
                   ` (9 more replies)
  0 siblings, 10 replies; 36+ messages in thread
From: Junho Ryu @ 2013-12-10 20:11 UTC (permalink / raw)
  To: xfs; +Cc: tytso, hughd, branto, Junho Ryu

Hi all,

Boris Ranto has sent a series of 3 patches adding tmpfs support.
http://oss.sgi.com/archives/xfs/2013-05/msg00069.html

This one only contains the first patch from Boris Ranto's series followed by
more patches in order to either run or not run all generic tests instead of
failing.

This one does not contains other 2 patches in Boris Ranto's series just because
rest these patehes do not depend on those, but it will be nice if those are
also reviewed as well.

Boris Ranto (1):
  xfstests: Add tmpfs support

Hugh Dickins (3):
  xfstests: use mount point instead of device name
  xfstests: _scratch_mkfs_sized() for tmpfs
  xfstests: add executable permission to tests

Junho Ryu (6):
  xfstests: increase tmpfs memory size
  xfstests: do not unmount tmpfs during remount.
  xfstests: fix generic/225 to check fiemap support
  xfstests: fix generic/127 to call _cleanup() only once
  xfstests: check O_DIRECT support before testing direct I/O
  xfstests: skip parts of tests which cannot work on tmpfs

 check                 |  2 ++
 common/rc             | 78 +++++++++++++++++++++++++++++++++++++++++++++++++--
 tests/generic/053     |  6 ++--
 tests/generic/076     |  2 ++
 tests/generic/091     |  1 +
 tests/generic/127     |  1 -
 tests/generic/130     |  1 +
 tests/generic/133     |  1 +
 tests/generic/135     | 18 +++---------
 tests/generic/169     | 20 ++++---------
 tests/generic/169.out |  6 ++--
 tests/generic/192     |  3 +-
 tests/generic/225     |  1 +
 tests/generic/226     |  4 +--
 tests/generic/256     |  2 +-
 tests/generic/258     |  3 +-
 tests/generic/263     |  1 +
 tests/generic/273     |  2 +-
 tests/generic/277     |  2 ++
 tests/generic/299     |  1 +
 tests/generic/300     |  1 +
 tests/generic/306     |  3 +-
 tests/generic/311     |  0
 tests/generic/314     |  0
 tests/generic/315     |  0
 tests/generic/321     |  0
 tests/generic/322     |  0
 27 files changed, 110 insertions(+), 49 deletions(-)
 mode change 100644 => 100755 tests/generic/311
 mode change 100644 => 100755 tests/generic/314
 mode change 100644 => 100755 tests/generic/315
 mode change 100644 => 100755 tests/generic/321
 mode change 100644 => 100755 tests/generic/322

-- 
1.8.5.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 01/10] xfstests: Add tmpfs support
  2013-12-10 20:11 [PATCH 00/10] Add tmpfs filesystem support Junho Ryu
@ 2013-12-10 20:11 ` Junho Ryu
  2013-12-11  7:40   ` Christoph Hellwig
  2013-12-17 16:40   ` Rich Johnston
  2013-12-10 20:11 ` [PATCH 02/10] xfstests: use mount point instead of device name Junho Ryu
                   ` (8 subsequent siblings)
  9 siblings, 2 replies; 36+ messages in thread
From: Junho Ryu @ 2013-12-10 20:11 UTC (permalink / raw)
  To: xfs; +Cc: tytso, hughd, branto, Junho Ryu

From: Boris Ranto <branto@redhat.com>

This is just a simple patch to get the tmpfs working as a target file
system. The patch copies the way nfs is handled in xfstests.

I didn't change the xfstests logic to recognize a proper SCRATCH_DEV.
Hence, the SCRATCH_DEV for tmpfs should be in nfs form (with ':' sign
in it) in order for this to work properly.

Signed-off-by: Boris Ranto <branto@redhat.com>
Signed-off-by: Junho Ryu <jayr@google.com>
---
 check     |  2 ++
 common/rc | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/check b/check
index c88bcf2..320ad26 100755
--- a/check
+++ b/check
@@ -73,6 +73,7 @@ usage()
 
 check options
     -nfs                test NFS
+    -tmpfs              test TMPFS
     -l			line mode diff
     -udiff		show unified diff (default)
     -n			show me, do not run tests
@@ -207,6 +208,7 @@ while [ $# -gt 0 ]; do
 	-\? | -h | --help) usage ;;
 
 	-nfs)	FSTYP=nfs ;;
+	-tmpfs)	FSTYP=tmpfs ;;
 
 	-g)	group=$2 ; shift ;
 		GROUP_LIST="$GROUP_LIST $group"
diff --git a/common/rc b/common/rc
index 5f61d57..a2005c9 100644
--- a/common/rc
+++ b/common/rc
@@ -112,6 +112,10 @@ _mount_opts()
 	# acls aren't turned on by default on gfs2
 	export MOUNT_OPTIONS="-o acl $GFS2_MOUNT_OPTIONS"
 	;;
+    tmpfs)
+	# We need to specify the size at mount, use 512 MB by default
+	export MOUNT_OPTIONS="-o size=512M $TMPFS_MOUNT_OPTIONS"
+	;;
     *)
 	;;
     esac
@@ -140,6 +144,9 @@ _mkfs_opts()
     jfs)
 	export MKFS_OPTIONS="$JFS_MKFS_OPTIONS -q"
 	;;
+    tmpfs)
+	export MKFS_OPTIONS="$TMPFS_MKFS_OPTIONS"
+	;;
     *)
 	;;
     esac
@@ -544,6 +551,9 @@ _scratch_mkfs()
     ext4)
 	_scratch_mkfs_ext4 $*
 	;;
+    tmpfs)
+	# do nothing for tmpfs
+	;;
     *)
 	yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* $SCRATCH_DEV
 	;;
@@ -977,6 +987,12 @@ _require_scratch()
 	nfs*)
                  _notrun "requires a scratch device"
 		 ;;
+	tmpfs)
+		if [ -z "$SCRATCH_DEV" -o ! -d "$SCRATCH_MNT" ];
+		then
+		    _notrun "this test requires a valid \$SCRATCH_MNT and unique $SCRATCH_DEV"
+		fi
+		;;
 	*)
 		 if [ -z "$SCRATCH_DEV" -o "`_is_block_dev $SCRATCH_DEV`" = "" ]
 		 then
@@ -1701,6 +1717,9 @@ _check_test_fs()
     btrfs)
 	_check_btrfs_filesystem $TEST_DEV
 	;;
+    tmpfs)
+	# no way to check consistency for tmpfs
+	;;
     *)
 	_check_generic_filesystem $TEST_DEV
 	;;
@@ -1733,6 +1752,9 @@ _check_scratch_fs()
     btrfs)
 	_check_btrfs_filesystem $device
 	;;
+    tmpfs)
+	# no way to check consistency for tmpfs
+	;;
     *)
 	_check_generic_filesystem $device
 	;;
-- 
1.8.5.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 02/10] xfstests: use mount point instead of device name
  2013-12-10 20:11 [PATCH 00/10] Add tmpfs filesystem support Junho Ryu
  2013-12-10 20:11 ` [PATCH 01/10] xfstests: Add tmpfs support Junho Ryu
@ 2013-12-10 20:11 ` Junho Ryu
  2013-12-11  7:42   ` Christoph Hellwig
  2013-12-10 20:11 ` [PATCH 03/10] xfstests: _scratch_mkfs_sized() for tmpfs Junho Ryu
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 36+ messages in thread
From: Junho Ryu @ 2013-12-10 20:11 UTC (permalink / raw)
  To: xfs; +Cc: tytso, hughd, branto, Junho Ryu

From: Hugh Dickins <hughd@google.com>

A tmpfs mount does not involve any block device, its $SCRATCH_DEV is
nothing but a place-holder, so apply 'df' or 'stat' to its mount point
$SCRATCH_MNT instead of to $SCRATCH_DEV.

It also fixes a bug in generic/256: "stat -f $SCRATCH_DEV" returning the
block size of the root file system, but not the test file system.

$ df | grep sda
/dev/sda1    233191    62959    157791    29%    /boot

$ sudo dumpe2fs -h /dev/sda1 | grep "Block size"
Block size:               1024

$ stat -f /dev/sda1 | grep "Block size"
Block size: 4096       Fundamental block size: 4096

$ stat -f /boot | grep "Block size"
Block size: 1024       Fundamental block size: 1024

Signed-off-by: Hugh Dickins <hughd@google.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Junho Ryu <jayr@google.com>
---
 tests/generic/256 | 2 +-
 tests/generic/273 | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/generic/256 b/tests/generic/256
index cfe7237..c6c8f2f 100755
--- a/tests/generic/256
+++ b/tests/generic/256
@@ -170,7 +170,7 @@ _scratch_mount
 # Test must be able to write files with non-root permissions
 chmod 777 $SCRATCH_MNT
 
-block_size=`stat -f $SCRATCH_DEV | grep "Block size" | cut -d " " -f3`
+block_size=`stat -f $SCRATCH_MNT | grep "Block size" | cut -d " " -f3`
 _test_full_fs_punch $(( $block_size * 2 )) $block_size 500 $SCRATCH_MNT/252.$$ $block_size
 
 status=0 ; exit
diff --git a/tests/generic/273 b/tests/generic/273
index 8e121a2..dc87a92 100755
--- a/tests/generic/273
+++ b/tests/generic/273
@@ -68,7 +68,7 @@ _file_create()
 
 	cd $SCRATCH_MNT/origin
 
-	_disksize=`$DF_PROG --block-size=1 $SCRATCH_DEV | tail -1 | awk '{ print $5 }'`
+	_disksize=`$DF_PROG --block-size=1 $SCRATCH_MNT | tail -1 | awk '{ print $5 }'`
 	_disksize=$(($_disksize / 3))
 	_num=$(($_disksize / $count / $threads / 4096))
 	_count=$count
-- 
1.8.5.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 03/10] xfstests: _scratch_mkfs_sized() for tmpfs
  2013-12-10 20:11 [PATCH 00/10] Add tmpfs filesystem support Junho Ryu
  2013-12-10 20:11 ` [PATCH 01/10] xfstests: Add tmpfs support Junho Ryu
  2013-12-10 20:11 ` [PATCH 02/10] xfstests: use mount point instead of device name Junho Ryu
@ 2013-12-10 20:11 ` Junho Ryu
  2013-12-11  7:44   ` Christoph Hellwig
  2013-12-10 20:11 ` [PATCH 04/10] xfstests: increase tmpfs memory size Junho Ryu
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 36+ messages in thread
From: Junho Ryu @ 2013-12-10 20:11 UTC (permalink / raw)
  To: xfs; +Cc: tytso, hughd, branto, Junho Ryu

From: Hugh Dickins <hughd@google.com>

_scratch_mkfs_sized() avoid blockdev and update MOUNT_OPTIONS with
required size on tmpfs, so those tests using it can now run.

Signed-off-by: Hugh Dickins <hughd@google.com>
Signed-off-by: Junho Ryu <jayr@google.com>
---
 common/rc | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/common/rc b/common/rc
index a2005c9..83d56a1 100644
--- a/common/rc
+++ b/common/rc
@@ -590,7 +590,7 @@ _scratch_mkfs_sized()
 
     blocks=`expr $fssize / $blocksize`
 
-    if [ "$HOSTOS" == "Linux" ]; then
+    if [ "$HOSTOS" == "Linux" -a "$FSTYP" != "tmpfs" ]; then
 	devsize=`blockdev --getsize64 $SCRATCH_DEV`
 	[ "$fssize" -gt "$devsize" ] && _notrun "Scratch device too small"
     fi
@@ -611,6 +611,9 @@ _scratch_mkfs_sized()
     btrfs)
 	$MKFS_BTRFS_PROG $MKFS_OPTIONS -b $fssize $SCRATCH_DEV
 	;;
+    tmpfs)
+	export MOUNT_OPTIONS="-o size=$fssize $TMPFS_MOUNT_OPTIONS"
+	;;
     *)
 	_notrun "Filesystem $FSTYP not supported in _scratch_mkfs_sized"
 	;;
-- 
1.8.5.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 04/10] xfstests: increase tmpfs memory size
  2013-12-10 20:11 [PATCH 00/10] Add tmpfs filesystem support Junho Ryu
                   ` (2 preceding siblings ...)
  2013-12-10 20:11 ` [PATCH 03/10] xfstests: _scratch_mkfs_sized() for tmpfs Junho Ryu
@ 2013-12-10 20:11 ` Junho Ryu
  2013-12-11  7:44   ` Christoph Hellwig
  2013-12-10 20:11 ` [PATCH 05/10] xfstests: do not unmount tmpfs during remount Junho Ryu
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 36+ messages in thread
From: Junho Ryu @ 2013-12-10 20:11 UTC (permalink / raw)
  To: xfs; +Cc: tytso, hughd, branto, Junho Ryu

512M is not enough for generic/129, generic/213 and generic/247.  Raise
default tmpfs size to 1G.

Signed-off-by: Hugh Dickins <hughd@google.com>
Signed-off-by: Junho Ryu <jayr@google.com>
---
 common/rc | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/common/rc b/common/rc
index 83d56a1..57f7ccd 100644
--- a/common/rc
+++ b/common/rc
@@ -113,8 +113,8 @@ _mount_opts()
 	export MOUNT_OPTIONS="-o acl $GFS2_MOUNT_OPTIONS"
 	;;
     tmpfs)
-	# We need to specify the size at mount, use 512 MB by default
-	export MOUNT_OPTIONS="-o size=512M $TMPFS_MOUNT_OPTIONS"
+	# We need to specify the size at mount
+	export MOUNT_OPTIONS="-o size=${DEFAULT_TMPFS_SIZE_MB}M $TMPFS_MOUNT_OPTIONS"
 	;;
     *)
 	;;
@@ -168,6 +168,8 @@ _fsck_opts()
 }
 
 [ -z "$FSTYP" ] && FSTYP=xfs
+# Default tmpfs size in megabytes.
+[ -z "$DEFAULT_TMPFS_SIZE_MB" ] && DEFAULT_TMPFS_SIZE_MB=1024
 [ -z "$MOUNT_OPTIONS" ] && _mount_opts
 [ -z "$MKFS_OPTIONS" ] && _mkfs_opts
 [ -z "$FSCK_OPTIONS" ] && _fsck_opts
-- 
1.8.5.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 05/10] xfstests: do not unmount tmpfs during remount.
  2013-12-10 20:11 [PATCH 00/10] Add tmpfs filesystem support Junho Ryu
                   ` (3 preceding siblings ...)
  2013-12-10 20:11 ` [PATCH 04/10] xfstests: increase tmpfs memory size Junho Ryu
@ 2013-12-10 20:11 ` Junho Ryu
  2013-12-11  7:46   ` Christoph Hellwig
  2013-12-10 20:11 ` [PATCH 06/10] xfstests: fix generic/225 to check fiemap support Junho Ryu
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 36+ messages in thread
From: Junho Ryu @ 2013-12-10 20:11 UTC (permalink / raw)
  To: xfs; +Cc: tytso, hughd, branto, Junho Ryu

Several tests unmount then re-mount the scratch filesystem, to check
that the content is unchanged; but unmounting a tmpfs is designed to
lose its content, which causes such tests to fail unnecessarily.  Add
_scratch_remount(), which instead uses remount on a tmpfs to preserve
its contents.  Also add _test_remount() to do the same for the test
device.

Signed-off-by: Hugh Dickins <hughd@google.com>
Signed-off-by: Junho Ryu <jayr@google.com>
---
 common/rc             | 34 ++++++++++++++++++++++++++++++++--
 tests/generic/135     | 17 +++--------------
 tests/generic/169     | 20 ++++++--------------
 tests/generic/169.out |  6 ++----
 tests/generic/192     |  3 +--
 tests/generic/226     |  3 +--
 tests/generic/258     |  3 +--
 tests/generic/306     |  3 +--
 8 files changed, 47 insertions(+), 42 deletions(-)

diff --git a/common/rc b/common/rc
index 57f7ccd..4d46008 100644
--- a/common/rc
+++ b/common/rc
@@ -302,8 +302,20 @@ _scratch_unmount()
 
 _scratch_remount()
 {
-    _scratch_unmount
-    _scratch_mount
+    CWD=`pwd`
+    cd /
+
+    case $FSTYP in
+    tmpfs)
+        mount -o remount "$@" $SCRATCH_MNT
+        ;;
+    *)
+        _scratch_unmount
+        _scratch_mount "$@"
+        ;;
+    esac
+
+    cd $CWD
 }
 
 _test_mount()
@@ -312,6 +324,24 @@ _test_mount()
     _mount -t $FSTYP $TEST_OPTIONS $TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS $* $TEST_DEV $TEST_DIR
 }
 
+_test_remount()
+{
+    CWD=`pwd`
+    cd /
+
+    case $FSTYP in
+    tmpfs)
+        mount -o remount "$@" $TEST_DIR
+        ;;
+    *)
+        umount $TEST_DIR
+        _test_mount "$@"
+        ;;
+    esac
+
+    cd $CWD
+}
+
 _scratch_mkfs_options()
 {
     _scratch_options mkfs
diff --git a/tests/generic/135 b/tests/generic/135
index e3b827a..4a8b690 100755
--- a/tests/generic/135
+++ b/tests/generic/135
@@ -40,19 +40,8 @@ _supported_fs generic
 _supported_os Linux IRIX
 
 _require_scratch
-_scratch_mkfs >/dev/null 2>&1
-
-_umount_mount()
-{
-    CWD=`pwd`
-    cd /
-    # pipe error into /dev/null, in case not mounted (after _require_scratch)
-    umount $SCRATCH_MNT 2>/dev/null
-    _scratch_mount
-    cd "$CWD"
-}
-
-_umount_mount
+_scratch_mkfs >/dev/null 2>&1 || _fail "mkfs failed"
+_scratch_mount > /dev/null 2>&1 || _fail "mount failed"
 
 cd $SCRATCH_MNT
 
@@ -70,7 +59,7 @@ $XFS_IO_PROG -f -c 'pwrite -b 4k -S 0x78 0 4k' trunc_file > /dev/null
 $XFS_IO_PROG -f -c 'truncate 2k' trunc_file > /dev/null
 $XFS_IO_PROG -c 'pwrite 1k 0 1k' trunc_file > /dev/null
 
-_umount_mount
+_scratch_remount
 
 # check file size and contents
 od -Ad -x async_file
diff --git a/tests/generic/169 b/tests/generic/169
index ebeffa7..ebfb106 100755
--- a/tests/generic/169
+++ b/tests/generic/169
@@ -73,13 +73,9 @@ $XFS_IO_PROG -a -c "pwrite 0 5k" -c "fsync" \
 	$SCRATCH_MNT/testfile \
 	| _show_wrote_and_stat_only
 
-echo "# unmounting scratch"
-umount $SCRATCH_MNT>>$seqres.full 2>&1 \
-    || _fail "unmount failed"
-
-echo "# mounting scratch"
-_scratch_mount >>$seqres.full 2>&1 \
-    || _fail "mount failed: $MOUNT_OPTIONS"
+echo "# remounting scratch"
+_scratch_remount >>$seqres.full 2>&1 \
+    || _fail "remount failed: $MOUNT_OPTIONS"
 
 echo "# stating file to confirm correct size"
 $XFS_IO_PROG -r -c "stat" $SCRATCH_MNT/testfile \
@@ -90,13 +86,9 @@ $XFS_IO_PROG -f -c "pwrite 0 5" -c s -c "pwrite 5 5" \
 	-c "stat" $SCRATCH_MNT/nextfile \
 	| _show_wrote_and_stat_only
 
-echo "# unmounting scratch"
-umount $SCRATCH_MNT>>$seqres.full 2>&1 \
-    || _fail "unmount failed"
-
-echo "# mounting scratch"
-_scratch_mount >>$seqres.full 2>&1 \
-    || _fail "mount failed: $MOUNT_OPTIONS"
+echo "# remounting scratch"
+_scratch_remount >>$seqres.full 2>&1 \
+    || _fail "remount failed: $MOUNT_OPTIONS"
 
 echo "# stating file to confirm correct size"
 $XFS_IO_PROG -r -c "stat" $SCRATCH_MNT/nextfile \
diff --git a/tests/generic/169.out b/tests/generic/169.out
index 22a5b77..5f7df39 100644
--- a/tests/generic/169.out
+++ b/tests/generic/169.out
@@ -5,15 +5,13 @@ wrote 5120/5120 bytes at offset 0
 wrote 5120/5120 bytes at offset 5120
 wrote 5120/5120 bytes at offset 10240
 stat.size = 15360
-# unmounting scratch
-# mounting scratch
+# remounting scratch
 # stating file to confirm correct size
 stat.size = 15360
 # appending 10 bytes to new file, sync at 5 bytes
 wrote 5/5 bytes at offset 0
 wrote 5/5 bytes at offset 5
 stat.size = 10
-# unmounting scratch
-# mounting scratch
+# remounting scratch
 # stating file to confirm correct size
 stat.size = 10
diff --git a/tests/generic/192 b/tests/generic/192
index 2b4e41c..ec069b0 100755
--- a/tests/generic/192
+++ b/tests/generic/192
@@ -76,8 +76,7 @@ cat $testfile
 time2=`_access_time $testfile | tee -a $seqres.full`
 
 cd /
-umount $TEST_DIR
-_test_mount
+_test_remount
 time3=`_access_time $testfile | tee -a $seqres.full`
 
 delta1=`expr $time2 - $time1`
diff --git a/tests/generic/226 b/tests/generic/226
index b98e889..3514965 100755
--- a/tests/generic/226
+++ b/tests/generic/226
@@ -60,8 +60,7 @@ for I in `seq 1 $loops`; do
 done
 
 echo
-umount $SCRATCH_DEV
-_scratch_mount
+_scratch_remount
 
 echo "--> $loops direct 64m writes in a loop"
 for I in `seq 1 $loops`; do
diff --git a/tests/generic/258 b/tests/generic/258
index 3aeb5ce..567a39a 100755
--- a/tests/generic/258
+++ b/tests/generic/258
@@ -61,8 +61,7 @@ fi
 
 # unmount, remount, and check the timestamp
 echo "Remounting to flush cache"
-umount $TEST_DEV
-_test_mount
+_test_remount
 
 # Should yield -315593940 (prior to epoch)
 echo "Testing for negative seconds since epoch"
diff --git a/tests/generic/306 b/tests/generic/306
index 04d28df..56313f2 100755
--- a/tests/generic/306
+++ b/tests/generic/306
@@ -66,8 +66,7 @@ touch $BINDFILE || _fail "Could not create bind mount file"
 touch $TARGET || _fail "Could not create symlink target"
 ln -s $TARGET $SYMLINK
 
-_scratch_unmount || _fail "Could not unmount scratch device"
-_scratch_mount -o ro || _fail "Could not mount scratch readonly"
+_scratch_remount -o ro || _fail "Could not remount scratch readonly"
 
 # We should be able to read & write to/from these devices even on an RO fs
 echo "== try to create new file"
-- 
1.8.5.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 06/10] xfstests: fix generic/225 to check fiemap support
  2013-12-10 20:11 [PATCH 00/10] Add tmpfs filesystem support Junho Ryu
                   ` (4 preceding siblings ...)
  2013-12-10 20:11 ` [PATCH 05/10] xfstests: do not unmount tmpfs during remount Junho Ryu
@ 2013-12-10 20:11 ` Junho Ryu
  2013-12-11  7:46   ` Christoph Hellwig
  2013-12-10 20:11 ` [PATCH 07/10] xfstests: fix generic/127 to call _cleanup() only once Junho Ryu
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 36+ messages in thread
From: Junho Ryu @ 2013-12-10 20:11 UTC (permalink / raw)
  To: xfs; +Cc: tytso, hughd, branto, Junho Ryu

generic/225 runs fiemap.  It would be nice to call
_require_xfs_io_fiemap like other tests which use fiemap.

Signed-off-by: Junho Ryu <jayr@google.com>
---
 tests/generic/225 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/generic/225 b/tests/generic/225
index 73b125a..af65506 100755
--- a/tests/generic/225
+++ b/tests/generic/225
@@ -44,6 +44,7 @@ _cleanup()
 _supported_fs generic
 _supported_os Linux
 _require_scratch
+_require_xfs_io_fiemap
 
 _scratch_mkfs > /dev/null 2>&1
 _scratch_mount > /dev/null 2>&1
-- 
1.8.5.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 07/10] xfstests: fix generic/127 to call _cleanup() only once
  2013-12-10 20:11 [PATCH 00/10] Add tmpfs filesystem support Junho Ryu
                   ` (5 preceding siblings ...)
  2013-12-10 20:11 ` [PATCH 06/10] xfstests: fix generic/225 to check fiemap support Junho Ryu
@ 2013-12-10 20:11 ` Junho Ryu
  2013-12-11  7:47   ` Christoph Hellwig
  2013-12-10 20:11 ` [PATCH 08/10] xfstests: check O_DIRECT support before testing direct I/O Junho Ryu
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 36+ messages in thread
From: Junho Ryu @ 2013-12-10 20:11 UTC (permalink / raw)
  To: xfs; +Cc: tytso, hughd, branto, Junho Ryu

The test is already trap-ing signal 0 (EXIT). umount prints error
message when target tmpfs is already unmounted.

Signed-off-by: Junho Ryu <jayr@google.com>
---
 tests/generic/127 | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/generic/127 b/tests/generic/127
index 58ca91e..47bd1ca 100755
--- a/tests/generic/127
+++ b/tests/generic/127
@@ -117,5 +117,4 @@ _fsx_std_nommap
 _fsx_std_mmap
 
 status=0
-_cleanup
 exit
-- 
1.8.5.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 08/10] xfstests: check O_DIRECT support before testing direct I/O
  2013-12-10 20:11 [PATCH 00/10] Add tmpfs filesystem support Junho Ryu
                   ` (6 preceding siblings ...)
  2013-12-10 20:11 ` [PATCH 07/10] xfstests: fix generic/127 to call _cleanup() only once Junho Ryu
@ 2013-12-10 20:11 ` Junho Ryu
  2013-12-11  7:47   ` Christoph Hellwig
  2013-12-10 20:12 ` [PATCH 09/10] xfstests: add executable permission to tests Junho Ryu
  2013-12-10 20:12 ` [PATCH 10/10] xfstests: skip parts of tests which cannot work on tmpfs Junho Ryu
  9 siblings, 1 reply; 36+ messages in thread
From: Junho Ryu @ 2013-12-10 20:11 UTC (permalink / raw)
  To: xfs; +Cc: tytso, hughd, branto, Junho Ryu

Some filesystems do not support O_DIRECT.  Check whether TEST_DIR supports it
by running xfs_io with and without -d flag.

Signed-off-by: Junho Ryu <jayr@google.com>
---
 common/rc         | 15 +++++++++++++++
 tests/generic/091 |  1 +
 tests/generic/130 |  1 +
 tests/generic/133 |  1 +
 tests/generic/135 |  1 +
 tests/generic/226 |  1 +
 tests/generic/263 |  1 +
 tests/generic/299 |  1 +
 tests/generic/300 |  1 +
 9 files changed, 23 insertions(+)

diff --git a/common/rc b/common/rc
index 4d46008..02c47ea 100644
--- a/common/rc
+++ b/common/rc
@@ -1185,6 +1185,7 @@ _require_aiodio()
         AIO_TEST=src/aio-dio-regress/$1
         [ -x $AIO_TEST ] || _notrun "$AIO_TEST not built"
     fi
+    _require_xfs_io_direct
 }
 
 # run an aio-dio program
@@ -1323,6 +1324,20 @@ _require_xfs_io_fiemap()
 		_notrun "xfs_io fiemap command failed (no fs support?)"
 }
 
+# check that xfs_io, kernel and filesystem support direct I/O
+_require_xfs_io_direct()
+{
+	testfile=$TEST_DIR/$$.direct
+	$XFS_IO_PROG -F -f -c "pwrite 0 20k" $testfile 2>&1
+	if [ $? -eq 0 ]; then
+		$XFS_IO_PROG -F -f -d -c "pwrite 0 20k" $testfile 2>&1
+		if [ $? -ne 0 ]; then
+			_notrun "O_DIRECT is not supported"
+		fi
+	fi
+	rm -f $testfile 2>&1 > /dev/null
+}
+
 # Check that a fs has enough free space (in 1024b blocks)
 #
 _require_fs_space()
diff --git a/tests/generic/091 b/tests/generic/091
index bb176f1..926042d 100755
--- a/tests/generic/091
+++ b/tests/generic/091
@@ -38,6 +38,7 @@ trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
 # real QA test starts here
 _supported_fs generic
 _supported_os Linux
+_require_xfs_io_direct
 
 rm -f $seqres.full
 
diff --git a/tests/generic/130 b/tests/generic/130
index 8367dc7..f9c885f 100755
--- a/tests/generic/130
+++ b/tests/generic/130
@@ -50,6 +50,7 @@ _supported_os Linux IRIX
 
 _require_scratch
 _require_sparse_files
+_require_xfs_io_direct
 
 _scratch_mkfs >/dev/null 2>&1
 _scratch_mount
diff --git a/tests/generic/133 b/tests/generic/133
index 7fb1695..d858756 100755
--- a/tests/generic/133
+++ b/tests/generic/133
@@ -38,6 +38,7 @@ trap "exit \$status" 0 1 2 3 15
 # real QA test starts here
 _supported_fs generic
 _supported_os Linux IRIX
+_require_xfs_io_direct
 
 echo "Buffered writer, buffered reader"
 $XFS_IO_PROG -f -d -c 'pwrite -b 64k 0 512m' $TEST_DIR/io_test > /dev/null
diff --git a/tests/generic/135 b/tests/generic/135
index 4a8b690..04ceebf 100755
--- a/tests/generic/135
+++ b/tests/generic/135
@@ -39,6 +39,7 @@ trap "exit \$status" 0 1 2 3 15
 _supported_fs generic
 _supported_os Linux IRIX
 
+_require_xfs_io_direct
 _require_scratch
 _scratch_mkfs >/dev/null 2>&1 || _fail "mkfs failed"
 _scratch_mount > /dev/null 2>&1 || _fail "mount failed"
diff --git a/tests/generic/226 b/tests/generic/226
index 3514965..0fbaea7 100755
--- a/tests/generic/226
+++ b/tests/generic/226
@@ -37,6 +37,7 @@ trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
 _supported_fs generic
 _supported_os Linux IRIX
 _require_scratch
+_require_xfs_io_direct
 
 # real QA test starts here
 rm -f $seqres.full
diff --git a/tests/generic/263 b/tests/generic/263
index bc59865..7bfe3ed 100755
--- a/tests/generic/263
+++ b/tests/generic/263
@@ -38,6 +38,7 @@ trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
 # real QA test starts here
 _supported_fs generic
 _supported_os Linux
+_require_xfs_io_direct
 
 rm -f $seqres.full
 
diff --git a/tests/generic/299 b/tests/generic/299
index 14cce96..f2a83ee 100755
--- a/tests/generic/299
+++ b/tests/generic/299
@@ -43,6 +43,7 @@ _supported_fs generic
 _supported_os Linux
 _need_to_be_root
 _require_scratch
+_require_xfs_io_direct
 
 NUM_JOBS=$((4*LOAD_FACTOR))
 BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
diff --git a/tests/generic/300 b/tests/generic/300
index 7c60728..006c214 100755
--- a/tests/generic/300
+++ b/tests/generic/300
@@ -43,6 +43,7 @@ _supported_fs generic
 _supported_os Linux
 _need_to_be_root
 _require_scratch
+_require_xfs_io_direct
 
 NUM_JOBS=$((4*LOAD_FACTOR))
 BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
-- 
1.8.5.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 09/10] xfstests: add executable permission to tests
  2013-12-10 20:11 [PATCH 00/10] Add tmpfs filesystem support Junho Ryu
                   ` (7 preceding siblings ...)
  2013-12-10 20:11 ` [PATCH 08/10] xfstests: check O_DIRECT support before testing direct I/O Junho Ryu
@ 2013-12-10 20:12 ` Junho Ryu
  2013-12-11  7:48   ` Christoph Hellwig
  2013-12-10 20:12 ` [PATCH 10/10] xfstests: skip parts of tests which cannot work on tmpfs Junho Ryu
  9 siblings, 1 reply; 36+ messages in thread
From: Junho Ryu @ 2013-12-10 20:12 UTC (permalink / raw)
  To: xfs; +Cc: tytso, hughd, branto, Junho Ryu

From: Hugh Dickins <hughd@google.com>

add executable permission to generic/311, generic/314 and generic/315.
Just like other tests in the same directory.

Signed-off-by: Hugh Dickins <hughd@google.com>
Signed-off-by: Junho Ryu <jayr@google.com>
---
 tests/generic/311 | 0
 tests/generic/314 | 0
 tests/generic/315 | 0
 tests/generic/321 | 0
 tests/generic/322 | 0
 5 files changed, 0 insertions(+), 0 deletions(-)
 mode change 100644 => 100755 tests/generic/311
 mode change 100644 => 100755 tests/generic/314
 mode change 100644 => 100755 tests/generic/315
 mode change 100644 => 100755 tests/generic/321
 mode change 100644 => 100755 tests/generic/322

diff --git a/tests/generic/311 b/tests/generic/311
old mode 100644
new mode 100755
diff --git a/tests/generic/314 b/tests/generic/314
old mode 100644
new mode 100755
diff --git a/tests/generic/315 b/tests/generic/315
old mode 100644
new mode 100755
diff --git a/tests/generic/321 b/tests/generic/321
old mode 100644
new mode 100755
diff --git a/tests/generic/322 b/tests/generic/322
old mode 100644
new mode 100755
-- 
1.8.5.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 10/10] xfstests: skip parts of tests which cannot work on tmpfs
  2013-12-10 20:11 [PATCH 00/10] Add tmpfs filesystem support Junho Ryu
                   ` (8 preceding siblings ...)
  2013-12-10 20:12 ` [PATCH 09/10] xfstests: add executable permission to tests Junho Ryu
@ 2013-12-10 20:12 ` Junho Ryu
  2013-12-11  7:51   ` Christoph Hellwig
  9 siblings, 1 reply; 36+ messages in thread
From: Junho Ryu @ 2013-12-10 20:12 UTC (permalink / raw)
  To: xfs; +Cc: tytso, hughd, branto, Junho Ryu

Skip generic/076 which tests concurrent access while testing tmpfs
because there are other tests cover the case using fsstress.

Skip generic/277 which requires chattr.

Even if generic/053 will not be able to test exactly what the test has
been intended to test, it will be still useful to have this test to try
out some functionality on tmpfs rather than always failing or being
skipped.

Signed-off-by: Hugh Dickins <hughd@google.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Junho Ryu <jayr@google.com>
---
 tests/generic/053 | 6 +++---
 tests/generic/076 | 2 ++
 tests/generic/277 | 2 ++
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/tests/generic/053 b/tests/generic/053
index b8bd9e6..885b87c 100755
--- a/tests/generic/053
+++ b/tests/generic/053
@@ -81,9 +81,9 @@ list_acls()
 
 echo "acls before repair:"
 list_acls
-_do 'unmount $SCRATCH_DEV' 'umount $SCRATCH_DEV'
-_do 'repair filesystem' '_check_scratch_fs'
-_do 'mount filesytem' '_scratch_mount'
+_do 'unmount $SCRATCH_DEV' '[ $FSTYP = tmpfs ] || umount $SCRATCH_DEV'
+_do 'repair filesystem' '[ $FSTYP = tmpfs ] || _check_scratch_fs'
+_do 'mount filesytem' '[ $FSTYP = tmpfs ] || _scratch_mount'
 echo "acls after repair: "
 list_acls
 
diff --git a/tests/generic/076 b/tests/generic/076
index a53840f..faeed2f 100755
--- a/tests/generic/076
+++ b/tests/generic/076
@@ -57,6 +57,8 @@ _supported_os IRIX Linux
 
 _require_scratch
 
+[ "$FSTYP" = "tmpfs" ] && _notrun "not applicable for the tmpfs file system"
+
 echo "*** init fs"
 
 rm -f $seqres.full
diff --git a/tests/generic/277 b/tests/generic/277
index 8461ad9..4672343 100755
--- a/tests/generic/277
+++ b/tests/generic/277
@@ -44,6 +44,8 @@ _supported_fs generic
 _supported_os Linux
 _require_scratch
 
+[ "$FSTYP" = "tmpfs" ] && _notrun "not applicable for the tmpfs file system"
+
 _scratch_mkfs > /dev/null 2>&1
 _scratch_mount
 
-- 
1.8.5.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH 01/10] xfstests: Add tmpfs support
  2013-12-10 20:11 ` [PATCH 01/10] xfstests: Add tmpfs support Junho Ryu
@ 2013-12-11  7:40   ` Christoph Hellwig
  2013-12-17 16:40   ` Rich Johnston
  1 sibling, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2013-12-11  7:40 UTC (permalink / raw)
  To: Junho Ryu; +Cc: hughd, tytso, branto, xfs

On Tue, Dec 10, 2013 at 12:11:52PM -0800, Junho Ryu wrote:
> From: Boris Ranto <branto@redhat.com>
> 
> This is just a simple patch to get the tmpfs working as a target file
> system. The patch copies the way nfs is handled in xfstests.
> 
> I didn't change the xfstests logic to recognize a proper SCRATCH_DEV.
> Hence, the SCRATCH_DEV for tmpfs should be in nfs form (with ':' sign
> in it) in order for this to work properly.
> 
> Signed-off-by: Boris Ranto <branto@redhat.com>
> Signed-off-by: Junho Ryu <jayr@google.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 02/10] xfstests: use mount point instead of device name
  2013-12-10 20:11 ` [PATCH 02/10] xfstests: use mount point instead of device name Junho Ryu
@ 2013-12-11  7:42   ` Christoph Hellwig
  0 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2013-12-11  7:42 UTC (permalink / raw)
  To: Junho Ryu; +Cc: hughd, tytso, branto, xfs

On Tue, Dec 10, 2013 at 12:11:53PM -0800, Junho Ryu wrote:
> From: Hugh Dickins <hughd@google.com>
> 
> A tmpfs mount does not involve any block device, its $SCRATCH_DEV is
> nothing but a place-holder, so apply 'df' or 'stat' to its mount point
> $SCRATCH_MNT instead of to $SCRATCH_DEV.
> 
> It also fixes a bug in generic/256: "stat -f $SCRATCH_DEV" returning the
> block size of the root file system, but not the test file system.
> 
> $ df | grep sda
> /dev/sda1    233191    62959    157791    29%    /boot
> 
> $ sudo dumpe2fs -h /dev/sda1 | grep "Block size"
> Block size:               1024
> 
> $ stat -f /dev/sda1 | grep "Block size"
> Block size: 4096       Fundamental block size: 4096
> 
> $ stat -f /boot | grep "Block size"
> Block size: 1024       Fundamental block size: 1024
> 
> Signed-off-by: Hugh Dickins <hughd@google.com>
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> Signed-off-by: Junho Ryu <jayr@google.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>


Although I have to wonder how you got these tests to run, as it seems
like you didn't implement _scratch_mkfs_sized for tmpfs.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 03/10] xfstests: _scratch_mkfs_sized() for tmpfs
  2013-12-10 20:11 ` [PATCH 03/10] xfstests: _scratch_mkfs_sized() for tmpfs Junho Ryu
@ 2013-12-11  7:44   ` Christoph Hellwig
  0 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2013-12-11  7:44 UTC (permalink / raw)
  To: Junho Ryu; +Cc: hughd, tytso, branto, xfs

On Tue, Dec 10, 2013 at 12:11:54PM -0800, Junho Ryu wrote:
> From: Hugh Dickins <hughd@google.com>
> 
> _scratch_mkfs_sized() avoid blockdev and update MOUNT_OPTIONS with
> required size on tmpfs, so those tests using it can now run.
> 
> Signed-off-by: Hugh Dickins <hughd@google.com>
> Signed-off-by: Junho Ryu <jayr@google.com>

Ah, here we go.

>      blocks=`expr $fssize / $blocksize`
>  
> -    if [ "$HOSTOS" == "Linux" ]; then
> +    if [ "$HOSTOS" == "Linux" -a "$FSTYP" != "tmpfs" ]; then

Should just check if SCRATCH_DEV is a block devices instead of
hardcoding a filesystem.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 04/10] xfstests: increase tmpfs memory size
  2013-12-10 20:11 ` [PATCH 04/10] xfstests: increase tmpfs memory size Junho Ryu
@ 2013-12-11  7:44   ` Christoph Hellwig
  0 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2013-12-11  7:44 UTC (permalink / raw)
  To: Junho Ryu; +Cc: hughd, tytso, branto, xfs

On Tue, Dec 10, 2013 at 12:11:55PM -0800, Junho Ryu wrote:
> 512M is not enough for generic/129, generic/213 and generic/247.  Raise
> default tmpfs size to 1G.
> 
> Signed-off-by: Hugh Dickins <hughd@google.com>
> Signed-off-by: Junho Ryu <jayr@google.com>
> ---
>  common/rc | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Please fold it into patch 1.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 05/10] xfstests: do not unmount tmpfs during remount.
  2013-12-10 20:11 ` [PATCH 05/10] xfstests: do not unmount tmpfs during remount Junho Ryu
@ 2013-12-11  7:46   ` Christoph Hellwig
  2013-12-11 22:40     ` Dave Chinner
  0 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2013-12-11  7:46 UTC (permalink / raw)
  To: Junho Ryu; +Cc: hughd, tytso, branto, xfs

On Tue, Dec 10, 2013 at 12:11:56PM -0800, Junho Ryu wrote:
> Several tests unmount then re-mount the scratch filesystem, to check
> that the content is unchanged; but unmounting a tmpfs is designed to
> lose its content, which causes such tests to fail unnecessarily.  Add
> _scratch_remount(), which instead uses remount on a tmpfs to preserve
> its contents.  Also add _test_remount() to do the same for the test
> device.

All these tests don't make sense if you never unmount the filesystem.
Istead they should exit with _notrun for tmpfs.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 06/10] xfstests: fix generic/225 to check fiemap support
  2013-12-10 20:11 ` [PATCH 06/10] xfstests: fix generic/225 to check fiemap support Junho Ryu
@ 2013-12-11  7:46   ` Christoph Hellwig
  2013-12-11 22:42     ` Dave Chinner
  0 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2013-12-11  7:46 UTC (permalink / raw)
  To: Junho Ryu; +Cc: hughd, tytso, branto, xfs

On Tue, Dec 10, 2013 at 12:11:57PM -0800, Junho Ryu wrote:
> generic/225 runs fiemap.  It would be nice to call
> _require_xfs_io_fiemap like other tests which use fiemap.

It's not exactly correct as it doesn't require xfs_io.  If you have some
spare cycles it would be good to factor out a _require_fiemap from it.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 07/10] xfstests: fix generic/127 to call _cleanup() only once
  2013-12-10 20:11 ` [PATCH 07/10] xfstests: fix generic/127 to call _cleanup() only once Junho Ryu
@ 2013-12-11  7:47   ` Christoph Hellwig
  0 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2013-12-11  7:47 UTC (permalink / raw)
  To: Junho Ryu; +Cc: hughd, tytso, branto, xfs

On Tue, Dec 10, 2013 at 12:11:58PM -0800, Junho Ryu wrote:
> The test is already trap-ing signal 0 (EXIT). umount prints error
> message when target tmpfs is already unmounted.
> 
> Signed-off-by: Junho Ryu <jayr@google.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 08/10] xfstests: check O_DIRECT support before testing direct I/O
  2013-12-10 20:11 ` [PATCH 08/10] xfstests: check O_DIRECT support before testing direct I/O Junho Ryu
@ 2013-12-11  7:47   ` Christoph Hellwig
  0 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2013-12-11  7:47 UTC (permalink / raw)
  To: Junho Ryu; +Cc: hughd, tytso, branto, xfs

On Tue, Dec 10, 2013 at 12:11:59PM -0800, Junho Ryu wrote:
> Some filesystems do not support O_DIRECT.  Check whether TEST_DIR supports it
> by running xfs_io with and without -d flag.

should be _require_odirect as we don't really care about xfs_io features
but kernel/filesystem features.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 09/10] xfstests: add executable permission to tests
  2013-12-10 20:12 ` [PATCH 09/10] xfstests: add executable permission to tests Junho Ryu
@ 2013-12-11  7:48   ` Christoph Hellwig
  0 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2013-12-11  7:48 UTC (permalink / raw)
  To: Junho Ryu; +Cc: hughd, tytso, branto, xfs

On Tue, Dec 10, 2013 at 12:12:00PM -0800, Junho Ryu wrote:
> From: Hugh Dickins <hughd@google.com>
> 
> add executable permission to generic/311, generic/314 and generic/315.
> Just like other tests in the same directory.
> 
> Signed-off-by: Hugh Dickins <hughd@google.com>
> Signed-off-by: Junho Ryu <jayr@google.com>

Looks good, but somehow the permissions seem to always get messed up
in xfstests anyway..

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 10/10] xfstests: skip parts of tests which cannot work on tmpfs
  2013-12-10 20:12 ` [PATCH 10/10] xfstests: skip parts of tests which cannot work on tmpfs Junho Ryu
@ 2013-12-11  7:51   ` Christoph Hellwig
  0 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2013-12-11  7:51 UTC (permalink / raw)
  To: Junho Ryu; +Cc: hughd, tytso, branto, xfs

On Tue, Dec 10, 2013 at 12:12:01PM -0800, Junho Ryu wrote:
> Skip generic/076 which tests concurrent access while testing tmpfs
> because there are other tests cover the case using fsstress.

Should check if SCRATCH_DEV is a block device.

> Skip generic/277 which requires chattr.

Add a feature test to see if the filesystem supports chattr.  Might
be worth to have a common _require_chattr helper chared with generic/079

> 
> Even if generic/053 will not be able to test exactly what the test has
> been intended to test, it will be still useful to have this test to try
> out some functionality on tmpfs rather than always failing or being
> skipped.

I'd rather add a simple ACL tests that covers this without the unmount
an repair and maybe more.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 05/10] xfstests: do not unmount tmpfs during remount.
  2013-12-11  7:46   ` Christoph Hellwig
@ 2013-12-11 22:40     ` Dave Chinner
  2013-12-12  0:16       ` Theodore Ts'o
  2013-12-12 18:01       ` Christoph Hellwig
  0 siblings, 2 replies; 36+ messages in thread
From: Dave Chinner @ 2013-12-11 22:40 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Junho Ryu, hughd, tytso, branto, xfs

On Tue, Dec 10, 2013 at 11:46:15PM -0800, Christoph Hellwig wrote:
> On Tue, Dec 10, 2013 at 12:11:56PM -0800, Junho Ryu wrote:
> > Several tests unmount then re-mount the scratch filesystem, to check
> > that the content is unchanged; but unmounting a tmpfs is designed to
> > lose its content, which causes such tests to fail unnecessarily.  Add
> > _scratch_remount(), which instead uses remount on a tmpfs to preserve
> > its contents.  Also add _test_remount() to do the same for the test
> > device.
> 
> All these tests don't make sense if you never unmount the filesystem.
> Istead they should exit with _notrun for tmpfs.

IOWs, adding tmpfs changes the definition of a "generic" test.

i.e. instead of:

_supported_fs generic

these tests are now:

_supported_fs xfs ext2 ext3 ext4 ext4dev btrfs gfs2 nfs udf reiserfs

and by that definition should be in the tests/shared directory....

That's a bit of a pain, but I really don't like the idea of having
"generic" tests that aren't actually generic by having to define
exceptions for them...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 06/10] xfstests: fix generic/225 to check fiemap support
  2013-12-11  7:46   ` Christoph Hellwig
@ 2013-12-11 22:42     ` Dave Chinner
  2013-12-12 18:01       ` Christoph Hellwig
  0 siblings, 1 reply; 36+ messages in thread
From: Dave Chinner @ 2013-12-11 22:42 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Junho Ryu, hughd, tytso, branto, xfs

On Tue, Dec 10, 2013 at 11:46:57PM -0800, Christoph Hellwig wrote:
> On Tue, Dec 10, 2013 at 12:11:57PM -0800, Junho Ryu wrote:
> > generic/225 runs fiemap.  It would be nice to call
> > _require_xfs_io_fiemap like other tests which use fiemap.
> 
> It's not exactly correct as it doesn't require xfs_io.  If you have some
> spare cycles it would be good to factor out a _require_fiemap from it.

I'd just rename if _require_fiemap, because we need both xfs_io and
kernel support for it, and we need xfs_io support to check for
kernel support....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 05/10] xfstests: do not unmount tmpfs during remount.
  2013-12-11 22:40     ` Dave Chinner
@ 2013-12-12  0:16       ` Theodore Ts'o
  2013-12-12  0:53         ` Dave Chinner
  2013-12-12 18:01       ` Christoph Hellwig
  1 sibling, 1 reply; 36+ messages in thread
From: Theodore Ts'o @ 2013-12-12  0:16 UTC (permalink / raw)
  To: Dave Chinner; +Cc: Christoph Hellwig, Junho Ryu, hughd, branto, xfs

On Thu, Dec 12, 2013 at 09:40:12AM +1100, Dave Chinner wrote:
> > 
> > All these tests don't make sense if you never unmount the filesystem.
> > Istead they should exit with _notrun for tmpfs.
> 
> IOWs, adding tmpfs changes the definition of a "generic" test.
> 
> i.e. instead of:
> 
> _supported_fs generic
> 
> these tests are now:
> 
> _supported_fs xfs ext2 ext3 ext4 ext4dev btrfs gfs2 nfs udf reiserfs
> 
> and by that definition should be in the tests/shared directory....

At a higher level, I wonder how useful having the distinction between
"generic" and "shared" really is.  Suppose in the future we add some
tests for networked file systems or cluster file systems --- do we end
up migrating even more tests from "generic" to "shared" when we find
tests that don't work for some new file system?

And to the extent that we have things like 

_require_<feature>

which skips certain tests, it's not even true that all generic tests
are run for all file systems.  So what should be the proper
distinction between generic and shared tests?  Whether we use
"_require_*" vs "_supported_fs"?  And if so, it seems rather arbitrary
that we decide whether not running a test based on the file system
type being <foo> is fundamentally different from deciding not to run a
test becuase we are missing the feature <bar>.

Perhaps it would make more sense to move all of the generic test to
shared, and eliminating the distinction?  That way it also becomes
easier becase we don't need to remember whether a test is generic/NNN
vs shared/NNN.  :-)

Just a thought,

					- Ted

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 05/10] xfstests: do not unmount tmpfs during remount.
  2013-12-12  0:16       ` Theodore Ts'o
@ 2013-12-12  0:53         ` Dave Chinner
  0 siblings, 0 replies; 36+ messages in thread
From: Dave Chinner @ 2013-12-12  0:53 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: Christoph Hellwig, Junho Ryu, hughd, branto, xfs

On Wed, Dec 11, 2013 at 07:16:55PM -0500, Theodore Ts'o wrote:
> On Thu, Dec 12, 2013 at 09:40:12AM +1100, Dave Chinner wrote:
> > > 
> > > All these tests don't make sense if you never unmount the filesystem.
> > > Istead they should exit with _notrun for tmpfs.
> > 
> > IOWs, adding tmpfs changes the definition of a "generic" test.
> > 
> > i.e. instead of:
> > 
> > _supported_fs generic
> > 
> > these tests are now:
> > 
> > _supported_fs xfs ext2 ext3 ext4 ext4dev btrfs gfs2 nfs udf reiserfs
> > 
> > and by that definition should be in the tests/shared directory....
> 
> At a higher level, I wonder how useful having the distinction between
> "generic" and "shared" really is.  Suppose in the future we add some
> tests for networked file systems or cluster file systems --- do we end
> up migrating even more tests from "generic" to "shared" when we find
> tests that don't work for some new file system?

That's not the problem I'm pointing out. The physical location ofthe
test at this point is irrelevant.

The fundamental problem is that "generic" is a wildcard that we use
to match all filesystems, and so adding a new filesystem changes
the definition and suddenly tests that were generic now no longer
run on all filesystems.

> And to the extent that we have things like 
> 
> _require_<feature>
> 
> which skips certain tests, it's not even true that all generic tests
> are run for all file systems.

_supported_fs is just a _requires_<foo> statement on steriods.
"generic" simply documents tests that are expected to run on every
single filesystem we can throw at it.

The thing is, tmpfs is a very special snowflake - it's a volatile
filesystem. xfstests was never designed to handle filesystems that
destroy their contents on unmount, and so adding a filesystem that
*doesn't support unmount/remount* breaks fundamental assumptions the
test harness makes about how filesystems behave.

Hence adding tmpfs support means that a generic test can no longer
unmount and mount filesystems. That's a major change in test
architecture right there, and I'm not sure we start turning stuff
upside down to support wacky quirks like volatile filesystems we
can't unmount...

That's the bigger issue here - tmpfs fundamentally violates the
assumption that xfstests is based on - that filesystems are
non-volatile and persist across unmount and hence can be rigorously
validated after an unmount...

The result of this is it changes cwthe definition of what a
"generic" filesystem is, and then everything flows downhill from
there.

> Perhaps it would make more sense to move all of the generic test to
> shared, and eliminating the distinction?  That way it also becomes
> easier becase we don't need to remember whether a test is generic/NNN
> vs shared/NNN.  :-)

Again, that does not solve the problem of tmpfs changing the
definition of a "generic" filesystem....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 05/10] xfstests: do not unmount tmpfs during remount.
  2013-12-11 22:40     ` Dave Chinner
  2013-12-12  0:16       ` Theodore Ts'o
@ 2013-12-12 18:01       ` Christoph Hellwig
  2013-12-12 22:56         ` Dave Chinner
  1 sibling, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2013-12-12 18:01 UTC (permalink / raw)
  To: Dave Chinner; +Cc: tytso, Junho Ryu, hughd, xfs, Christoph Hellwig, branto

On Thu, Dec 12, 2013 at 09:40:12AM +1100, Dave Chinner wrote:
> IOWs, adding tmpfs changes the definition of a "generic" test.
> 
> i.e. instead of:
> 
> _supported_fs generic
> 
> these tests are now:
> 
> _supported_fs xfs ext2 ext3 ext4 ext4dev btrfs gfs2 nfs udf reiserfs
> 
> and by that definition should be in the tests/shared directory....
> 
> That's a bit of a pain, but I really don't like the idea of having
> "generic" tests that aren't actually generic by having to define
> exceptions for them...

We already have tons of these:

tests/generic/079:    _notrun "Setting immutable/append flag not supported"
tests/generic/192:      _notrun "this test makes no sense with noatime"
tests/generic/225:            _notrun "FIEMAP not supported by this filesystem type: $FSTYP"
tests/generic/251:_test_batched_discard $SCRATCH_MNT || _notrun "FITRIM not supported on $SCRATCH_DEV"
tests/generic/260:_test_batched_discard $SCRATCH_MNT || _notrun "FITRIM not supported on $SCRATCH_DEV"
tests/generic/288:_test_batched_discard $SCRATCH_MNT || _notrun "FITRIM
 not supported on $SCRATCH_DEV"


tests/generic/053:_require_acls
tests/generic/077:_require_acls
tests/generic/099:_require_acls
tests/generic/105:_require_acls
tests/generic/237:_require_acls
tests/generic/314:_require_acls
tests/generic/318:_require_acls
tests/generic/319:_require_acls

tests/generic/020:_require_attrs
tests/generic/062:_require_attrs
tests/generic/070:_require_attrs
tests/generic/077:_require_attrs
tests/generic/079:_require_attrs
tests/generic/093:_require_attrs
tests/generic/097:_require_attrs
tests/generic/117:_require_attrs

tests/generic/213:_require_xfs_io_falloc
tests/generic/214:_require_xfs_io_falloc
tests/generic/223:_require_xfs_io_falloc
tests/generic/228:_require_xfs_io_falloc
tests/generic/255:_require_xfs_io_falloc_punch
tests/generic/255:_require_xfs_io_falloc
tests/generic/256:_require_xfs_io_falloc_punch
tests/generic/274:_require_xfs_io_falloc
tests/generic/312:_require_xfs_io_falloc
tests/generic/316:_require_xfs_io_falloc_punch

tests/generic/068:_require_freeze
tests/generic/280:_require_freezeA

tests/generic/219:_require_quota
tests/generic/230:_require_quota
tests/generic/231:_require_quota
tests/generic/232:_require_quota
tests/generic/233:_require_quota
tests/generic/234:_require_quota
tests/generic/235:_require_quota
tests/generic/270:_require_quota
tests/generic/280:_require_quota

tests/generic/285:_require_seek_data_hole
tests/generic/286:_require_seek_data_hole

tests/generic/014:_require_sparse_files
tests/generic/129:_require_sparse_files
tests/generic/130:_require_sparse_files
tests/generic/239:_require_sparse_files
tests/generic/240:_require_sparse_files

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 06/10] xfstests: fix generic/225 to check fiemap support
  2013-12-11 22:42     ` Dave Chinner
@ 2013-12-12 18:01       ` Christoph Hellwig
  2013-12-12 22:44         ` Junho Ryu
  0 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2013-12-12 18:01 UTC (permalink / raw)
  To: Dave Chinner; +Cc: tytso, Junho Ryu, hughd, xfs, Christoph Hellwig, branto

On Thu, Dec 12, 2013 at 09:42:29AM +1100, Dave Chinner wrote:
> I'd just rename if _require_fiemap, because we need both xfs_io and
> kernel support for it, and we need xfs_io support to check for
> kernel support....

Fine with me.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 06/10] xfstests: fix generic/225 to check fiemap support
  2013-12-12 18:01       ` Christoph Hellwig
@ 2013-12-12 22:44         ` Junho Ryu
  2013-12-12 23:00           ` Dave Chinner
  0 siblings, 1 reply; 36+ messages in thread
From: Junho Ryu @ 2013-12-12 22:44 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Hugh Dickins, tytso, Boris Ranto, xfs

>> I'd just rename if _require_fiemap, because we need both xfs_io and
>> kernel support for it, and we need xfs_io support to check for
>> kernel support....
>
> Fine with me.

There are 3 more _require_xfs_io_.* functions. I can create another
patch if you want these names to be consistent.

_require_xfs_io_falloc()
_require_xfs_io_falloc_punch()
_require_xfs_io_zero()

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 05/10] xfstests: do not unmount tmpfs during remount.
  2013-12-12 18:01       ` Christoph Hellwig
@ 2013-12-12 22:56         ` Dave Chinner
  2013-12-13  0:00           ` Junho Ryu
                             ` (2 more replies)
  0 siblings, 3 replies; 36+ messages in thread
From: Dave Chinner @ 2013-12-12 22:56 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Junho Ryu, hughd, tytso, branto, xfs

On Thu, Dec 12, 2013 at 10:01:30AM -0800, Christoph Hellwig wrote:
> On Thu, Dec 12, 2013 at 09:40:12AM +1100, Dave Chinner wrote:
> > IOWs, adding tmpfs changes the definition of a "generic" test.
> > 
> > i.e. instead of:
> > 
> > _supported_fs generic
> > 
> > these tests are now:
> > 
> > _supported_fs xfs ext2 ext3 ext4 ext4dev btrfs gfs2 nfs udf reiserfs
> > 
> > and by that definition should be in the tests/shared directory....
> > 
> > That's a bit of a pain, but I really don't like the idea of having
> > "generic" tests that aren't actually generic by having to define
> > exceptions for them...
> 
> We already have tons of these:

Yes, but that's not the same situation as tmpfs here.  A generic
filesystem fits a certain model but the given configuration/kernel
does not necessarily support the feature being tested. Those
features are easily testable by a _requires_* line.

This case with tmpfs is different - it doesn't support *being
unmounted* during a test because it is volatile. That's a
fundamental change to the assumptions xfstests makes about
filesystems being tested.  IOWs, we've got a "generic" filesystem
that is anything but generic.

Adding "_requires_non_volatile_fs" to all the generic tests that do
an unmount so that tmpfs can be considered "generic" is just as
painful as modifying the "_supported_fs generic" lines in all the
tests. It just leads us to a game of whack-a-mole.

I don't know what the solution here is - everything I think of is
either messy, ugly or unmaintainable. All I'm trying to do is find a
way to handle tmpfs filesystems in a way that is maintainable and
doesn't require every developer to be aware of the quirks of tmpfs
when writing and reviewing new generic tests....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 06/10] xfstests: fix generic/225 to check fiemap support
  2013-12-12 22:44         ` Junho Ryu
@ 2013-12-12 23:00           ` Dave Chinner
  0 siblings, 0 replies; 36+ messages in thread
From: Dave Chinner @ 2013-12-12 23:00 UTC (permalink / raw)
  To: Junho Ryu; +Cc: Christoph Hellwig, tytso, Hugh Dickins, Boris Ranto, xfs

On Thu, Dec 12, 2013 at 02:44:47PM -0800, Junho Ryu wrote:
> >> I'd just rename if _require_fiemap, because we need both xfs_io and
> >> kernel support for it, and we need xfs_io support to check for
> >> kernel support....
> >
> > Fine with me.
> 
> There are 3 more _require_xfs_io_.* functions. I can create another
> patch if you want these names to be consistent.
> 
> _require_xfs_io_falloc()
> _require_xfs_io_falloc_punch()

yes, those 2 can be changed similar to _requires_fiemap

> _require_xfs_io_zero()

That one is only testing for xfs_io command support, so it should
remain the way it is...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 05/10] xfstests: do not unmount tmpfs during remount.
  2013-12-12 22:56         ` Dave Chinner
@ 2013-12-13  0:00           ` Junho Ryu
  2013-12-13  1:41             ` Dave Chinner
  2013-12-13  4:56           ` Theodore Ts'o
  2013-12-13 11:04           ` Christoph Hellwig
  2 siblings, 1 reply; 36+ messages in thread
From: Junho Ryu @ 2013-12-13  0:00 UTC (permalink / raw)
  To: Dave Chinner
  Cc: Christoph Hellwig, Theodore Ts'o, Hugh Dickins, Boris Ranto, xfs

> I don't know what the solution here is - everything I think of is
> either messy, ugly or unmaintainable. All I'm trying to do is find a
> way to handle tmpfs filesystems in a way that is maintainable and
> doesn't require every developer to be aware of the quirks of tmpfs
> when writing and reviewing new generic tests....

If it is acceptable that tmpfs running tests which does not make much
sense without actually re-mounting devices, all other developers need
to care is using _scratch_remount() and _test_remount().

Even if someone does not use the functions, tests will only fail on
tmpfs, and people like me who cares about it will be happy to fix it.

So far, generic/053 is the only test which does something else between
umount and mount.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 05/10] xfstests: do not unmount tmpfs during remount.
  2013-12-13  0:00           ` Junho Ryu
@ 2013-12-13  1:41             ` Dave Chinner
  2013-12-13 11:12               ` Christoph Hellwig
  0 siblings, 1 reply; 36+ messages in thread
From: Dave Chinner @ 2013-12-13  1:41 UTC (permalink / raw)
  To: Junho Ryu
  Cc: Christoph Hellwig, Theodore Ts'o, Hugh Dickins, Boris Ranto, xfs

On Thu, Dec 12, 2013 at 04:00:44PM -0800, Junho Ryu wrote:
> > I don't know what the solution here is - everything I think of is
> > either messy, ugly or unmaintainable. All I'm trying to do is find a
> > way to handle tmpfs filesystems in a way that is maintainable and
> > doesn't require every developer to be aware of the quirks of tmpfs
> > when writing and reviewing new generic tests....
> 
> If it is acceptable that tmpfs running tests which does not make much
> sense without actually re-mounting devices, all other developers need
> to care is using _scratch_remount() and _test_remount().

And how are they to know whether it makes sense ot run on tmpfs or
not? That's the point I'm trying to make - tmpfs adds new
restrictions on how tests are written or constructed, and we still
need a method of saying no to tmpfs....

> Even if someone does not use the functions, tests will only fail on
> tmpfs, and people like me who cares about it will be happy to fix it.

Yes, that's the game of whack-a-mole I was talking about.

> So far, generic/053 is the only test which does something else between
> umount and mount.

All the generic tests that use dm_flakey are likely to be busted.
Anything assumes SCRATCH_DEV or TEST_DEV are block devices are
busted. Do loop devices work properly when hosted on tmpfs
filesystems? And so on...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 05/10] xfstests: do not unmount tmpfs during remount.
  2013-12-12 22:56         ` Dave Chinner
  2013-12-13  0:00           ` Junho Ryu
@ 2013-12-13  4:56           ` Theodore Ts'o
  2013-12-13 11:04           ` Christoph Hellwig
  2 siblings, 0 replies; 36+ messages in thread
From: Theodore Ts'o @ 2013-12-13  4:56 UTC (permalink / raw)
  To: Dave Chinner; +Cc: Christoph Hellwig, Junho Ryu, hughd, branto, xfs

On Fri, Dec 13, 2013 at 09:56:57AM +1100, Dave Chinner wrote:
> This case with tmpfs is different - it doesn't support *being
> unmounted* during a test because it is volatile. That's a
> fundamental change to the assumptions xfstests makes about
> filesystems being tested....
> 
> I don't know what the solution here is - everything I think of is
> either messy, ugly or unmaintainable. All I'm trying to do is find a
> way to handle tmpfs filesystems in a way that is maintainable and
> doesn't require every developer to be aware of the quirks of tmpfs
> when writing and reviewing new generic tests....

There should be a relatively small number of reasons why a generic
test would need to umount and remount a file system; the most common
case is so it can run fsck on the file system.

What's actually strange is that is that generic/053 is explicitly
umounting and remounting the file system:

_do 'unmount $SCRATCH_DEV' 'umount $SCRATCH_DEV'
_do 'repair filesystem' '_check_scratch_fs'
_do 'mount filesytem' '_scratch_mount'

In fact, that's not necessary, because _check_test_fs and
_check_scratch_fs will take care of umounting and remounting the file
system.  So if this is the only problem case which Junho has found,
why not just patch generic/053 so it doesn't explicitly umount and
remount the file system, since we've already taught _check_*_fs to be
a no-op for tmpfs.

As for dm_flakey, tests, we can just have _require_dm_flaky be false
for tmpfs file system.

So we're still playing whack-a-mole, yes, but on classes of test
requirements instead of individual tests.  If we address the
umount/remount for fsck and dm_flakey, are there other significant
classes of tests that would still be problematic for tmpfs?

						- Ted

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 05/10] xfstests: do not unmount tmpfs during remount.
  2013-12-12 22:56         ` Dave Chinner
  2013-12-13  0:00           ` Junho Ryu
  2013-12-13  4:56           ` Theodore Ts'o
@ 2013-12-13 11:04           ` Christoph Hellwig
  2 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2013-12-13 11:04 UTC (permalink / raw)
  To: Dave Chinner; +Cc: tytso, Junho Ryu, hughd, xfs, Christoph Hellwig, branto

On Fri, Dec 13, 2013 at 09:56:57AM +1100, Dave Chinner wrote:
> Yes, but that's not the same situation as tmpfs here.  A generic
> filesystem fits a certain model but the given configuration/kernel
> does not necessarily support the feature being tested. Those
> features are easily testable by a _requires_* line.
> 
> This case with tmpfs is different - it doesn't support *being
> unmounted* during a test because it is volatile. That's a
> fundamental change to the assumptions xfstests makes about
> filesystems being tested.  IOWs, we've got a "generic" filesystem
> that is anything but generic.

It's not really a fundamental change for tests supposed to be run on
the TEST_DIR fs.  In fact I'd go so far to say that one patch that
unmounts and repairs it manually should switch to the scratch fs.

Running the scratch tests on tmpfs is a bit more fragile, but I don't
really see a big problem with the concept.

> Adding "_requires_non_volatile_fs" to all the generic tests that do
> an unmount so that tmpfs can be considered "generic" is just as
> painful as modifying the "_supported_fs generic" lines in all the
> tests. It just leads us to a game of whack-a-mole.

What's the problem?  It's not that bad if you look at the series, and
as long as someone maintains tmpfs support it shouldn't burden anyone
else.  If it stops being maintained we can drop the support after a
while.  Then again we still have utterly bitrotted IRIX support around..

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 05/10] xfstests: do not unmount tmpfs during remount.
  2013-12-13  1:41             ` Dave Chinner
@ 2013-12-13 11:12               ` Christoph Hellwig
  0 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2013-12-13 11:12 UTC (permalink / raw)
  To: Dave Chinner
  Cc: Theodore Ts'o, Junho Ryu, Hugh Dickins, xfs,
	Christoph Hellwig, Boris Ranto

On Fri, Dec 13, 2013 at 12:41:31PM +1100, Dave Chinner wrote:
> All the generic tests that use dm_flakey are likely to be busted.
> Anything assumes SCRATCH_DEV or TEST_DEV are block devices are
> busted.

These are already busted on nfs, which we've support in some form
forever, and cifs, and btrfs with it's pools if they assume the
actual device is the block device.

> Do loop devices work properly when hosted on tmpfs
> filesystems? And so on...

They do.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 01/10] xfstests: Add tmpfs support
  2013-12-10 20:11 ` [PATCH 01/10] xfstests: Add tmpfs support Junho Ryu
  2013-12-11  7:40   ` Christoph Hellwig
@ 2013-12-17 16:40   ` Rich Johnston
  1 sibling, 0 replies; 36+ messages in thread
From: Rich Johnston @ 2013-12-17 16:40 UTC (permalink / raw)
  To: Junho Ryu, xfs; +Cc: hughd, tytso, branto

This patch stands alone and has been committed.

Thanks
--Rich

commit bc95a5351f824e8cf18d13667b9c2e439045259f
Author: Boris Ranto <branto@redhat.com>
Date:   Tue Dec 10 20:11:52 2013 +0000

     xfstests: Add tmpfs support

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2013-12-17 16:40 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-10 20:11 [PATCH 00/10] Add tmpfs filesystem support Junho Ryu
2013-12-10 20:11 ` [PATCH 01/10] xfstests: Add tmpfs support Junho Ryu
2013-12-11  7:40   ` Christoph Hellwig
2013-12-17 16:40   ` Rich Johnston
2013-12-10 20:11 ` [PATCH 02/10] xfstests: use mount point instead of device name Junho Ryu
2013-12-11  7:42   ` Christoph Hellwig
2013-12-10 20:11 ` [PATCH 03/10] xfstests: _scratch_mkfs_sized() for tmpfs Junho Ryu
2013-12-11  7:44   ` Christoph Hellwig
2013-12-10 20:11 ` [PATCH 04/10] xfstests: increase tmpfs memory size Junho Ryu
2013-12-11  7:44   ` Christoph Hellwig
2013-12-10 20:11 ` [PATCH 05/10] xfstests: do not unmount tmpfs during remount Junho Ryu
2013-12-11  7:46   ` Christoph Hellwig
2013-12-11 22:40     ` Dave Chinner
2013-12-12  0:16       ` Theodore Ts'o
2013-12-12  0:53         ` Dave Chinner
2013-12-12 18:01       ` Christoph Hellwig
2013-12-12 22:56         ` Dave Chinner
2013-12-13  0:00           ` Junho Ryu
2013-12-13  1:41             ` Dave Chinner
2013-12-13 11:12               ` Christoph Hellwig
2013-12-13  4:56           ` Theodore Ts'o
2013-12-13 11:04           ` Christoph Hellwig
2013-12-10 20:11 ` [PATCH 06/10] xfstests: fix generic/225 to check fiemap support Junho Ryu
2013-12-11  7:46   ` Christoph Hellwig
2013-12-11 22:42     ` Dave Chinner
2013-12-12 18:01       ` Christoph Hellwig
2013-12-12 22:44         ` Junho Ryu
2013-12-12 23:00           ` Dave Chinner
2013-12-10 20:11 ` [PATCH 07/10] xfstests: fix generic/127 to call _cleanup() only once Junho Ryu
2013-12-11  7:47   ` Christoph Hellwig
2013-12-10 20:11 ` [PATCH 08/10] xfstests: check O_DIRECT support before testing direct I/O Junho Ryu
2013-12-11  7:47   ` Christoph Hellwig
2013-12-10 20:12 ` [PATCH 09/10] xfstests: add executable permission to tests Junho Ryu
2013-12-11  7:48   ` Christoph Hellwig
2013-12-10 20:12 ` [PATCH 10/10] xfstests: skip parts of tests which cannot work on tmpfs Junho Ryu
2013-12-11  7:51   ` Christoph Hellwig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.