[PATCH] fstests: Add 9p network filesystem support

[PATCH] fstests: Add 9p network filesystem support

[Tuomas Tynkkynen]

This commit adds support for the 9p network file system, which is mainly
used by QEMU for sharing a file system from the host to the guest VM.

To run xfstests on it, launch QEMU with e.g.:

-virtfs local,path=$TMPDIR/p9-test,security_model=mapped-xattr,mount_tag=p9-test
-virtfs local,path=$TMPDIR/p9-scratch,security_model=mapped-xattr,mount_tag=p9-scratch

and inside the VM run xfstests with:

export TEST_DEV=p9-test
export SCRATCH_DEV=p9-scratch
export MOUNT_OPTIONS="-o trans=virtio,version=9p2000.L,cache=loose,posixacl"
export TEST_FS_MOUNT_OPTS="$MOUNT_OPTIONS"

Signed-off-by: Tuomas Tynkkynen <tuomas@tuxera.com>
---
 check         |  2 ++
 common/attr   |  4 ++--
 common/config |  3 +++
 common/rc     | 32 +++++++++++++++++++++++++++++++-
 4 files changed, 38 insertions(+), 3 deletions(-)

diff --git check check
index f8db3cd6..6078b1ef 100755
--- check
+++ check
@@ -67,6 +67,7 @@ check options
     -nfs                test NFS
     -glusterfs                test GlusterFS
     -cifs               test CIFS
+    -9p			test 9p
     -overlay		test overlay
     -pvfs2          test PVFS2
     -tmpfs              test TMPFS
@@ -265,6 +266,7 @@ while [ $# -gt 0 ]; do
 	-nfs)		FSTYP=nfs ;;
 	-glusterfs)	FSTYP=glusterfs ;;
 	-cifs)		FSTYP=cifs ;;
+	-9p)		FSTYP=9p ;;
 	-overlay)	FSTYP=overlay; export OVERLAY=true ;;
 	-pvfs2)		FSTYP=pvfs2 ;;
 	-tmpfs)		FSTYP=tmpfs ;;
diff --git common/attr common/attr
index 8a5c9eac..66b0227f 100644
--- common/attr
+++ common/attr
@@ -243,7 +243,7 @@ _sort_getfattr_output()
 
 # set maximum total attr space based on fs type
 case "$FSTYP" in
-xfs|udf|pvfs2|ceph)
+xfs|udf|pvfs2|9p|ceph)
 	MAX_ATTRS=1000
 	;;
 *)
@@ -263,7 +263,7 @@ xfs|udf|btrfs)
 pvfs2)
 	MAX_ATTRVAL_SIZE=8192
 	;;
-ceph)
+9p|ceph)
 	MAX_ATTRVAL_SIZE=65536
 	;;
 *)
diff --git common/config common/config
index d0fbfe55..5e4a73ed 100644
--- common/config
+++ common/config
@@ -459,6 +459,9 @@ _check_device()
 	fi
 
 	case "$FSTYP" in
+	9p)
+		# 9p mount tags are just plain strings, so anything is allowed
+		;;
 	overlay)
 		if [ ! -d "$dev" ]; then
 			_fatal "common/config: $name ($dev) is not a directory for overlay"
diff --git common/rc common/rc
index 4c053a53..0d2da68c 100644
--- common/rc
+++ common/rc
@@ -166,6 +166,8 @@ case "$FSTYP" in
 	 ;;
     cifs)
 	 ;;
+    9p)
+	 ;;
     ceph)
 	 ;;
     glusterfs)
@@ -578,6 +580,9 @@ _test_mkfs()
     cifs)
 	# do nothing for cifs
 	;;
+    9p)
+	# do nothing for 9p
+	;;
     ceph)
 	# do nothing for ceph
 	;;
@@ -612,6 +617,9 @@ _mkfs_dev()
     nfs*)
 	# do nothing for nfs
 	;;
+    9p)
+	# do nothing for 9p
+	;;
     overlay)
 	# do nothing for overlay
 	;;
@@ -676,7 +684,7 @@ _scratch_mkfs()
 	local mkfs_status
 
 	case $FSTYP in
-	nfs*|cifs|ceph|overlay|glusterfs|pvfs2)
+	nfs*|cifs|ceph|overlay|glusterfs|pvfs2|9p)
 		# 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
