Message ID | 20171212171154.5222-1-tuomas@tuxera.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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 -- 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
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. -- 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
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. -- 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
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 -- 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
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 ;;
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(-)