common: add support for the "local" file system type
PATCH 1/1 common: add support for the "local" file system type
About a year and half ago, I sent the patch attached below to add support for a "local" file system type where the file system could not be mounted or dismounted, but where writes to TEST_DIR and SCRATCH_MNT would be testing the file system in question. At the time, I couldn't describe the use case in any greater detail than what I had in the commit description, and Dave Chinner at the time rejected the patch since he didn't like xfstests being used to test a proprietary file system for which I was not able to explain the details of what we were trying to do.
Not a big deal, I just kept the patch in my private fork[1] of xfstests on github.
[1] https://github.com/tytso/xfstests
However, happily, we can now talk a lot more about what the "local" file system type in xfstests was used to test. Earlier today, Google announced gVisor[2], and published it as open source on github[3]. gVisor works much like User Mode Linux, and I suspect much like the Windows Subsystem for Linux in that it uses the x86_64's hardware virtualization extensions (so it has the security fencing much like a VM) but instead of emulating hardware, instead the emulation layer is done at the system call level (so it's more efficient than a VM, since there is no guest kernel).
[2] https://cloudplatform.googleblog.com/2018/05/Open-sourcing-gVisor-a-sandboxed-container-runtime.html
[3] https://github.com/google/gvisor
File systems can be implemented using Gophers[4] in a separate process, where the communication between the gVisor sandbox and the Gopher is via the 9P2000.L protocol. This means that if you want to try to exploit a buffer overflow in the userspace file system, first you have to get past the system call validation checks done by the gVisor Sentry process (which is written in Go which makes this rather more difficult, especially since the Sentry process itself is protected using seccomp[5]). Then the buffer overflow attack has to make it past the 9P protocol encoding/decoding, and then survive the 9P protocol validation checks in the Gopher. The Gopher process provides an emulated file system service using the Cloud Provider's internal cluster storage services, much like in GCE, the Persistent Disk service provides an emulated block device service.
[4] https://news.ycombinator.com/item?id=16979126
[5] https://news.ycombinator.com/item?id=16976557
This whole system was designed with security first and foremost. Of course, we also want it to pass the file system checks, which is why were using xfstests.
The way we actually tested the gVisor file system was via a Docker container (the Dockerfile[6] is in the xfstests-bld git repo) which would then get consumed by gVisor's Docker/Kubernetes integration layer.
[6] https://github.com/tytso/xfstests-bld/blob/master/Dockerfile
Anyway, back in September, 2016, Dave Chinner was peeved that I couldn't give this full description, and I'm glad to say, now we can finally rectify that gap. :-)
Could this commit therefore please be considered for inclusion in xfstests upstream?
Many thanks!
- Ted
Quoted 1 line
From
8b40b28866dca119d0c807c31ae48f153ec2dc53Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <[email protected]>
Date: Sat, 17 Sep 2016 19:08:18 -0400
Subject: [PATCH] common: add support for the "local" file system type
It is sometimes useful to be able to test the local file system provided in a restricted execution environment (such as that which is provided by Docker, for example) where it is not possible to mount and unmount the file system under test.
To support this test case, add support for a new file system type called "local". The TEST_DEV and SCRATCH_DEV should be have a non-block device format (e.g., local:/test or local:/scratch), and the TEST_DIR and SCRATCH_MNT directories should be pre-existing directories provided by the execution environment.
--- common/rc | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 70 insertions(+), 3 deletions(-)
common/rc +70 -3
@@ -351,6 +351,18 @@ _supports_filetype() esac } +_local_validate_mount_opt() +{ + case "$*" in + ro|ro,*|*,ro) _notrun "ro mount option not supported" ;; + *nosuid*) _notrun "nosuid mount option not supported" ;; + *noatime*) _notrun "noatime mount option not supported" ;; + *relatime*) _notrun "relatime mount option not supported" ;; + *diratime*) _notrun "diratime mount option not supported" ;; + *strictatime*) _notrun "strictatime mount option not supported" ;; + esac +} + # mount scratch device with given options but don't check mount status _try_scratch_mount() {
@@ -376,6 +388,9 @@ _scratch_unmount() btrfs) $UMOUNT_PROG $SCRATCH_MNT ;; + local) + rm -rf $SCRATCH_MNT/* + ;; *) $UMOUNT_PROG $SCRATCH_DEV ;;
@@ -386,6 +401,10 @@ _scratch_remount() { local opts="$1" + if [ "$FSTYP" = "local" ]; then + _local_validate_mount_opt "$*" + return 0; + fi if test -n "$opts"; then mount -o "remount,$opts" $SCRATCH_MNT fi
@@ -395,7 +414,7 @@ _scratch_cycle_mount() { local opts="$1" - if [ "$FSTYP" = tmpfs ]; then + if [ "$FSTYP" = "tmpfs" -o "$FSTYP" = "local" ] ; then _scratch_remount "$opts" return fi
@@ -429,6 +448,10 @@ _test_mount() _overlay_test_mount $* return $? fi + if [ "$FSTYP" == "local" ]; then + mkdir -p $TEST_DIR + return $? + fi _test_options mount _mount -t $FSTYP $TEST_OPTIONS $TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS $* $TEST_DEV $TEST_DIR }
@@ -437,7 +460,7 @@ _test_unmount() { if [ "$FSTYP" == "overlay" ]; then _overlay_test_unmount - else + elif [ "$FSTYP" != "local" ]; then $UMOUNT_PROG $TEST_DEV fi }
@@ -723,7 +746,7 @@ _scratch_mkfs() local mkfs_status case $FSTYP in - nfs*|cifs|ceph|overlay|glusterfs|pvfs2|9p) + nfs*|cifs|ceph|overlay|glusterfs|pvfs2|9p|local) # unable to re-create this fstyp, just remove all files in # $SCRATCH_MNT to avoid EEXIST caused by the leftover files # created in previous runs
@@ -1465,6 +1488,10 @@ _check_mounted_on() local mnt=$4 local type=$5 + if [ "$FSTYP" == "local" ]; then + return 0 + fi + # find $dev as the source, and print result in "$dev $mnt" format local mount_rec=`findmnt -rncv -S $dev -o SOURCE,TARGET` [ -n "$mount_rec" ] || return 1 # 1 = not mounted
@@ -1562,6 +1589,13 @@ _require_scratch_nocheck() _notrun "this test requires a valid \$SCRATCH_MNT" fi ;; + local) + if [ -z "$SCRATCH_DEV" -o ! -d "$SCRATCH_MNT" ]; + then + _notrun "this test requires a valid \$SCRATCH_MNT and unique $SCRATCH_DEV" + fi + return 0 + ;; *) if [ -z "$SCRATCH_DEV" -o "`_is_block_dev "$SCRATCH_DEV"`" = "" ] then
@@ -1683,6 +1717,13 @@ _require_test() _notrun "this test requires a valid \$TEST_DIR" fi ;; + local) + if [ -z "$TEST_DEV" -o ! -d "$TEST_DIR" ]; + then + _notrun "this test requires a valid \$TEST_DIR and unique $TEST_DEV" + fi + return 0 + ;; *) if [ -z "$TEST_DEV" ] || [ "`_is_block_dev "$TEST_DEV"`" = "" ] then
@@ -2438,6 +2479,10 @@ _remount() local device=$1 local mode=$2 + if [ "$FSTYP" == "local" ] ; then + return 0 + fi + if ! mount -o remount,$mode $device then echo "_remount: failed to remount filesystem on $device as $mode"
@@ -2483,6 +2528,10 @@ _mount_or_remount_rw() local device=$2 local mountpoint=$3 + if [ "$FSTYP" == "local" ] ; then + return 0 + fi + if [ $USE_REMOUNT -eq 0 ]; then if [ "$FSTYP" != "overlay" ]; then _mount -t $FSTYP $mount_opts $device $mountpoint
@@ -2636,6 +2685,9 @@ _check_test_fs() ubifs) # there is no fsck program for ubifs yet ;; + local) + # no way to check consistency for local + ;; *) _check_generic_filesystem $TEST_DEV ;;
@@ -2691,6 +2743,9 @@ _check_scratch_fs() ubifs) # there is no fsck program for ubifs yet ;; + local) + # no way to check consistency for local + ;; *) _check_generic_filesystem $device ;;
@@ -3003,6 +3058,9 @@ _require_fio() # Does freeze work on this fs? _require_freeze() { + if [ "$FSTYP" == "local" ]; then + _notrun "local does not support freeze" + fi xfs_freeze -f "$TEST_DIR" >/dev/null 2>&1 local result=$? xfs_freeze -u "$TEST_DIR" >/dev/null 2>&1
@@ -3024,6 +3082,9 @@ _require_scratch_shutdown() { [ -x src/godown ] || _notrun "src/godown executable not found" + if [ "$FSTYP" == "local" ]; then + _notrun "local does not support shutdown" + fi _scratch_mkfs > /dev/null 2>&1 || _notrun "_scratch_mkfs failed on $SCRATCH_DEV" _scratch_mount
@@ -3049,6 +3110,9 @@ _require_scratch_shutdown() # Does dax mount option work on this dev/fs? _require_scratch_dax() { + if [ "$FSTYP" == "local" ]; then + _notrun "local does not support dax" + fi _require_scratch _scratch_mkfs > /dev/null 2>&1 _try_scratch_mount -o dax || \
@@ -3063,6 +3127,9 @@ _require_scratch_dax() # Does norecovery support by this fs? _require_norecovery() { + if [ "$FSTYP" == "local" ]; then + _notrun "local does not support norecovery" + fi _try_scratch_mount -o ro,norecovery || \ _notrun "$FSTYP does not support norecovery" _scratch_unmount
Signature · 1 line
2.16.1.72.g5be1f00a9a
On Thu, May 3, 2018 at 6:43 AM, Theodore Y. Ts'o <[email protected]> wrote:
[...]
Quoted 10 lines
From
8b40b28866dca119d0c807c31ae48f153ec2dc53Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <[email protected]>
Date: Sat, 17 Sep 2016 19:08:18 -0400
Subject: [PATCH] common: add support for the "local" file system typeIt is sometimes useful to be able to test the local file system provided in a restricted execution environment (such as that which is provided by Docker, for example) where it is not possible to mount and unmount the file system under test.
Quoted 5 lines
To support this test case, add support for a new file system type called "local". The
TEST_DEVandSCRATCH_DEVshould be have a non-block device format (e.g., local:/test or local:/scratch), and theTEST_DIRandSCRATCH_MNTdirectories should be pre-existing directories provided by the execution environment.
Ted,
This looks like a very useful feature, but I suspect that some bits of the patch may be a bit too specific to your use case (see below) - I may be wrong. I bet there is a large variety of out of tree xfstests patches named "Add support for XXX fs" that could be avoided with this feature.
The ultimate proof would be to demonstrate the usefulness of the feature to more than a single use case - how about FUSE passthrough example? Perhaps FSTYP=user would be more descriptive in general to the use case at hand, because 'local' is usually the counter of 'remote', but I'm fine with FSTYP=local.
Another way to "market" the feature is FSTYP=generic, which is the prototype of all other filesystems. Naturally, it only runs tests under tests/generic (as FSTYP=local does) and any test that requires an operation that has no generic implementation is notrun.
Quoted 15 lines
Signed-off-by Theodore Ts'o <[email protected]>--- common/rc | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 70 insertions(+), 3 deletions(-)common/rc +1
-0@@ -351,6 +351,18 @@ _supports_filetype() esac } +_local_validate_mount_opt()
common/rc _supports_filetype() · line 364
Quoted 10 lines
common/rc +10
-0+{ + case "$*" in + ro|ro,*|*,ro) _notrun "ro mount option not supported" ;; + *nosuid*) _notrun "nosuid mount option not supported" ;; + *noatime*) _notrun "noatime mount option not supported" ;; + *relatime*) _notrun "relatime mount option not supported" ;; + *diratime*) _notrun "diratime mount option not supported" ;; + *strictatime*) _notrun "strictatime mount option not supported" ;; + esac +}
Why specifically these mount options? Is this really generic?
common/rc _scratch_unmount() · line 393
Quoted 11 lines
common/rc +4
-0+ # mount scratch device with given options but don't check mount status _try_scratch_mount() {@@ -376,6 +388,9 @@ _scratch_unmount() btrfs) $UMOUNT_PROG $SCRATCH_MNT ;; + local) + rm -rf $SCRATCH_MNT/* + ;;
_scratch_mkfs already does that. Why does it make sense in _scratch_unmount?
common/rc _scratch_remount() · line 407
Quoted 11 lines
common/rc +4
-0*) $UMOUNT_PROG $SCRATCH_DEV ;;@@ -386,6 +401,10 @@ _scratch_remount() { local opts="$1" + if [ "$FSTYP" = "local" ]; then + _local_validate_mount_opt "$*" + return 0; + fi
It makes more sense to me to _require_scratch_remount for tests that need to remount in the beginning of tests - yeh that's a bit more work.
common/rc _scratch_cycle_mount() · line 418
Quoted 10 lines
common/rc +1
-1if test -n "$opts"; then mount -o "remount,$opts" $SCRATCH_MNT fi@@ -395,7 +414,7 @@ _scratch_cycle_mount() { local opts="$1" - if [ "$FSTYP" = tmpfs ]; then + if [ "$FSTYP" = "tmpfs" -o "$FSTYP" = "local" ] ; then _scratch_remount "$opts"
That seems like cheating - seems better to implement and use _require_scratch_cycle_mount
common/rc _test_mount() · line 454
Quoted 10 lines
common/rc +4
-0return fi@@ -429,6 +448,10 @@ _test_mount() _overlay_test_mount $* return $? fi + if [ "$FSTYP" == "local" ]; then + mkdir -p $TEST_DIR + return $? + fi
What is the desired logic here? can you add a comment? Seems to me that we need to verify there is something mounted at $TEST_DIR. no?
Quoted 22 lines
common/rc +2
-2_test_options mount _mount -t $FSTYP $TEST_OPTIONS $TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS $* $TEST_DEV $TEST_DIR }@@ -437,7 +460,7 @@ _test_unmount() { if [ "$FSTYP" == "overlay" ]; then _overlay_test_unmount - else + elif [ "$FSTYP" != "local" ]; then $UMOUNT_PROG $TEST_DEV fi }@@ -723,7 +746,7 @@ _scratch_mkfs() local mkfs_status case $FSTYP in - nfs*|cifs|ceph|overlay|glusterfs|pvfs2|9p) + nfs*|cifs|ceph|overlay|glusterfs|pvfs2|9p|local) # unable to re-create this fstyp, just remove all files in # $SCRATCH_MNT to avoid EEXIST caused by the leftover files # created in previous runs
common/rc _check_mounted_on() · line 1494
Quoted 7 lines
common/rc +4
-0@@ -1465,6 +1488,10 @@ _check_mounted_on() local mnt=$4 local type=$5 + if [ "$FSTYP" == "local" ]; then + return 0 + fi +
Shouldn't we check that "something" is mounted on $mnt?
Quoted 74 lines
common/rc +28
-0# find $dev as the source, and print result in "$dev $mnt" format local mount_rec=`findmnt -rncv -S $dev -o SOURCE,TARGET` [ -n "$mount_rec" ] || return 1 # 1 = not mounted@@ -1562,6 +1589,13 @@ _require_scratch_nocheck() _notrun "this test requires a valid \$SCRATCH_MNT" fi ;; + local) + if [ -z "$SCRATCH_DEV" -o ! -d "$SCRATCH_MNT" ]; + then + _notrun "this test requires a valid \$SCRATCH_MNT and unique $SCRATCH_DEV" + fi + return 0 + ;; *) if [ -z "$SCRATCH_DEV" -o "`_is_block_dev "$SCRATCH_DEV"`" = "" ] then@@ -1683,6 +1717,13 @@ _require_test() _notrun "this test requires a valid \$TEST_DIR" fi ;; + local) + if [ -z "$TEST_DEV" -o ! -d "$TEST_DIR" ]; + then + _notrun "this test requires a valid \$TEST_DIR and unique $TEST_DEV" + fi + return 0 + ;; *) if [ -z "$TEST_DEV" ] || [ "`_is_block_dev "$TEST_DEV"`" = "" ] then@@ -2438,6 +2479,10 @@ _remount() local device=$1 local mode=$2 + if [ "$FSTYP" == "local" ] ; then + return 0 + fi + if ! mount -o remount,$mode $device then echo "_remount: failed to remount filesystem on $device as $mode"@@ -2483,6 +2528,10 @@ _mount_or_remount_rw() local device=$2 local mountpoint=$3 + if [ "$FSTYP" == "local" ] ; then + return 0 + fi + if [ $USE_REMOUNT -eq 0 ]; then if [ "$FSTYP" != "overlay" ]; then _mount -t $FSTYP $mount_opts $device $mountpoint@@ -2636,6 +2685,9 @@ _check_test_fs() ubifs) # there is no fsck program for ubifs yet ;; + local) + # no way to check consistency for local + ;; *) _check_generic_filesystem $TEST_DEV ;;@@ -2691,6 +2743,9 @@ _check_scratch_fs() ubifs) # there is no fsck program for ubifs yet ;; + local) + # no way to check consistency for local + ;; *) _check_generic_filesystem $device ;;
common/rc _require_fio() · line 3062
Quoted 5 lines
common/rc +2
-0@@ -3003,6 +3058,9 @@ _require_fio() # Does freeze work on this fs? _require_freeze() { + if [ "$FSTYP" == "local" ]; then + _notrun "local does not support freeze"
When users read this warning they may be confused, same for shutdown/dax/morecovery.
Something like "user defined fs does not support freeze"
Thanks,
Amir.
Quoted 3 lines · Amir Goldstein
This looks like a very useful feature, but I suspect that some bits of the patch may be a bit too specific to your use case (see below) - I may be wrong.
The primary characteristic of "local" is there nothing to mount or unmount. So for example, it should be possible to use this Microsoft's Subsystem for Linux.
Quoted 5 lines
The ultimate proof would be to demonstrate the usefulness of the feature to more than a single use case - how about FUSE passthrough example? Perhaps FSTYP=user would be more descriptive in general to the use case at hand, because 'local' is usually the counter of 'remote', but I'm fine with FSTYP=local.
It certainly wouldn't be problem to demo using the local file system type for FUSE --- but it would not be _ideal_ for fuse, since fuse has the concept of mounting and unmounting.
Quoted 4 lines
Quoted 4 lines
common/rc +4
-0+{ + case "$*" in + ro|ro,*|*,ro) _notrun "ro mount option not supported" ;; + *nosuid*) _notrun "nosuid mount option not supported" ;;
Quoted 8 lines
common/rc _supports_filetype() · line 364
Quoted 6 lines
common/rc +6
-0+ *noatime*) _notrun "noatime mount option not supported" ;; + *relatime*) _notrun "relatime mount option not supported" ;; + *diratime*) _notrun "diratime mount option not supported" ;; + *strictatime*) _notrun "strictatime mount option not supported" ;; + esac +}Why specifically these mount options? Is this really generic?
The local file system type does not support mount, umount, or remount command. So there is no way to modulate noatime, relatime, etc. So this would be useful if someone wanted to test Windows System for Linux, for example.
Quoted 5 lines
Quoted 5 lines
common/rc +1
-0+ # mount scratch device with given options but don't check mount status _try_scratch_mount() {@@ -376,6 +388,9 @@ _scratch_unmount()
Quoted 8 lines
common/rc _scratch_unmount() · line 393
Quoted 6 lines
common/rc +3
-0btrfs) $UMOUNT_PROG $SCRATCH_MNT ;; + local) + rm -rf $SCRATCH_MNT/* + ;;
_scratch_mkfsalready does that. Why does it make sense in_scratch_unmount?
Just for cleanup purposes.
Quoted 4 lines
Quoted 4 lines
common/rc +0
-0@@ -386,6 +401,10 @@ _scratch_remount() { local opts="$1"
Quoted 8 lines
common/rc _scratch_remount() · line 407
Quoted 4 lines
common/rc +4
-0+ if [ "$FSTYP" = "local" ]; then + _local_validate_mount_opt "$*" + return 0; + fiIt makes more sense to me to
_require_scratch_remountfor tests that need to remount in the beginning of tests - yeh that's a bit more work.
There are some tests where the remount operation is just to reset the file system. So the test is just to unmount and remount the file system, and making sure the file status are the same. So I didn't want to block all remounts; instead, to let some remounts be no-op's so the rest of the test could still be used to provide test coverage, and only block those remounts which were modulating things like ro/rw, noatime, etc.
Quoted 10 lines
common/rc _scratch_cycle_mount() · line 418
Quoted 7 lines
common/rc +1
-1@@ -395,7 +414,7 @@ _scratch_cycle_mount() { local opts="$1" - if [ "$FSTYP" = tmpfs ]; then + if [ "$FSTYP" = "tmpfs" -o "$FSTYP" = "local" ] ; then _scratch_remount "$opts"That seems like cheating - seems better to implement and use
_require_scratch_cycle_mount
Again, I didn't want to end up skipping a huge number of tests. I didn't consider this "cheating"; I considered it a case of "let's maximize test coverage".
Quoted 8 lines
common/rc _test_mount() · line 454
Quoted 4 lines
common/rc +4
-0+ if [ "$FSTYP" == "local" ]; then + mkdir -p $TEST_DIR + return $? + fiWhat is the desired logic here? can you add a comment? Seems to me that we need to verify there is something mounted at $TEST_DIR. no?
Again -- the big thing about the "local" file system is there is no way to mount, umount, or remount. So no block device, no mount point, nothing to shutdown, etc.
The mkdir -p isn't strictly necessary here. It could be done by the kvm-xfstests or whatever is running the xfstests. It was more of a safety thing, but if people don't like it, we can drop it.
Quoted 10 lines
common/rc _check_mounted_on() · line 1494
Quoted 8 lines
common/rc +4
-0@@ -1465,6 +1488,10 @@ _check_mounted_on() local mnt=$4 local type=$5 + if [ "$FSTYP" == "local" ]; then + return 0 + fi +Shouldn't we check that "something" is mounted on $mnt?
Nope. See above. "local" means never having to say you need to mount something.
Quoted 10 lines
common/rc _require_fio() · line 3062
Quoted 6 lines
common/rc +2
-0@@ -3003,6 +3058,9 @@ _require_fio() # Does freeze work on this fs? _require_freeze() { + if [ "$FSTYP" == "local" ]; then + _notrun "local does not support freeze"When users read this warning they may be confused, same for shutdown/dax/morecovery.
Something like "user defined fs does not support freeze"
I think you have a very different idea of what "local" means. The basic model was that this is something where the file system is provided for use by the docker infrastructure (or by Windows 10 in the WSL case), and so the docker job can't actually mount or unmount the file system. However, it is still desirable to check to see how POSIX compliant the underlying file system might be, and it can also be used to exercise the underlying file system.
I'm not tied to the name "local", but for me what it means is "the local file system". If people want to nominate a different name, I'm certainly open to suggestions.
- Ted
Quoted 85 lines · Theodore Y. Ts'o
Quoted 3 lines · Amir Goldstein
This looks like a very useful feature, but I suspect that some bits of the patch may be a bit too specific to your use case (see below) - I may be wrong.
The primary characteristic of "local" is there nothing to mount or unmount. So for example, it should be possible to use this Microsoft's Subsystem for Linux.
Quoted 5 lines
The ultimate proof would be to demonstrate the usefulness of the feature to more than a single use case - how about FUSE passthrough example? Perhaps FSTYP=user would be more descriptive in general to the use case at hand, because 'local' is usually the counter of 'remote', but I'm fine with FSTYP=local.
It certainly wouldn't be problem to demo using the local file system type for FUSE --- but it would not be _ideal_ for fuse, since fuse has the concept of mounting and unmounting.
Quoted 4 lines
Quoted 4 lines
common/rc +4
-0+{ + case "$*" in + ro|ro,*|*,ro) _notrun "ro mount option not supported" ;; + *nosuid*) _notrun "nosuid mount option not supported" ;;Quoted 8 lines
common/rc _supports_filetype() · line 364
Quoted 6 lines
common/rc +6
-0+ *noatime*) _notrun "noatime mount option not supported" ;; + *relatime*) _notrun "relatime mount option not supported" ;; + *diratime*) _notrun "diratime mount option not supported" ;; + *strictatime*) _notrun "strictatime mount option not supported" ;; + esac +}Why specifically these mount options? Is this really generic?
The local file system type does not support mount, umount, or remount command. So there is no way to modulate noatime, relatime, etc. So this would be useful if someone wanted to test Windows System for Linux, for example.
Quoted 5 lines
Quoted 5 lines
common/rc +1
-0+ # mount scratch device with given options but don't check mount status _try_scratch_mount() {@@ -376,6 +388,9 @@ _scratch_unmount()Quoted 8 lines
common/rc _scratch_unmount() · line 393
Quoted 6 lines
common/rc +3
-0btrfs) $UMOUNT_PROG $SCRATCH_MNT ;; + local) + rm -rf $SCRATCH_MNT/* + ;;
_scratch_mkfsalready does that. Why does it make sense in_scratch_unmount?Just for cleanup purposes.
Quoted 4 lines
Quoted 4 lines
common/rc +0
-0@@ -386,6 +401,10 @@ _scratch_remount() { local opts="$1"Quoted 8 lines
common/rc _scratch_remount() · line 407
Quoted 4 lines
common/rc +4
-0+ if [ "$FSTYP" = "local" ]; then + _local_validate_mount_opt "$*" + return 0; + fiIt makes more sense to me to
_require_scratch_remountfor tests that need to remount in the beginning of tests - yeh that's a bit more work.There are some tests where the remount operation is just to reset the file system. So the test is just to unmount and remount the file system, and making sure the file status are the same. So I didn't want to block all remounts; instead, to let some remounts be no-op's so the rest of the test could still be used to provide test coverage, and only block those remounts which were modulating things like ro/rw, noatime, etc.
Quoted 10 lines
common/rc _scratch_cycle_mount() · line 418
Quoted 7 lines
common/rc +1
-1@@ -395,7 +414,7 @@ _scratch_cycle_mount() { local opts="$1" - if [ "$FSTYP" = tmpfs ]; then + if [ "$FSTYP" = "tmpfs" -o "$FSTYP" = "local" ] ; then _scratch_remount "$opts"That seems like cheating - seems better to implement and use
_require_scratch_cycle_mount
Quoted 4 lines
Again, I didn't want to end up skipping a huge number of tests. I didn't consider this "cheating"; I considered it a case of "let's maximize test coverage".
OK. I've used _scratch_cycle_mount as well in exportfs tests to make sure caches are evicted, so perhaps the "not cheating" edition of _scratch_cycle_mount for local should at least drop caches?
Thanks,
Amir.
Quoted 54 lines · Theodore Y. Ts'o
About a year and half ago, I sent the patch attached below to add support for a "local" file system type where the file system could not be mounted or dismounted, but where writes to
TEST_DIRandSCRATCH_MNTwould be testing the file system in question. At the time, I couldn't describe the use case in any greater detail than what I had in the commit description, and Dave Chinner at the time rejected the patch since he didn't like xfstests being used to test a proprietary file system for which I was not able to explain the details of what we were trying to do.Not a big deal, I just kept the patch in my private fork[1] of xfstests on github.
[1] https://github.com/tytso/xfstests
However, happily, we can now talk a lot more about what the "local" file system type in xfstests was used to test. Earlier today, Google announced gVisor[2], and published it as open source on github[3]. gVisor works much like User Mode Linux, and I suspect much like the Windows Subsystem for Linux in that it uses the x86_64's hardware virtualization extensions (so it has the security fencing much like a VM) but instead of emulating hardware, instead the emulation layer is done at the system call level (so it's more efficient than a VM, since there is no guest kernel).
[2] https://cloudplatform.googleblog.com/2018/05/Open-sourcing-gVisor-a-sandboxed-container-runtime.html
[3] https://github.com/google/gvisorFile systems can be implemented using Gophers[4] in a separate process, where the communication between the gVisor sandbox and the Gopher is via the 9P2000.L protocol. This means that if you want to try to exploit a buffer overflow in the userspace file system, first you have to get past the system call validation checks done by the gVisor Sentry process (which is written in Go which makes this rather more difficult, especially since the Sentry process itself is protected using seccomp[5]). Then the buffer overflow attack has to make it past the 9P protocol encoding/decoding, and then survive the 9P protocol validation checks in the Gopher. The Gopher process provides an emulated file system service using the Cloud Provider's internal cluster storage services, much like in GCE, the Persistent Disk service provides an emulated block device service.
[4] https://news.ycombinator.com/item?id=16979126
[5] https://news.ycombinator.com/item?id=16976557This whole system was designed with security first and foremost. Of course, we also want it to pass the file system checks, which is why were using xfstests.
The way we actually tested the gVisor file system was via a Docker container (the Dockerfile[6] is in the xfstests-bld git repo) which would then get consumed by gVisor's Docker/Kubernetes integration layer.
Quoted 8 lines
[6] https://github.com/tytso/xfstests-bld/blob/master/Dockerfile
Anyway, back in September, 2016, Dave Chinner was peeved that I couldn't give this full description, and I'm glad to say, now we can finally rectify that gap. :-)
Could this commit therefore please be considered for inclusion in xfstests upstream?
I still think it's useful, and thanks for resending with such detailed information! And I'm fine with the 'local' file system type. But I may need some time to do careful testing and go into the details. Just some random comments inline.
Quoted 11 lines
Many thanks!
- Ted
From
8b40b28866dca119d0c807c31ae48f153ec2dc53Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <[email protected]>
Date: Sat, 17 Sep 2016 19:08:18 -0400
Subject: [PATCH] common: add support for the "local" file system type
Quoted 8 lines
It is sometimes useful to be able to test the local file system provided in a restricted execution environment (such as that which is provided by Docker, for example) where it is not possible to mount and unmount the file system under test.
To support this test case, add support for a new file system type called "local". The
TEST_DEVandSCRATCH_DEVshould be have a non-block device format (e.g., local:/test or local:/scratch), and the
Does it require a non-block device format in this version of the patch? I don't think so, as we have the new 'local' FSTYP introduced now.
Quoted 6 lines
TEST_DIRandSCRATCH_MNTdirectories should be pre-existing directories provided by the execution environment.Signed-off-by Theodore Ts'o <[email protected]>--- common/rc | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
We really need some documentation added in README :)
Quoted 11 lines
1 file changed, 70 insertions(+), 3 deletions(-)common/rc +1
-0@@ -351,6 +351,18 @@ _supports_filetype() esac } +_local_validate_mount_opt()
common/rc _supports_filetype() · line 364
Quoted 10 lines
common/rc +10
-0+{ + case "$*" in + ro|ro,*|*,ro) _notrun "ro mount option not supported" ;; + *nosuid*) _notrun "nosuid mount option not supported" ;; + *noatime*) _notrun "noatime mount option not supported" ;; + *relatime*) _notrun "relatime mount option not supported" ;; + *diratime*) _notrun "diratime mount option not supported" ;; + *strictatime*) _notrun "strictatime mount option not supported" ;; + esac +}
Would be good to have some comments on why these mount options are not supported for 'local'.
common/rc _scratch_unmount() · line 393
Quoted 11 lines
common/rc +4
-0+ # mount scratch device with given options but don't check mount status _try_scratch_mount() {@@ -376,6 +388,9 @@ _scratch_unmount() btrfs) $UMOUNT_PROG $SCRATCH_MNT ;; + local) + rm -rf $SCRATCH_MNT/* + ;;
We do this in _scratch_mkfs by calling _scratch_cleanup_files. I noticed that you already did this in _scratch_mkfs, just return here for 'local'?
Quoted 24 lines
common/rc +5
-1*) $UMOUNT_PROG $SCRATCH_DEV ;;@@ -386,6 +401,10 @@ _scratch_remount() { local opts="$1" + if [ "$FSTYP" = "local" ]; then + _local_validate_mount_opt "$*" + return 0; + fi if test -n "$opts"; then mount -o "remount,$opts" $SCRATCH_MNT fi@@ -395,7 +414,7 @@ _scratch_cycle_mount() { local opts="$1" - if [ "$FSTYP" = tmpfs ]; then + if [ "$FSTYP" = "tmpfs" -o "$FSTYP" = "local" ] ; then _scratch_remount "$opts" return fi
common/rc _test_mount() · line 454
Quoted 7 lines
common/rc +4
-0@@ -429,6 +448,10 @@ _test_mount() _overlay_test_mount $* return $? fi + if [ "$FSTYP" == "local" ]; then + mkdir -p $TEST_DIR + return $? + fi
$TEST_DIR is guaranteed to be there (in common/config at initialization time and in _require_test ). Perhaps we can just return in the 'local' case?
Quoted 47 lines
common/rc +13
-2_test_options mount _mount -t $FSTYP $TEST_OPTIONS $TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS $* $TEST_DEV $TEST_DIR }@@ -437,7 +460,7 @@ _test_unmount() { if [ "$FSTYP" == "overlay" ]; then _overlay_test_unmount - else + elif [ "$FSTYP" != "local" ]; then $UMOUNT_PROG $TEST_DEV fi }@@ -723,7 +746,7 @@ _scratch_mkfs() local mkfs_status case $FSTYP in - nfs*|cifs|ceph|overlay|glusterfs|pvfs2|9p) + nfs*|cifs|ceph|overlay|glusterfs|pvfs2|9p|local) # unable to re-create this fstyp, just remove all files in # $SCRATCH_MNT to avoid EEXIST caused by the leftover files # created in previous runs@@ -1465,6 +1488,10 @@ _check_mounted_on() local mnt=$4 local type=$5 + if [ "$FSTYP" == "local" ]; then + return 0 + fi + # find $dev as the source, and print result in "$dev $mnt" format local mount_rec=`findmnt -rncv -S $dev -o SOURCE,TARGET` [ -n "$mount_rec" ] || return 1 # 1 = not mounted@@ -1562,6 +1589,13 @@ _require_scratch_nocheck() _notrun "this test requires a valid \$SCRATCH_MNT" fi ;; + local) + if [ -z "$SCRATCH_DEV" -o ! -d "$SCRATCH_MNT" ]; + then + _notrun "this test requires a valid \$SCRATCH_MNT and unique $SCRATCH_DEV" + fi + return 0 + ;; *) if [ -z "$SCRATCH_DEV" -o "`_is_block_dev "$SCRATCH_DEV"`" = "" ] then
common/rc _require_test() · line 1726
Quoted 10 lines
common/rc +7
-0@@ -1683,6 +1717,13 @@ _require_test() _notrun "this test requires a valid \$TEST_DIR" fi ;; + local) + if [ -z "$TEST_DEV" -o ! -d "$TEST_DIR" ]; + then + _notrun "this test requires a valid \$TEST_DIR and unique $TEST_DEV" + fi + return 0 + ;;
It seems $SCRATCH_DEV and $TEST_DEV are not important for 'local' type, as long as we have SCRATCH_MNT and/or TEST_DIR defined as directories.
Thanks,
Eryu
Quoted 92 lines
common/rc +26
-0*) if [ -z "$TEST_DEV" ] || [ "`_is_block_dev "$TEST_DEV"`" = "" ] then@@ -2438,6 +2479,10 @@ _remount() local device=$1 local mode=$2 + if [ "$FSTYP" == "local" ] ; then + return 0 + fi + if ! mount -o remount,$mode $device then echo "_remount: failed to remount filesystem on $device as $mode"@@ -2483,6 +2528,10 @@ _mount_or_remount_rw() local device=$2 local mountpoint=$3 + if [ "$FSTYP" == "local" ] ; then + return 0 + fi + if [ $USE_REMOUNT -eq 0 ]; then if [ "$FSTYP" != "overlay" ]; then _mount -t $FSTYP $mount_opts $device $mountpoint@@ -2636,6 +2685,9 @@ _check_test_fs() ubifs) # there is no fsck program for ubifs yet ;; + local) + # no way to check consistency for local + ;; *) _check_generic_filesystem $TEST_DEV ;;@@ -2691,6 +2743,9 @@ _check_scratch_fs() ubifs) # there is no fsck program for ubifs yet ;; + local) + # no way to check consistency for local + ;; *) _check_generic_filesystem $device ;;@@ -3003,6 +3058,9 @@ _require_fio() # Does freeze work on this fs? _require_freeze() { + if [ "$FSTYP" == "local" ]; then + _notrun "local does not support freeze" + fi xfs_freeze -f "$TEST_DIR" >/dev/null 2>&1 local result=$? xfs_freeze -u "$TEST_DIR" >/dev/null 2>&1@@ -3024,6 +3082,9 @@ _require_scratch_shutdown() { [ -x src/godown ] || _notrun "src/godown executable not found" + if [ "$FSTYP" == "local" ]; then + _notrun "local does not support shutdown" + fi _scratch_mkfs > /dev/null 2>&1 || _notrun "_scratch_mkfs failed on $SCRATCH_DEV" _scratch_mount@@ -3049,6 +3110,9 @@ _require_scratch_shutdown() # Does dax mount option work on this dev/fs? _require_scratch_dax() { + if [ "$FSTYP" == "local" ]; then + _notrun "local does not support dax" + fi _require_scratch _scratch_mkfs > /dev/null 2>&1 _try_scratch_mount -o dax || \@@ -3063,6 +3127,9 @@ _require_scratch_dax() # Does norecovery support by this fs? _require_norecovery() { + if [ "$FSTYP" == "local" ]; then + _notrun "local does not support norecovery" + fi _try_scratch_mount -o ro,norecovery || \ _notrun "$FSTYP does not support norecovery" _scratch_unmountSignature · 6 lines
2.16.1.72.g5be1f00a9a -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ted,
On Wed, May 02, 2018 at 11:43:40PM -0400, Theodore Y. Ts'o wrote:
[snip the background details]
Quoted 10 lines
From
8b40b28866dca119d0c807c31ae48f153ec2dc53Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <[email protected]>
Date: Sat, 17 Sep 2016 19:08:18 -0400
Subject: [PATCH] common: add support for the "local" file system typeIt is sometimes useful to be able to test the local file system provided in a restricted execution environment (such as that which is provided by Docker, for example) where it is not possible to mount and unmount the file system under test.
Quoted 7 lines
To support this test case, add support for a new file system type called "local". The
TEST_DEVandSCRATCH_DEVshould be have a non-block device format (e.g., local:/test or local:/scratch), and theTEST_DIRandSCRATCH_MNTdirectories should be pre-existing directories provided by the execution environment.Signed-off-by Theodore Ts'o <[email protected]>
I tested the patch with XFS and ext4 as the mounted underlying filesystem for multiple rounds and found some more issues (in addition to previous review comments) that need to be addressed. I'll reply inline.
Quoted 3 lines
--- common/rc | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 70 insertions(+), 3 deletions(-)
I think we need a new "-local" option in './check' to do some initial setups, e.g. FSTYP=local, as other fs types that can't be retrieved by running blkid on $TEST_DEV.
Also need some documentations in README to describe "local"'s use case and mention that TEST_DEV and SCRATCH_DEV should be have a non-block device format.
Quoted 14 lines
common/rc +5
-0@@ -351,6 +351,18 @@ _supports_filetype() esac } +_local_validate_mount_opt() +{ + case "$*" in + ro|ro,*|*,ro) _notrun "ro mount option not supported" ;; + *nosuid*) _notrun "nosuid mount option not supported" ;;
common/rc _supports_filetype() · line 368
Quoted 10 lines
common/rc +7
-0+ *noatime*) _notrun "noatime mount option not supported" ;; + *relatime*) _notrun "relatime mount option not supported" ;; + *diratime*) _notrun "diratime mount option not supported" ;; + *strictatime*) _notrun "strictatime mount option not supported" ;; + esac +} + # mount scratch device with given options but don't check mount status _try_scratch_mount() {
We need to add a check for "local" in _try_scratch_mount() to let it do nothing for "local" too, otherwise 'FSTYP=local ./check' won't run for me, as _scratch_mount() failed and exit the test in 'check' line 629.
(Note that the exit on _scratch_mount() failure behavior was introduced by commit 69eb6281a9d3 ("fstests: _fail test by default when _scratch_mount fails"))
common/rc _scratch_unmount() · line 393
Quoted 7 lines
common/rc +3
-0@@ -376,6 +388,9 @@ _scratch_unmount() btrfs) $UMOUNT_PROG $SCRATCH_MNT ;; + local) + rm -rf $SCRATCH_MNT/* + ;;
As noted in previous review, we could just return for 'local' in _scratch_unmount(), _scratch_mkfs() already removed all the files there.
common/rc _scratch_remount() · line 407
Quoted 11 lines
common/rc +4
-0*) $UMOUNT_PROG $SCRATCH_DEV ;;@@ -386,6 +401,10 @@ _scratch_remount() { local opts="$1" + if [ "$FSTYP" = "local" ]; then + _local_validate_mount_opt "$*" + return 0; + fi
I found that there're more places that need to do _local_validate_mount_opt, e.g. in _try_scratch_mount() and _test_mount, otherwise tests that use _try_scratch_mount to do the remount would fail, like generic/294.
Quoted 13 lines
common/rc +1
-1if test -n "$opts"; then mount -o "remount,$opts" $SCRATCH_MNT fi@@ -395,7 +414,7 @@ _scratch_cycle_mount() { local opts="$1" - if [ "$FSTYP" = tmpfs ]; then + if [ "$FSTYP" = "tmpfs" -o "$FSTYP" = "local" ] ; then _scratch_remount "$opts" return fi
common/rc _test_mount() · line 452
Quoted 5 lines
common/rc +2
-0@@ -429,6 +448,10 @@ _test_mount() _overlay_test_mount $* return $? fi + if [ "$FSTYP" == "local" ]; then + mkdir -p $TEST_DIR
Probably need _local_validate_mount_opt here as mentioned above.
And there're some tests or functions that call umount on $SCRATCH_MNT directly, which would cause the underlying filesystem to be umounted if it can be umounted (thouth this is not an intended use case for "local", I think it's better to fix them too). e.g.
generic/034 generic/330 and generic/332 need to use _scratch_unmount instead of a bare umount.
All the "$UMOUNT_PROG $SCRATCH_MNT"s in dm device helper functions, we need to change them to umount the actual device, e.g. $FLAKEY_DEV
generic/108 needs to umount $SCSI_DEBUG_DEV in _cleanup()
Similarly, generic/459 needs to umount the lvm snapshot device /dev/mapper/$vgname-$snapname
Thanks,
Eryu
Quoted 151 lines
common/rc +48
-2+ return $? + fi _test_options mount _mount -t $FSTYP $TEST_OPTIONS $TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS $* $TEST_DEV $TEST_DIR }@@ -437,7 +460,7 @@ _test_unmount() { if [ "$FSTYP" == "overlay" ]; then _overlay_test_unmount - else + elif [ "$FSTYP" != "local" ]; then $UMOUNT_PROG $TEST_DEV fi }@@ -723,7 +746,7 @@ _scratch_mkfs() local mkfs_status case $FSTYP in - nfs*|cifs|ceph|overlay|glusterfs|pvfs2|9p) + nfs*|cifs|ceph|overlay|glusterfs|pvfs2|9p|local) # unable to re-create this fstyp, just remove all files in # $SCRATCH_MNT to avoid EEXIST caused by the leftover files # created in previous runs@@ -1465,6 +1488,10 @@ _check_mounted_on() local mnt=$4 local type=$5 + if [ "$FSTYP" == "local" ]; then + return 0 + fi + # find $dev as the source, and print result in "$dev $mnt" format local mount_rec=`findmnt -rncv -S $dev -o SOURCE,TARGET` [ -n "$mount_rec" ] || return 1 # 1 = not mounted@@ -1562,6 +1589,13 @@ _require_scratch_nocheck() _notrun "this test requires a valid \$SCRATCH_MNT" fi ;; + local) + if [ -z "$SCRATCH_DEV" -o ! -d "$SCRATCH_MNT" ]; + then + _notrun "this test requires a valid \$SCRATCH_MNT and unique $SCRATCH_DEV" + fi + return 0 + ;; *) if [ -z "$SCRATCH_DEV" -o "`_is_block_dev "$SCRATCH_DEV"`" = "" ] then@@ -1683,6 +1717,13 @@ _require_test() _notrun "this test requires a valid \$TEST_DIR" fi ;; + local) + if [ -z "$TEST_DEV" -o ! -d "$TEST_DIR" ]; + then + _notrun "this test requires a valid \$TEST_DIR and unique $TEST_DEV" + fi + return 0 + ;; *) if [ -z "$TEST_DEV" ] || [ "`_is_block_dev "$TEST_DEV"`" = "" ] then@@ -2438,6 +2479,10 @@ _remount() local device=$1 local mode=$2 + if [ "$FSTYP" == "local" ] ; then + return 0 + fi + if ! mount -o remount,$mode $device then echo "_remount: failed to remount filesystem on $device as $mode"@@ -2483,6 +2528,10 @@ _mount_or_remount_rw() local device=$2 local mountpoint=$3 + if [ "$FSTYP" == "local" ] ; then + return 0 + fi + if [ $USE_REMOUNT -eq 0 ]; then if [ "$FSTYP" != "overlay" ]; then _mount -t $FSTYP $mount_opts $device $mountpoint@@ -2636,6 +2685,9 @@ _check_test_fs() ubifs) # there is no fsck program for ubifs yet ;; + local) + # no way to check consistency for local + ;; *) _check_generic_filesystem $TEST_DEV ;;@@ -2691,6 +2743,9 @@ _check_scratch_fs() ubifs) # there is no fsck program for ubifs yet ;; + local) + # no way to check consistency for local + ;; *) _check_generic_filesystem $device ;;@@ -3003,6 +3058,9 @@ _require_fio() # Does freeze work on this fs? _require_freeze() { + if [ "$FSTYP" == "local" ]; then + _notrun "local does not support freeze" + fi xfs_freeze -f "$TEST_DIR" >/dev/null 2>&1 local result=$? xfs_freeze -u "$TEST_DIR" >/dev/null 2>&1@@ -3024,6 +3082,9 @@ _require_scratch_shutdown() { [ -x src/godown ] || _notrun "src/godown executable not found" + if [ "$FSTYP" == "local" ]; then + _notrun "local does not support shutdown" + fi _scratch_mkfs > /dev/null 2>&1 || _notrun "_scratch_mkfs failed on $SCRATCH_DEV" _scratch_mount@@ -3049,6 +3110,9 @@ _require_scratch_shutdown() # Does dax mount option work on this dev/fs? _require_scratch_dax() { + if [ "$FSTYP" == "local" ]; then + _notrun "local does not support dax" + fi _require_scratch _scratch_mkfs > /dev/null 2>&1 _try_scratch_mount -o dax || \@@ -3063,6 +3127,9 @@ _require_scratch_dax() # Does norecovery support by this fs? _require_norecovery() { + if [ "$FSTYP" == "local" ]; then + _notrun "local does not support norecovery" + fi _try_scratch_mount -o ro,norecovery || \ _notrun "$FSTYP does not support norecovery" _scratch_unmountSignature · 6 lines
2.16.1.72.g5be1f00a9a -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html