@@ -1448,6 +1456,14 @@ _require_scratch_nocheck()
 			_notrun "this test requires a valid \$SCRATCH_MNT"
 		fi
 		;;
+	9p)
+		if [ -z "$SCRATCH_DEV" ]; then
+			_notrun "this test requires a valid \$SCRATCH_DEV"
+		fi
+		if [ ! -d "$SCRATCH_MNT" ]; then
+			_notrun "this test requires a valid \$SCRATCH_MNT"
+		fi
+		;;
 	nfs*|ceph)
 		echo $SCRATCH_DEV | grep -q ":/" > /dev/null 2>&1
 		if [ -z "$SCRATCH_DEV" -o "$?" != "0" ]; then
@@ -1554,6 +1570,14 @@ _require_test()
 			_notrun "this test requires a valid \$TEST_DIR"
 		fi
 		;;
+	9p)
+		if [ -z "$TEST_DEV" ]; then
+			_notrun "this test requires a valid \$TEST_DEV"
+		fi
+		if [ ! -d "$TEST_DIR" ]; then
+			_notrun "this test requires a valid \$TEST_DIR"
+		fi
+		;;
 	nfs*|ceph)
 		echo $TEST_DEV | grep -q ":/" > /dev/null 2>&1
 		if [ -z "$TEST_DEV" -o "$?" != "0" ]; then
@@ -2500,6 +2524,9 @@ _check_test_fs()
     cifs)
 	# no way to check consistency for cifs
 	;;
+    9p)
+	# no way to check consistency for 9p
+	;;
     ceph)
 	# no way to check consistency for CephFS
 	;;
@@ -2555,6 +2582,9 @@ _check_scratch_fs()
     cifs)
 	# Don't know how to check a CIFS filesystem, yet.
 	;;
+    9p)
+	# no way to check consistency for 9p
+	;;
     ceph)
 	# no way to check consistency for CephFS
 	;;
-- 
2.15.0

Re: [PATCH] fstests: Add 9p network filesystem support

[Eryu Guan]

On Tue, Dec 12, 2017 at 07:11:54PM +0200, Tuomas Tynkkynen wrote:
> This commit adds support for the 9p network file system, which is mainly
> used by QEMU for sharing a file system from the host to the guest VM.
> 
> To run xfstests on it, launch QEMU with e.g.:
> 
> -virtfs local,path=$TMPDIR/p9-test,security_model=mapped-xattr,mount_tag=p9-test
> -virtfs local,path=$TMPDIR/p9-scratch,security_model=mapped-xattr,mount_tag=p9-scratch
> 
> and inside the VM run xfstests with:
> 
> export TEST_DEV=p9-test
> export SCRATCH_DEV=p9-scratch
> export MOUNT_OPTIONS="-o trans=virtio,version=9p2000.L,cache=loose,posixacl"
> export TEST_FS_MOUNT_OPTS="$MOUNT_OPTIONS"

We can take 9P_MOUNT_OPTIONS as the default value for both MOUNT_OPTIONS
and TEST_FS_MOUNT_OPTS in common/config, similar to CIFS_MOUNT_OPTIONS
etc.

> 
> Signed-off-by: Tuomas Tynkkynen <tuomas@tuxera.com>

This looks fine to me overall. I did a 'quick' group test and tests ran
fine, though seems that some tests failed and they and/or some of the
common helpers may need further tweaks. I think follow-up patches would
be fine.

> ---
>  check         |  2 ++
>  common/attr   |  4 ++--
>  common/config |  3 +++
>  common/rc     | 32 +++++++++++++++++++++++++++++++-
>  4 files changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git check check
> index f8db3cd6..6078b1ef 100755
> --- check
> +++ check

This patch doesn't apply by default, I have to edit it manually to make
'git am' work, e.g.

--- a/check
+++ b/check

I think 'git format-patch' would generate the correct patch format.

Thanks,
Eryu

