* [PATCH 1/2] generic/430: Fix filename in "copy beyond end" test @ 2017-06-26 16:40 Luis Henriques 2017-06-26 16:40 ` [PATCH 2/2] common/rc: handle xfs_io copy_range when copy_file_range syscall isn't available Luis Henriques 2017-06-28 14:21 ` [PATCH 1/2] generic/430: Fix filename in "copy beyond end" test David Disseldorp 0 siblings, 2 replies; 7+ messages in thread From: Luis Henriques @ 2017-06-26 16:40 UTC (permalink / raw) To: fstests; +Cc: Anna Schumaker, Luis Henriques The cmp command was using the wrong file to perform the comparison. Use $testdir/beyond instead of $testdir/end. Signed-off-by: Luis Henriques <lhenriques@suse.com> --- tests/generic/430 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/generic/430 b/tests/generic/430 index 2ec126db9847..5d6b8d6c2aa3 100755 --- a/tests/generic/430 +++ b/tests/generic/430 @@ -87,7 +87,7 @@ md5sum $testdir/{file,end} | _filter_test_dir echo "Copy beyond end of original file" $XFS_IO_PROG -f -c "copy_range -s 4000 -l 2000 $testdir/file" "$testdir/beyond" -cmp -n 1000 $testdir/file $testdir/end 4000 +cmp -n 1000 $testdir/file $testdir/beyond 4000 echo "md5sums after copying beyond:" md5sum $testdir/{file,beyond} | _filter_test_dir ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] common/rc: handle xfs_io copy_range when copy_file_range syscall isn't available 2017-06-26 16:40 [PATCH 1/2] generic/430: Fix filename in "copy beyond end" test Luis Henriques @ 2017-06-26 16:40 ` Luis Henriques 2017-06-27 5:25 ` Eryu Guan 2017-06-28 14:21 ` [PATCH 1/2] generic/430: Fix filename in "copy beyond end" test David Disseldorp 1 sibling, 1 reply; 7+ messages in thread From: Luis Henriques @ 2017-06-26 16:40 UTC (permalink / raw) To: fstests; +Cc: Anna Schumaker, Luis Henriques _require_xfs_io_command() isn't handling the case where the copy_file_range syscall isn't available. Unfortunately, old versions of xfs_io don't handle it correctly either and the test will succeed with an empty file. To fix this function, we need to add two checks: 1) for old xfs_io versions, fail if the test seems to succeed (no output) but the file created is empty, 2) for newer versions, use the error returned. Signed-off-by: Luis Henriques <lhenriques@suse.com> --- common/rc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/common/rc b/common/rc index 2972f89e9527..aad065cb2ade 100644 --- a/common/rc +++ b/common/rc @@ -2146,6 +2146,10 @@ _require_xfs_io_command() $XFS_IO_PROG -F -f -c "pwrite 0 4k" $testfile > /dev/null 2>&1 testio=`$XFS_IO_PROG -F -f -c "copy_range $testfile" $testcopy 2>&1` rm -f $testcopy > /dev/null 2>&1 + [ -z "$testio" -a ! -s "$testcopy" ] && \ + _notrun "xfs_io $command support is missing" + echo $testio | egrep -q "Function not implemented" && \ + _notrun "xfs_io $command support is missing" ;; "falloc" ) testio=`$XFS_IO_PROG -F -f -c "falloc $param 0 1m" $testfile 2>&1` ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] common/rc: handle xfs_io copy_range when copy_file_range syscall isn't available 2017-06-26 16:40 ` [PATCH 2/2] common/rc: handle xfs_io copy_range when copy_file_range syscall isn't available Luis Henriques @ 2017-06-27 5:25 ` Eryu Guan 2017-06-27 8:50 ` Luis Henriques 0 siblings, 1 reply; 7+ messages in thread From: Eryu Guan @ 2017-06-27 5:25 UTC (permalink / raw) To: Luis Henriques; +Cc: fstests, Anna Schumaker On Mon, Jun 26, 2017 at 05:40:53PM +0100, Luis Henriques wrote: > _require_xfs_io_command() isn't handling the case where the copy_file_range > syscall isn't available. Unfortunately, old versions of xfs_io don't > handle it correctly either and the test will succeed with an empty file. If copy_file_range syscall isn't available on the system, xfs_io should have no "copy_range" built either, or if you're using a pre-built xfs_io binary shipped by a distro, it should have no copy_range either to match the kernel space. Anyway, I don't think it would cause any problem if there's no copy_file_range syscall support. I'm curious what's your environment setup. > > To fix this function, we need to add two checks: > > 1) for old xfs_io versions, fail if the test seems to succeed (no output) > but the file created is empty, > 2) for newer versions, use the error returned. > > Signed-off-by: Luis Henriques <lhenriques@suse.com> > --- > common/rc | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/common/rc b/common/rc > index 2972f89e9527..aad065cb2ade 100644 > --- a/common/rc > +++ b/common/rc > @@ -2146,6 +2146,10 @@ _require_xfs_io_command() > $XFS_IO_PROG -F -f -c "pwrite 0 4k" $testfile > /dev/null 2>&1 > testio=`$XFS_IO_PROG -F -f -c "copy_range $testfile" $testcopy 2>&1` > rm -f $testcopy > /dev/null 2>&1 $testcopy is removed here... > + [ -z "$testio" -a ! -s "$testcopy" ] && \ then on hosts with copy_file_range support, this check always returns true, and test _notrun when it should run. Thanks, Eryu > + _notrun "xfs_io $command support is missing" > + echo $testio | egrep -q "Function not implemented" && \ > + _notrun "xfs_io $command support is missing" > ;; > "falloc" ) > testio=`$XFS_IO_PROG -F -f -c "falloc $param 0 1m" $testfile 2>&1` > -- > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] common/rc: handle xfs_io copy_range when copy_file_range syscall isn't available 2017-06-27 5:25 ` Eryu Guan @ 2017-06-27 8:50 ` Luis Henriques 2017-06-28 7:50 ` Eryu Guan 0 siblings, 1 reply; 7+ messages in thread From: Luis Henriques @ 2017-06-27 8:50 UTC (permalink / raw) To: Eryu Guan; +Cc: fstests, Anna Schumaker On Tue, Jun 27, 2017 at 01:25:29PM +0800, Eryu Guan wrote: > On Mon, Jun 26, 2017 at 05:40:53PM +0100, Luis Henriques wrote: > > _require_xfs_io_command() isn't handling the case where the copy_file_range > > syscall isn't available. Unfortunately, old versions of xfs_io don't > > handle it correctly either and the test will succeed with an empty file. > > If copy_file_range syscall isn't available on the system, xfs_io should > have no "copy_range" built either, or if you're using a pre-built xfs_io > binary shipped by a distro, it should have no copy_range either to match > the kernel space. Anyway, I don't think it would cause any problem if > there's no copy_file_range syscall support. I'm curious what's your > environment setup. I am building an initramfs that contains the testing tools from a recent distro, and running the tests against an old kernel (which does not include this syscall). I'm not sure how usual this sort of setup is. Would you rather have the test ignoring these sort of setups, and just include the 2nd check (i.e., always assume the tools (xfs_io) are in sync with the kernel)? > > > > To fix this function, we need to add two checks: > > > > 1) for old xfs_io versions, fail if the test seems to succeed (no output) > > but the file created is empty, > > 2) for newer versions, use the error returned. > > > > Signed-off-by: Luis Henriques <lhenriques@suse.com> > > --- > > common/rc | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/common/rc b/common/rc > > index 2972f89e9527..aad065cb2ade 100644 > > --- a/common/rc > > +++ b/common/rc > > @@ -2146,6 +2146,10 @@ _require_xfs_io_command() > > $XFS_IO_PROG -F -f -c "pwrite 0 4k" $testfile > /dev/null 2>&1 > > testio=`$XFS_IO_PROG -F -f -c "copy_range $testfile" $testcopy 2>&1` > > rm -f $testcopy > /dev/null 2>&1 > > $testcopy is removed here... > > > + [ -z "$testio" -a ! -s "$testcopy" ] && \ > > then on hosts with copy_file_range support, this check always returns > true, and test _notrun when it should run. /me blushes and hides I'll send v2 once you let me know if you want me to keep these 2 checks. Cheers, -- Luís > > Thanks, > Eryu > > > + _notrun "xfs_io $command support is missing" > > + echo $testio | egrep -q "Function not implemented" && \ > > + _notrun "xfs_io $command support is missing" > > ;; > > "falloc" ) > > testio=`$XFS_IO_PROG -F -f -c "falloc $param 0 1m" $testfile 2>&1` > > -- > > 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] common/rc: handle xfs_io copy_range when copy_file_range syscall isn't available 2017-06-27 8:50 ` Luis Henriques @ 2017-06-28 7:50 ` Eryu Guan 2017-06-28 11:20 ` [PATCH 2/2 v2] " Luis Henriques 0 siblings, 1 reply; 7+ messages in thread From: Eryu Guan @ 2017-06-28 7:50 UTC (permalink / raw) To: Luis Henriques; +Cc: fstests, Anna Schumaker On Tue, Jun 27, 2017 at 09:50:13AM +0100, Luis Henriques wrote: > On Tue, Jun 27, 2017 at 01:25:29PM +0800, Eryu Guan wrote: > > On Mon, Jun 26, 2017 at 05:40:53PM +0100, Luis Henriques wrote: > > > _require_xfs_io_command() isn't handling the case where the copy_file_range > > > syscall isn't available. Unfortunately, old versions of xfs_io don't > > > handle it correctly either and the test will succeed with an empty file. > > > > If copy_file_range syscall isn't available on the system, xfs_io should > > have no "copy_range" built either, or if you're using a pre-built xfs_io > > binary shipped by a distro, it should have no copy_range either to match > > the kernel space. Anyway, I don't think it would cause any problem if > > there's no copy_file_range syscall support. I'm curious what's your > > environment setup. > > I am building an initramfs that contains the testing tools from a recent > distro, and running the tests against an old kernel (which does not > include this syscall). I'm not sure how usual this sort of setup is. > > Would you rather have the test ignoring these sort of setups, and just > include the 2nd check (i.e., always assume the tools (xfs_io) are in sync > with the kernel)? I'm fine with having this "Function not implemented" check, it'd be better to add more background information in the commit log. And perhaps we should do it after the case switch so it not only applies to copy_file_range syscall, but also to all other syscalls/commands. But I don't think we should workaround a xfsprogs bug in test, it revealed a real bug and that's what should be really fixed, i.e. build the initramfs with a newer version of xfsprogs with that bug fixed? Thanks, Eryu > > > > > > > To fix this function, we need to add two checks: > > > > > > 1) for old xfs_io versions, fail if the test seems to succeed (no output) > > > but the file created is empty, > > > 2) for newer versions, use the error returned. > > > > > > Signed-off-by: Luis Henriques <lhenriques@suse.com> > > > --- > > > common/rc | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/common/rc b/common/rc > > > index 2972f89e9527..aad065cb2ade 100644 > > > --- a/common/rc > > > +++ b/common/rc > > > @@ -2146,6 +2146,10 @@ _require_xfs_io_command() > > > $XFS_IO_PROG -F -f -c "pwrite 0 4k" $testfile > /dev/null 2>&1 > > > testio=`$XFS_IO_PROG -F -f -c "copy_range $testfile" $testcopy 2>&1` > > > rm -f $testcopy > /dev/null 2>&1 > > > > $testcopy is removed here... > > > > > + [ -z "$testio" -a ! -s "$testcopy" ] && \ > > > > then on hosts with copy_file_range support, this check always returns > > true, and test _notrun when it should run. > > /me blushes and hides > > I'll send v2 once you let me know if you want me to keep these 2 checks. > > Cheers, > -- > Luís > > > > > Thanks, > > Eryu > > > > > + _notrun "xfs_io $command support is missing" > > > + echo $testio | egrep -q "Function not implemented" && \ > > > + _notrun "xfs_io $command support is missing" > > > ;; > > > "falloc" ) > > > testio=`$XFS_IO_PROG -F -f -c "falloc $param 0 1m" $testfile 2>&1` > > > -- > > > 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 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2 v2] common/rc: handle xfs_io copy_range when copy_file_range syscall isn't available 2017-06-28 7:50 ` Eryu Guan @ 2017-06-28 11:20 ` Luis Henriques 0 siblings, 0 replies; 7+ messages in thread From: Luis Henriques @ 2017-06-28 11:20 UTC (permalink / raw) To: Eryu Guan; +Cc: fstests, Anna Schumaker, Luis Henriques _require_xfs_io_command() isn't handling the case where the copy_file_range syscall isn't available. To fix this simply check the error returned by xfs_io. Signed-off-by: Luis Henriques <lhenriques@suse.com> --- Changes since v1: - dropped check that worked around an xfs_io bug. common/rc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/rc b/common/rc index 2972f89e9527..57d596cd4f2e 100644 --- a/common/rc +++ b/common/rc @@ -2195,6 +2195,8 @@ _require_xfs_io_command() _notrun "xfs_io $command failed (old kernel/wrong fs/bad args?)" echo $testio | grep -q "foreign file active" && \ _notrun "xfs_io $command not supported on $FSTYP" + echo $testio | egrep -q "Function not implemented" && \ + _notrun "xfs_io $command support is missing (missing syscall?)" if [ -n "$param" -a $param_checked -eq 0 ]; then $XFS_IO_PROG -c "help $command" | grep -q "^ $param --" || \ ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] generic/430: Fix filename in "copy beyond end" test 2017-06-26 16:40 [PATCH 1/2] generic/430: Fix filename in "copy beyond end" test Luis Henriques 2017-06-26 16:40 ` [PATCH 2/2] common/rc: handle xfs_io copy_range when copy_file_range syscall isn't available Luis Henriques @ 2017-06-28 14:21 ` David Disseldorp 1 sibling, 0 replies; 7+ messages in thread From: David Disseldorp @ 2017-06-28 14:21 UTC (permalink / raw) To: Luis Henriques; +Cc: fstests, Anna Schumaker On Mon, 26 Jun 2017 17:40:52 +0100, Luis Henriques wrote: > The cmp command was using the wrong file to perform the comparison. Use > $testdir/beyond instead of $testdir/end. > > Signed-off-by: Luis Henriques <lhenriques@suse.com> This looks correct to me. Reviewed-by: David Disseldorp <ddiss@suse.de> ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-06-28 14:22 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-06-26 16:40 [PATCH 1/2] generic/430: Fix filename in "copy beyond end" test Luis Henriques 2017-06-26 16:40 ` [PATCH 2/2] common/rc: handle xfs_io copy_range when copy_file_range syscall isn't available Luis Henriques 2017-06-27 5:25 ` Eryu Guan 2017-06-27 8:50 ` Luis Henriques 2017-06-28 7:50 ` Eryu Guan 2017-06-28 11:20 ` [PATCH 2/2 v2] " Luis Henriques 2017-06-28 14:21 ` [PATCH 1/2] generic/430: Fix filename in "copy beyond end" test David Disseldorp
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.