| Message ID | 1487566979-6147-1-git-send-email-yangx.jy@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Feb 20, 2017 at 01:02:59PM +0800, Xiao Yang wrote: > 1) If the kernel bug has been fixed, stat command fails to get i_size > and reports "Structure needs cleaning". So we use debugfs -R "stat" > instead of stat command to make sure debugfs sets negative i_size. > These cases have been broken by commit 0e13e40b247a1 ("shared/005,7: > make sure debugfs sets negative i_size"). > > 2) shared/007 passes unexpectedly on RHEL6.8GA and RHEL6.9Beta > because of invalid argument. When debugfs fails to set i_size > to -512, dd with direct flag fails and reports "invalid argument". > So we can add strict check. But you've already added the check for i_size and _notrun if debugfs failed to set i_size to -512, there's no chance for dd to hit EINVAL. And I think any error dd is hitting is acceptable, as long as test runs (which means i_size was set to -1/-512 successfully). So I don't think we need the check on dd's output. Did I miss anything? 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
On 2017/02/22 12:20, Eryu Guan wrote: > On Mon, Feb 20, 2017 at 01:02:59PM +0800, Xiao Yang wrote: >> 1) If the kernel bug has been fixed, stat command fails to get i_size >> and reports "Structure needs cleaning". So we use debugfs -R "stat" >> instead of stat command to make sure debugfs sets negative i_size. >> These cases have been broken by commit 0e13e40b247a1 ("shared/005,7: >> make sure debugfs sets negative i_size"). >> >> 2) shared/007 passes unexpectedly on RHEL6.8GA and RHEL6.9Beta >> because of invalid argument. When debugfs fails to set i_size >> to -512, dd with direct flag fails and reports "invalid argument". >> So we can add strict check. > But you've already added the check for i_size and _notrun if debugfs > failed to set i_size to -512, there's no chance for dd to hit EINVAL. > And I think any error dd is hitting is acceptable, as long as test runs > (which means i_size was set to -1/-512 successfully). So I don't think > we need the check on dd's output. Did I miss anything? Hi Eryu Thanks for your comments. xfs/134 passed unexpectedly on RHEL7.3GA because dd hits ENOSPC. So I think all of these cases should fail instead of pass when hitting unexpected error. Do you think whether we only check on dd's output of xfs/133,4 or remove all? :-) Best Regards, XIao Yang > > > 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
On Wed, Feb 22, 2017 at 12:56:54PM +0800, Xiao Yang wrote: > On 2017/02/22 12:20, Eryu Guan wrote: > > On Mon, Feb 20, 2017 at 01:02:59PM +0800, Xiao Yang wrote: > > > 1) If the kernel bug has been fixed, stat command fails to get i_size > > > and reports "Structure needs cleaning". So we use debugfs -R "stat" > > > instead of stat command to make sure debugfs sets negative i_size. > > > These cases have been broken by commit 0e13e40b247a1 ("shared/005,7: > > > make sure debugfs sets negative i_size"). > > > > > > 2) shared/007 passes unexpectedly on RHEL6.8GA and RHEL6.9Beta > > > because of invalid argument. When debugfs fails to set i_size > > > to -512, dd with direct flag fails and reports "invalid argument". > > > So we can add strict check. > > But you've already added the check for i_size and _notrun if debugfs > > failed to set i_size to -512, there's no chance for dd to hit EINVAL. > > And I think any error dd is hitting is acceptable, as long as test runs > > (which means i_size was set to -1/-512 successfully). So I don't think > > we need the check on dd's output. Did I miss anything? > Hi Eryu > > Thanks for your comments. > > xfs/134 passed unexpectedly on RHEL7.3GA because dd hits ENOSPC. So I think Hmm, xfs/134 hits ENOSPC even on 4.9 kernel (where xfs/133 triggers kernel BUG_ON), so I think the buffer I/O tests are targeded regression tests and the dio tests are nice-to-have tests. > all of these cases should fail instead of pass when hitting unexpected > error. > Do you think whether we only check on dd's output of xfs/133,4 or remove > all? :-) IMO, what we're going to test is to make sure kernel doesn't hang or crash when dealing with negative i_size, any error is acceptable as long as kernel could detect this corruption without crashing or hanging. 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 a/tests/shared/005 b/tests/shared/005 index 76c551c..23de873 100755 --- a/tests/shared/005 +++ b/tests/shared/005 @@ -65,16 +65,17 @@ echo "Corrupt filesystem" _scratch_unmount $DEBUGFS_PROG -w -R "sif /a size -1" $SCRATCH_DEV >> $seqres.full 2>&1 -echo "Remount, try to append" -_scratch_mount - # check whether debugfs succeeds to set i_size to -1 or not -i_size=$(stat -c "%s" $testdir/a) -if [ $i_size -ne -1 ]; then +$DEBUGFS_PROG -R "stat /a" $SCRATCH_DEV 2>&1 | grep -q "Size: 18446744073709551615" || \ _notrun "Could not set i_size to -1 successfully, skip test." -fi -dd if=/dev/zero of=$testdir/a bs=512 count=1 oflag=append conv=notrunc >> $seqres.full 2>&1 || echo "Write did not succeed (ok)." +echo "Remount, try to append" +_scratch_mount + +# dd may fail because of unexpected cause(eg. invalid argument), so we add strict check. +dd_out=$(dd if=/dev/zero of=$testdir/a bs=512 count=1 oflag=append conv=notrunc 2>&1) +echo $dd_out | grep -q "Structure needs cleaning" && echo "Write did not succeed (ok)." +echo $dd_out >> $seqres.full sync # success, all done diff --git a/tests/shared/007 b/tests/shared/007 index 77636f4..fc7e3ac 100755 --- a/tests/shared/007 +++ b/tests/shared/007 @@ -67,16 +67,17 @@ _scratch_unmount # -1 so that we can perform a dio write. $DEBUGFS_PROG -w -R "sif /a size 0xFFFFFFFFFFFFFE00" $SCRATCH_DEV >> $seqres.full 2>&1 -echo "Remount, try to append" -_scratch_mount - # check whether debugfs succeeds to set i_size to -512 or not -i_size=$(stat -c "%s" $testdir/a) -if [ $i_size -ne -512 ]; then +$DEBUGFS_PROG -R "stat /a" $SCRATCH_DEV 2>&1 | grep -q "Size: 18446744073709551104" || \ _notrun "Could not set i_size to -512 successfully, skip test." -fi -dd if=/dev/zero of=$testdir/a bs=512 count=1 oflag=direct,append conv=notrunc >> $seqres.full 2>&1 || echo "Write did not succeed (ok)." +echo "Remount, try to append" +_scratch_mount + +# dd may fail because of unexpected cause (eg. invalid argument), so we add strict check. +dd_out=$(dd if=/dev/zero of=$testdir/a bs=512 count=1 oflag=direct,append conv=notrunc 2>&1) +echo $dd_out | grep -q "Structure needs cleaning" && echo "Write did not succeed (ok)." +echo $dd_out >> $seqres.full sync # success, all done
1) If the kernel bug has been fixed, stat command fails to get i_size and reports "Structure needs cleaning". So we use debugfs -R "stat" instead of stat command to make sure debugfs sets negative i_size. These cases have been broken by commit 0e13e40b247a1 ("shared/005,7: make sure debugfs sets negative i_size"). 2) shared/007 passes unexpectedly on RHEL6.8GA and RHEL6.9Beta because of invalid argument. When debugfs fails to set i_size to -512, dd with direct flag fails and reports "invalid argument". So we can add strict check. Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com> --- tests/shared/005 | 15 ++++++++------- tests/shared/007 | 15 ++++++++------- 2 files changed, 16 insertions(+), 14 deletions(-)