> @@ -67,6 +67,7 @@ check options
>      -nfs                test NFS
>      -glusterfs                test GlusterFS
>      -cifs               test CIFS
> +    -9p			test 9p
>      -overlay		test overlay
>      -pvfs2          test PVFS2
>      -tmpfs              test TMPFS
> @@ -265,6 +266,7 @@ while [ $# -gt 0 ]; do
>  	-nfs)		FSTYP=nfs ;;
>  	-glusterfs)	FSTYP=glusterfs ;;
>  	-cifs)		FSTYP=cifs ;;
> +	-9p)		FSTYP=9p ;;
>  	-overlay)	FSTYP=overlay; export OVERLAY=true ;;
>  	-pvfs2)		FSTYP=pvfs2 ;;
>  	-tmpfs)		FSTYP=tmpfs ;;
> diff --git common/attr common/attr
> index 8a5c9eac..66b0227f 100644
> --- common/attr
> +++ common/attr
> @@ -243,7 +243,7 @@ _sort_getfattr_output()
>  
>  # set maximum total attr space based on fs type
>  case "$FSTYP" in
> -xfs|udf|pvfs2|ceph)
> +xfs|udf|pvfs2|9p|ceph)
>  	MAX_ATTRS=1000
>  	;;
>  *)
> @@ -263,7 +263,7 @@ xfs|udf|btrfs)
>  pvfs2)
>  	MAX_ATTRVAL_SIZE=8192
>  	;;
> -ceph)
> +9p|ceph)
>  	MAX_ATTRVAL_SIZE=65536
>  	;;
>  *)
> diff --git common/config common/config
> index d0fbfe55..5e4a73ed 100644
> --- common/config
> +++ common/config
> @@ -459,6 +459,9 @@ _check_device()
>  	fi
>  
>  	case "$FSTYP" in
> +	9p)
> +		# 9p mount tags are just plain strings, so anything is allowed
> +		;;
>  	overlay)
>  		if [ ! -d "$dev" ]; then
>  			_fatal "common/config: $name ($dev) is not a directory for overlay"
> diff --git common/rc common/rc
> index 4c053a53..0d2da68c 100644
> --- common/rc
> +++ common/rc
> @@ -166,6 +166,8 @@ case "$FSTYP" in
>  	 ;;
>      cifs)
>  	 ;;
> +    9p)
> +	 ;;
>      ceph)
>  	 ;;
>      glusterfs)
> @@ -578,6 +580,9 @@ _test_mkfs()
>      cifs)
>  	# do nothing for cifs
>  	;;
> +    9p)
> +	# do nothing for 9p
> +	;;
>      ceph)
>  	# do nothing for ceph
>  	;;
> @@ -612,6 +617,9 @@ _mkfs_dev()
>      nfs*)
>  	# do nothing for nfs
>  	;;
> +    9p)
> +	# do nothing for 9p
> +	;;
>      overlay)
>  	# do nothing for overlay
>  	;;
> @@ -676,7 +684,7 @@ _scratch_mkfs()
>  	local mkfs_status
>  
>  	case $FSTYP in
> -	nfs*|cifs|ceph|overlay|glusterfs|pvfs2)
> +	nfs*|cifs|ceph|overlay|glusterfs|pvfs2|9p)
>  		# 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
> @@ -1448,6 +1456,14 @@ _require_scratch_nocheck()
>  			_notrun "this test requires a valid \$SCRATCH_MNT"
>  		fi
>  		;;
> +	9p)
> +		if [ -z "$SCRATCH_DEV" ]; then
> +			_notrun "this test requires a valid \$SCRATCH_DEV"
> +		fi
> +		if [ ! -d "$SCRATCH_MNT" ]; then
> +			_notrun "this test requires a valid \$SCRATCH_MNT"
> +		fi
> +		;;
>  	nfs*|ceph)
>  		echo $SCRATCH_DEV | grep -q ":/" > /dev/null 2>&1
>  		if [ -z "$SCRATCH_DEV" -o "$?" != "0" ]; then
> @@ -1554,6 +1570,14 @@ _require_test()
>  			_notrun "this test requires a valid \$TEST_DIR"
>  		fi
>  		;;
> +	9p)
> +		if [ -z "$TEST_DEV" ]; then
> +			_notrun "this test requires a valid \$TEST_DEV"
> +		fi
> +		if [ ! -d "$TEST_DIR" ]; then
> +			_notrun "this test requires a valid \$TEST_DIR"
> +		fi
> +		;;
>  	nfs*|ceph)
>  		echo $TEST_DEV | grep -q ":/" > /dev/null 2>&1
>  		if [ -z "$TEST_DEV" -o "$?" != "0" ]; then
> @@ -2500,6 +2524,9 @@ _check_test_fs()
>      cifs)
>  	# no way to check consistency for cifs
>  	;;
> +    9p)
> +	# no way to check consistency for 9p
> +	;;
>      ceph)
>  	# no way to check consistency for CephFS
>  	;;
> @@ -2555,6 +2582,9 @@ _check_scratch_fs()
>      cifs)
>  	# Don't know how to check a CIFS filesystem, yet.
>  	;;
> +    9p)
> +	# no way to check consistency for 9p
> +	;;
>      ceph)
>  	# no way to check consistency for CephFS
>  	;;
> -- 
> 2.15.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] fstests: Add 9p network filesystem support

[Tuomas Tynkkynen]

Hi Eryu,

On Mon, 2017-12-18 at 22:12 +0800, Eryu Guan wrote:
> On Tue, Dec 12, 2017 at 07:11:54PM +0200, Tuomas Tynkkynen wrote:
> > This commit adds support for the 9p network file system, which is
> > mainly
> > used by QEMU for sharing a file system from the host to the guest
> > VM.
> > 
> > To run xfstests on it, launch QEMU with e.g.:
> > 
> > -virtfs local,path=$TMPDIR/p9-test,security_model=mapped-
> > xattr,mount_tag=p9-test
> > -virtfs local,path=$TMPDIR/p9-scratch,security_model=mapped-
> > xattr,mount_tag=p9-scratch
> > 
> > and inside the VM run xfstests with:
> > 
> > export TEST_DEV=p9-test
> > export SCRATCH_DEV=p9-scratch
> > export MOUNT_OPTIONS="-o
> > trans=virtio,version=9p2000.L,cache=loose,posixacl"
> > export TEST_FS_MOUNT_OPTS="$MOUNT_OPTIONS"
> 
> We can take 9P_MOUNT_OPTIONS as the default value for both
> MOUNT_OPTIONS
> and TEST_FS_MOUNT_OPTS in common/config, similar to
> CIFS_MOUNT_OPTIONS
> etc.
> 

Ok, will add in next version.

> > 
> > Signed-off-by: Tuomas Tynkkynen <tuomas@tuxera.com>
> 
> This looks fine to me overall. I did a 'quick' group test and tests
> ran
> fine, though seems that some tests failed and they and/or some of the
> common helpers may need further tweaks. I think follow-up patches
> would
> be fine.
> 

I think most of the failures are legitimate bugs in either the kernel
driver, the protocol itself or QEMU. Many of those are quite difficult
to fix in userspace-based file servers; in particular those that deal
with timestamps. E.g. generic/120 which tests that atime of a file
doesn't change on exec...

Here are some of my notes for some failing cases:

generic/037: QEMU bug CVE-2017-15038. The CVE bugfix just fixes the
infoleak, but the correctness problem with getattr() remains.
generic/062, generic/097: The 9p protocol doesn't have a way to set
empty xattrs, such a protocol message is treated as xattr deletion.
generic/003 generic/120 generic/192 generic/221 generic/307 \
    generic/313 generic/423: timestamp problems
generic/403: Can't set trusted.* xattr from an userspace file server

One additional problem I know which doesn't get exercised by any
xfstest (AFAICT) is that open("foo", O_CREAT|O_WRONLY, 0444) fails with
-EPERM, at least under some mount options (related to writeback
caching).

> > ---
> >  check         |  2 ++
> >  common/attr   |  4 ++--
> >  common/config |  3 +++
> >  common/rc     | 32 +++++++++++++++++++++++++++++++-
> >  4 files changed, 38 insertions(+), 3 deletions(-)
> > 
> > diff --git check check
> > index f8db3cd6..6078b1ef 100755
> > --- check
> > +++ check
> 
> This patch doesn't apply by default, I have to edit it manually to
> make
> 'git am' work, e.g.
> 
> --- a/check
> +++ b/check
> 
> I think 'git format-patch' would generate the correct patch format.
> 
> 

Yeah, sorry, I forgot to disable the diff.noprefix option in git before
running format-patch which leads to this. Will fix for the next
submission.

Re: [PATCH] fstests: Add 9p network filesystem support

[Tuomas Tynkkynen]

On Mon, 2017-12-18 at 16:58 +0200, Tuomas Tynkkynen wrote:
> Hi Eryu,
> 
> On Mon, 2017-12-18 at 22:12 +0800, Eryu Guan wrote:
> > On Tue, Dec 12, 2017 at 07:11:54PM +0200, Tuomas Tynkkynen wrote:
> > > This commit adds support for the 9p network file system, which is
> > > mainly
> > > used by QEMU for sharing a file system from the host to the guest
> > > VM.
> > > 
> > > To run xfstests on it, launch QEMU with e.g.:
> > > 
> > > -virtfs local,path=$TMPDIR/p9-test,security_model=mapped-
> > > xattr,mount_tag=p9-test
> > > -virtfs local,path=$TMPDIR/p9-scratch,security_model=mapped-
> > > xattr,mount_tag=p9-scratch
> > > 
> > > and inside the VM run xfstests with:
> > > 
> > > export TEST_DEV=p9-test
> > > export SCRATCH_DEV=p9-scratch
> > > export MOUNT_OPTIONS="-o
> > > trans=virtio,version=9p2000.L,cache=loose,posixacl"
> > > export TEST_FS_MOUNT_OPTS="$MOUNT_OPTIONS"
> > 
> > We can take 9P_MOUNT_OPTIONS as the default value for both
> > MOUNT_OPTIONS
> > and TEST_FS_MOUNT_OPTS in common/config, similar to
> > CIFS_MOUNT_OPTIONS
> > etc.
> > 
> 
> Ok, will add in next version.
> 

Oops; no. Bash doesn't like environment variables starting with
numbers. P9_MOUNT_OPTIONS maybe then? Though honestly, I've never used
nor really understood the reason for these foo_MOUNT_OPTIONS... same
for the -nfs etc. command line flags.

Re: [PATCH] fstests: Add 9p network filesystem support

[Eryu Guan]

On Thu, Dec 21, 2017 at 05:13:10AM +0200, Tuomas Tynkkynen wrote:
> On Mon, 2017-12-18 at 16:58 +0200, Tuomas Tynkkynen wrote:
> > Hi Eryu,
> > 
> > On Mon, 2017-12-18 at 22:12 +0800, Eryu Guan wrote:
> > > On Tue, Dec 12, 2017 at 07:11:54PM +0200, Tuomas Tynkkynen wrote:
> > > > This commit adds support for the 9p network file system, which is
> > > > mainly
> > > > used by QEMU for sharing a file system from the host to the guest
> > > > VM.
> > > > 
> > > > To run xfstests on it, launch QEMU with e.g.:
> > > > 
> > > > -virtfs local,path=$TMPDIR/p9-test,security_model=mapped-
> > > > xattr,mount_tag=p9-test
> > > > -virtfs local,path=$TMPDIR/p9-scratch,security_model=mapped-
> > > > xattr,mount_tag=p9-scratch
> > > > 
> > > > and inside the VM run xfstests with:
> > > > 
> > > > export TEST_DEV=p9-test
> > > > export SCRATCH_DEV=p9-scratch
> > > > export MOUNT_OPTIONS="-o
> > > > trans=virtio,version=9p2000.L,cache=loose,posixacl"
> > > > export TEST_FS_MOUNT_OPTS="$MOUNT_OPTIONS"
> > > 
> > > We can take 9P_MOUNT_OPTIONS as the default value for both
> > > MOUNT_OPTIONS
> > > and TEST_FS_MOUNT_OPTS in common/config, similar to
> > > CIFS_MOUNT_OPTIONS
> > > etc.
> > > 
> > 
> > Ok, will add in next version.
> > 
> 
> Oops; no. Bash doesn't like environment variables starting with

Ah, that's right.

> numbers. P9_MOUNT_OPTIONS maybe then? Though honestly, I've never used

Or PLAN9_MOUNT_OPTIONS?

> nor really understood the reason for these foo_MOUNT_OPTIONS... same
> for the -nfs etc. command line flags.

Make update fs-specific mount options in local.config file more easily
when switching filesysm type to test? You don't have to update
MOUNT_OPTIONS and TEST_FS_MOUNT_OPTS.

The '-nfs'/'-9p' command line flags are used to indicate which
filesystem it's testing and set FSTYP variable correctly. Because these
filesystems can't be detected from $TEST_DEV automatically like XFS,
ext4 etc.

Thanks,
Eryu