* [PATCH] shared/005,7: add check whether debugfs succeeds to set a negative i_size or not
@ 2017-02-13 11:21 Xiao Yang
2017-02-20 2:13 ` Xiao Yang
0 siblings, 1 reply; 13+ messages in thread
From: Xiao Yang @ 2017-02-13 11:21 UTC (permalink / raw)
To: fstests; +Cc: Xiao Yang
shared/005 and shared/007 work abnornamlly on RHEL6.8GA and
RHEL6.9Beta because debugfs fails to set i_size to -1 or -512
and reports "exceeds field size maximum". When debugfs fails
to set a negative i_size, we can skip these cases which don't
trigger the kernel bug.
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
tests/shared/005 | 7 +++++++
tests/shared/007 | 7 +++++++
2 files changed, 14 insertions(+)
diff --git a/tests/shared/005 b/tests/shared/005
index 296de17..37912d6 100755
--- a/tests/shared/005
+++ b/tests/shared/005
@@ -67,6 +67,13 @@ $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
+ _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)."
sync
diff --git a/tests/shared/007 b/tests/shared/007
index 0729ab8..06055a0 100755
--- a/tests/shared/007
+++ b/tests/shared/007
@@ -69,6 +69,13 @@ $DEBUGFS_PROG -w -R "sif /a size 0xFFFFFFFFFFFFFE00" $SCRATCH_DEV >> $seqres.ful
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
+ _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)."
sync
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] shared/005,7: add check whether debugfs succeeds to set a negative i_size or not
2017-02-13 11:21 [PATCH] shared/005,7: add check whether debugfs succeeds to set a negative i_size or not Xiao Yang
@ 2017-02-20 2:13 ` Xiao Yang
2017-02-20 2:36 ` Eryu Guan
0 siblings, 1 reply; 13+ messages in thread
From: Xiao Yang @ 2017-02-20 2:13 UTC (permalink / raw)
To: Xiao Yang; +Cc: fstests
Hi Eryu
Sorry, The previous patch is defective. Please see the following two points:
1) If the kernel bug has been fixed, stat command fails to get i_size and
reports "Structure needs cleaning".
2) shared/007 passed unexpectedly because of invalid argument when debugfs
fails to set i_size to the highest multiple of 512 below -1. So we need
to add strict check.
Thanks for you review. I will rewrite this patch.
Best Regards,
Xiao Yang
On 2017/02/13 19:21, Xiao Yang Wrote:
> shared/005 and shared/007 work abnornamlly on RHEL6.8GA and
> RHEL6.9Beta because debugfs fails to set i_size to -1 or -512
> and reports "exceeds field size maximum". When debugfs fails
> to set a negative i_size, we can skip these cases which don't
> trigger the kernel bug.
>
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
> tests/shared/005 | 7 +++++++
> tests/shared/007 | 7 +++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/tests/shared/005 b/tests/shared/005
> index 296de17..37912d6 100755
> --- a/tests/shared/005
> +++ b/tests/shared/005
> @@ -67,6 +67,13 @@ $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
> + _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)."
> sync
>
> diff --git a/tests/shared/007 b/tests/shared/007
> index 0729ab8..06055a0 100755
> --- a/tests/shared/007
> +++ b/tests/shared/007
> @@ -69,6 +69,13 @@ $DEBUGFS_PROG -w -R "sif /a size 0xFFFFFFFFFFFFFE00" $SCRATCH_DEV >> $seqres.ful
>
> 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
> + _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)."
> sync
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] shared/005,7: add check whether debugfs succeeds to set a negative i_size or not
2017-02-20 2:13 ` Xiao Yang
@ 2017-02-20 2:36 ` Eryu Guan
2017-02-20 5:02 ` [PATCH] shared/005,7: fix making sure debugfs sets negative i_size && add strict check for dd Xiao Yang
2017-02-20 9:37 ` [PATCH] xfs/133,4: make sure xfs_db sets negative i_size && add strict check for dd Xiao Yang
0 siblings, 2 replies; 13+ messages in thread
From: Eryu Guan @ 2017-02-20 2:36 UTC (permalink / raw)
To: Xiao Yang; +Cc: fstests
On Mon, Feb 20, 2017 at 10:13:46AM +0800, Xiao Yang wrote:
> Hi Eryu
>
> Sorry, The previous patch is defective. Please see the following two points:
>
> 1) If the kernel bug has been fixed, stat command fails to get i_size and
> reports "Structure needs cleaning".
>
> 2) shared/007 passed unexpectedly because of invalid argument when debugfs
> fails to set i_size to the highest multiple of 512 below -1. So we need
> to add strict check.
Thanks for the update! I've pushed your patch to upstream yesterday (not
sure why I didn't see the announce email in the list.. I'll resend
shortly, but the upstream repo should have been updated). Please send a
fix based on new master.
Thanks,
Eryu
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] shared/005,7: fix making sure debugfs sets negative i_size && add strict check for dd
2017-02-20 2:36 ` Eryu Guan
@ 2017-02-20 5:02 ` Xiao Yang
2017-02-22 4:20 ` Eryu Guan
2017-02-20 9:37 ` [PATCH] xfs/133,4: make sure xfs_db sets negative i_size && add strict check for dd Xiao Yang
1 sibling, 1 reply; 13+ messages in thread
From: Xiao Yang @ 2017-02-20 5:02 UTC (permalink / raw)
To: eguan; +Cc: fstests, Xiao Yang
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(-)
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.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH] xfs/133,4: make sure xfs_db sets negative i_size && add strict check for dd
2017-02-20 2:36 ` Eryu Guan
2017-02-20 5:02 ` [PATCH] shared/005,7: fix making sure debugfs sets negative i_size && add strict check for dd Xiao Yang
@ 2017-02-20 9:37 ` Xiao Yang
2017-02-22 9:31 ` [PATCH v2] xfs/133,4: make sure xfs_db sets negative i_size Xiao Yang
1 sibling, 1 reply; 13+ messages in thread
From: Xiao Yang @ 2017-02-20 9:37 UTC (permalink / raw)
To: eguan; +Cc: fstests, Xiao Yang
1) xfs/133 and xfs/134 work abnornamlly on RHEL6.8GA and RHEL6.9Beta
because xfs_db fails to set i_size to -1 or -512 and reports "usage:
write fieldname value". The special argument "--" is only used to
end option-scanning in getopt(). So we should add "--" for write
command in xfs_db when -c option is supported by write command.
getopt() has been produced by 'commit c9f5e3db22098 ("xfs_db: Allow
writes of corrupted data")'.
2) When xfs_db fails to set a negative i_size due to unknown error, we can
skip these cases which don't trigger the kernel bug.
3) xfs/134 passes unexpectedly on RHEL7.3GA due to no space left on device,
So we can add strict check.
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
tests/xfs/133 | 20 ++++++++++++++++++--
tests/xfs/134 | 23 +++++++++++++++++++----
2 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/tests/xfs/133 b/tests/xfs/133
index d756d2e..0d7587e 100755
--- a/tests/xfs/133
+++ b/tests/xfs/133
@@ -63,11 +63,27 @@ inum=$(stat -c "%i" $testdir/a)
echo "Corrupt filesystem"
_scratch_unmount
-_scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -- -1' >> $seqres.full
+
+# 1) we should add "--" for write command in xfs_db when -c option is
+# supported by write command.
+# 2) check whether xfs_db succeeds to set i_size to -1 or not
+_scratch_xfs_db -x -c "help" | grep -q "write \[-c\]"
+if [ $? -eq 0 ]; then
+ _scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -- -1' >> $seqres.full 2>&1 || \
+ _notrun "Could not set i_size to -1 successfully, skip test."
+else
+ _scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -1' >> $seqres.full 2>&1 || \
+ _notrun "Could not set i_size to -1 successfully, skip test."
+fi
echo "Remount, try to append"
_scratch_mount
-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)."
+
+# dd may fail unexpectedly(eg. no space left on device), 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/xfs/134 b/tests/xfs/134
index 09466cd..d6d2dec 100755
--- a/tests/xfs/134
+++ b/tests/xfs/134
@@ -63,13 +63,28 @@ inum=$(stat -c "%i" $testdir/a)
echo "Corrupt filesystem"
_scratch_unmount
-# Set the file size to the highest multiple of 512 below
-# -1 so that we can perform a dio write.
-_scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -- -512' >> $seqres.full
+
+# 1) Set the file size to the highest multiple of 512 below -1 so
+# that we can perform a dio write.
+# 2) we should add "--" for write command in xfs_db when -c option is
+# supported by write command.
+# 3) check whether xfs_db succeeds to set i_size to -512 or not
+_scratch_xfs_db -x -c "help" | grep -q "write \[-c\]"
+if [ $? -eq 0 ]; then
+ _scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -- -512' >> $seqres.full 2>&1 || \
+ _notrun "Could not set i_size to -512 successfully, skip test."
+else
+ _scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -512' >> $seqres.full 2>&1 || \
+ _notrun "Could not set i_size to -512 successfully, skip test."
+fi
echo "Remount, try to append"
_scratch_mount
-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)."
+
+# dd may fail unexpectedly(eg. no space left on device), 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.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] shared/005,7: fix making sure debugfs sets negative i_size && add strict check for dd
2017-02-20 5:02 ` [PATCH] shared/005,7: fix making sure debugfs sets negative i_size && add strict check for dd Xiao Yang
@ 2017-02-22 4:20 ` Eryu Guan
2017-02-22 4:56 ` Xiao Yang
0 siblings, 1 reply; 13+ messages in thread
From: Eryu Guan @ 2017-02-22 4:20 UTC (permalink / raw)
To: Xiao Yang; +Cc: fstests
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] shared/005,7: fix making sure debugfs sets negative i_size && add strict check for dd
2017-02-22 4:20 ` Eryu Guan
@ 2017-02-22 4:56 ` Xiao Yang
2017-02-22 6:53 ` Eryu Guan
0 siblings, 1 reply; 13+ messages in thread
From: Xiao Yang @ 2017-02-22 4:56 UTC (permalink / raw)
To: Eryu Guan; +Cc: fstests
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
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] shared/005,7: fix making sure debugfs sets negative i_size && add strict check for dd
2017-02-22 4:56 ` Xiao Yang
@ 2017-02-22 6:53 ` Eryu Guan
2017-02-22 8:17 ` [PATCH v2] shared/005,7: fix making sure debugfs sets negative i_size Xiao Yang
0 siblings, 1 reply; 13+ messages in thread
From: Eryu Guan @ 2017-02-22 6:53 UTC (permalink / raw)
To: Xiao Yang; +Cc: fstests
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2] shared/005,7: fix making sure debugfs sets negative i_size
2017-02-22 6:53 ` Eryu Guan
@ 2017-02-22 8:17 ` Xiao Yang
0 siblings, 0 replies; 13+ messages in thread
From: Xiao Yang @ 2017-02-22 8:17 UTC (permalink / raw)
To: eguan; +Cc: fstests, Xiao Yang
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").
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
tests/shared/005 | 9 +++------
tests/shared/007 | 9 +++------
2 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/tests/shared/005 b/tests/shared/005
index 76c551c..de7b84f 100755
--- a/tests/shared/005
+++ b/tests/shared/005
@@ -65,15 +65,12 @@ 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
+echo "Remount, try to append"
+_scratch_mount
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)."
sync
diff --git a/tests/shared/007 b/tests/shared/007
index 77636f4..785f19f 100755
--- a/tests/shared/007
+++ b/tests/shared/007
@@ -67,15 +67,12 @@ _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
+echo "Remount, try to append"
+_scratch_mount
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)."
sync
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2] xfs/133,4: make sure xfs_db sets negative i_size
2017-02-20 9:37 ` [PATCH] xfs/133,4: make sure xfs_db sets negative i_size && add strict check for dd Xiao Yang
@ 2017-02-22 9:31 ` Xiao Yang
2017-02-22 10:49 ` Eryu Guan
0 siblings, 1 reply; 13+ messages in thread
From: Xiao Yang @ 2017-02-22 9:31 UTC (permalink / raw)
To: eguan; +Cc: fstests, Xiao Yang
1) xfs/133 and xfs/134 work abnornamlly on RHEL6.8GA and RHEL6.9Beta
because xfs_db fails to set i_size to -1 or -512 and reports "usage:
write fieldname value". The special argument "--" is only used to
end option-scanning in getopt(). So we should add "--" for write
command in xfs_db when -c option is supported by write command.
getopt() has been produced by 'commit c9f5e3db22098 ("xfs_db: Allow
writes of corrupted data")'.
2) When xfs_db fails to set a negative i_size due to unknown error, we
can skip these cases which don't trigger the kernel bug.
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
tests/xfs/133 | 13 ++++++++++++-
tests/xfs/134 | 17 ++++++++++++++---
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/tests/xfs/133 b/tests/xfs/133
index d756d2e..91b3b9b 100755
--- a/tests/xfs/133
+++ b/tests/xfs/133
@@ -63,7 +63,18 @@ inum=$(stat -c "%i" $testdir/a)
echo "Corrupt filesystem"
_scratch_unmount
-_scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -- -1' >> $seqres.full
+
+# 1) we should add "--" for write command in xfs_db when -c option is
+# supported by write command.
+# 2) check whether xfs_db succeeds to set i_size to -1 or not
+_scratch_xfs_db -x -c "help" | grep -q "write \[-c\]"
+if [ $? -eq 0 ]; then
+ _scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -- -1' >> $seqres.full 2>&1 || \
+ _notrun "Could not set i_size to -1 successfully, skip test."
+else
+ _scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -1' >> $seqres.full 2>&1 || \
+ _notrun "Could not set i_size to -1 successfully, skip test."
+fi
echo "Remount, try to append"
_scratch_mount
diff --git a/tests/xfs/134 b/tests/xfs/134
index 09466cd..788f8e9 100755
--- a/tests/xfs/134
+++ b/tests/xfs/134
@@ -63,9 +63,20 @@ inum=$(stat -c "%i" $testdir/a)
echo "Corrupt filesystem"
_scratch_unmount
-# Set the file size to the highest multiple of 512 below
-# -1 so that we can perform a dio write.
-_scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -- -512' >> $seqres.full
+
+# 1) Set the file size to the highest multiple of 512 below -1 so
+# that we can perform a dio write.
+# 2) we should add "--" for write command in xfs_db when -c option is
+# supported by write command.
+# 3) check whether xfs_db succeeds to set i_size to -512 or not
+_scratch_xfs_db -x -c "help" | grep -q "write \[-c\]"
+if [ $? -eq 0 ]; then
+ _scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -- -512' >> $seqres.full 2>&1 || \
+ _notrun "Could not set i_size to -512 successfully, skip test."
+else
+ _scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -512' >> $seqres.full 2>&1 || \
+ _notrun "Could not set i_size to -512 successfully, skip test."
+fi
echo "Remount, try to append"
_scratch_mount
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2] xfs/133,4: make sure xfs_db sets negative i_size
2017-02-22 9:31 ` [PATCH v2] xfs/133,4: make sure xfs_db sets negative i_size Xiao Yang
@ 2017-02-22 10:49 ` Eryu Guan
2017-02-23 3:23 ` Xiao Yang
2017-02-23 3:35 ` [PATCH v3] " Xiao Yang
0 siblings, 2 replies; 13+ messages in thread
From: Eryu Guan @ 2017-02-22 10:49 UTC (permalink / raw)
To: Xiao Yang; +Cc: fstests
On Wed, Feb 22, 2017 at 05:31:40PM +0800, Xiao Yang wrote:
> 1) xfs/133 and xfs/134 work abnornamlly on RHEL6.8GA and RHEL6.9Beta
> because xfs_db fails to set i_size to -1 or -512 and reports "usage:
> write fieldname value". The special argument "--" is only used to
> end option-scanning in getopt(). So we should add "--" for write
> command in xfs_db when -c option is supported by write command.
> getopt() has been produced by 'commit c9f5e3db22098 ("xfs_db: Allow
> writes of corrupted data")'.
>
> 2) When xfs_db fails to set a negative i_size due to unknown error, we
> can skip these cases which don't trigger the kernel bug.
>
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
Sorry for the late review on this patch. I just got some time to look at
xfs/133,4 a bit closely.
It turns out that the EINVAL error you saw is because of
echo m > $testdir/a
and direct append failed because the write offset is not 512-aligned.
Just "touch $testdir/a" should work (you can see test failure on RHEL6.8
with current master).
> ---
> tests/xfs/133 | 13 ++++++++++++-
> tests/xfs/134 | 17 ++++++++++++++---
> 2 files changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/tests/xfs/133 b/tests/xfs/133
> index d756d2e..91b3b9b 100755
> --- a/tests/xfs/133
> +++ b/tests/xfs/133
> @@ -63,7 +63,18 @@ inum=$(stat -c "%i" $testdir/a)
>
> echo "Corrupt filesystem"
> _scratch_unmount
> -_scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -- -1' >> $seqres.full
> +
> +# 1) we should add "--" for write command in xfs_db when -c option is
> +# supported by write command.
> +# 2) check whether xfs_db succeeds to set i_size to -1 or not
> +_scratch_xfs_db -x -c "help" | grep -q "write \[-c\]"
> +if [ $? -eq 0 ]; then
> + _scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -- -1' >> $seqres.full 2>&1 || \
> + _notrun "Could not set i_size to -1 successfully, skip test."
> +else
> + _scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -1' >> $seqres.full 2>&1 || \
> + _notrun "Could not set i_size to -1 successfully, skip test."
> +fi
This check looks tedious, and xfs_db doesn't return non-zero on error,
so this will never _notrun.
I'd suggest running the two xfs_db commands unconditionally (with clear
comments), there has to be one suitable call for the xfs_db version in
use, and check core.size is -1/-512 by printing it with xfs_db. e.g.
# <comments about these two calls>
_scratch_xfs_db -x -c "inode ${inum}" -c "write core.size -1" >>$seqres.full 2>&1
_scratch_xfs_db -x -c "inode ${inum}" -c "write core.size -- -1" >>$seqres.full 2>&1
# check core.size and _notrun if it's not set correctly
isize=`_scratch_xfs_db -c "inode ${inum}" -c "print core.size" | $AWK_PROG '{print $3}'`
if [ $isize -ne -1 ]; then
_notrun "..."
fi
Thanks,
Eryu
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] xfs/133,4: make sure xfs_db sets negative i_size
2017-02-22 10:49 ` Eryu Guan
@ 2017-02-23 3:23 ` Xiao Yang
2017-02-23 3:35 ` [PATCH v3] " Xiao Yang
1 sibling, 0 replies; 13+ messages in thread
From: Xiao Yang @ 2017-02-23 3:23 UTC (permalink / raw)
To: Eryu Guan; +Cc: fstests
On 2017/02/22 18:49, Eryu Guan wrote:
> On Wed, Feb 22, 2017 at 05:31:40PM +0800, Xiao Yang wrote:
>> 1) xfs/133 and xfs/134 work abnornamlly on RHEL6.8GA and RHEL6.9Beta
>> because xfs_db fails to set i_size to -1 or -512 and reports "usage:
>> write fieldname value". The special argument "--" is only used to
>> end option-scanning in getopt(). So we should add "--" for write
>> command in xfs_db when -c option is supported by write command.
>> getopt() has been produced by 'commit c9f5e3db22098 ("xfs_db: Allow
>> writes of corrupted data")'.
>>
>> 2) When xfs_db fails to set a negative i_size due to unknown error, we
>> can skip these cases which don't trigger the kernel bug.
>>
>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
> Sorry for the late review on this patch. I just got some time to look at
> xfs/133,4 a bit closely.
>
> It turns out that the EINVAL error you saw is because of
>
> echo m> $testdir/a
>
> and direct append failed because the write offset is not 512-aligned.
>
> Just "touch $testdir/a" should work (you can see test failure on RHEL6.8
> with current master).
>
Hi Eryu
I saw test failure on RHEL6.8 occasionally when using touch command to
create file. :-)
I will rewrite this patch as you suggestions.
Best Regards,
Xiao Yang
>> ---
>> tests/xfs/133 | 13 ++++++++++++-
>> tests/xfs/134 | 17 ++++++++++++++---
>> 2 files changed, 26 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/xfs/133 b/tests/xfs/133
>> index d756d2e..91b3b9b 100755
>> --- a/tests/xfs/133
>> +++ b/tests/xfs/133
>> @@ -63,7 +63,18 @@ inum=$(stat -c "%i" $testdir/a)
>>
>> echo "Corrupt filesystem"
>> _scratch_unmount
>> -_scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -- -1'>> $seqres.full
>> +
>> +# 1) we should add "--" for write command in xfs_db when -c option is
>> +# supported by write command.
>> +# 2) check whether xfs_db succeeds to set i_size to -1 or not
>> +_scratch_xfs_db -x -c "help" | grep -q "write \[-c\]"
>> +if [ $? -eq 0 ]; then
>> + _scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -- -1'>> $seqres.full 2>&1 || \
>> + _notrun "Could not set i_size to -1 successfully, skip test."
>> +else
>> + _scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -1'>> $seqres.full 2>&1 || \
>> + _notrun "Could not set i_size to -1 successfully, skip test."
>> +fi
> This check looks tedious, and xfs_db doesn't return non-zero on error,
> so this will never _notrun.
>
> I'd suggest running the two xfs_db commands unconditionally (with clear
> comments), there has to be one suitable call for the xfs_db version in
> use, and check core.size is -1/-512 by printing it with xfs_db. e.g.
>
> #<comments about these two calls>
> _scratch_xfs_db -x -c "inode ${inum}" -c "write core.size -1">>$seqres.full 2>&1
> _scratch_xfs_db -x -c "inode ${inum}" -c "write core.size -- -1">>$seqres.full 2>&1
>
> # check core.size and _notrun if it's not set correctly
> isize=`_scratch_xfs_db -c "inode ${inum}" -c "print core.size" | $AWK_PROG '{print $3}'`
> if [ $isize -ne -1 ]; then
> _notrun "..."
> fi
> 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
>
>
> .
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3] xfs/133,4: make sure xfs_db sets negative i_size
2017-02-22 10:49 ` Eryu Guan
2017-02-23 3:23 ` Xiao Yang
@ 2017-02-23 3:35 ` Xiao Yang
1 sibling, 0 replies; 13+ messages in thread
From: Xiao Yang @ 2017-02-23 3:35 UTC (permalink / raw)
To: eguan; +Cc: fstests, Xiao Yang
1) xfs/133 and xfs/134 work abnornamlly on RHEL6.8GA and RHEL6.9Beta
because xfs_db fails to set negative i_size and reports "usage:
write fieldname value". The special argument "--" is only used to
end option-scanning in getopt(). So we can run two xfs_db commands
to set negative i_size regardless of the special argument "--" is
needed or not. getopt() has been produced by 'commit c9f5e3db22098
("xfs_db: Allow writes of corrupted data")'.
2) xfs/134 passes unexpectedly on RHEL6.8GA due to EINVAL, so we use
touch command to create 512-aligned test file.
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
tests/xfs/133 | 12 +++++++++++-
tests/xfs/134 | 19 +++++++++++++++----
2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/tests/xfs/133 b/tests/xfs/133
index d756d2e..ff73a41 100755
--- a/tests/xfs/133
+++ b/tests/xfs/133
@@ -63,7 +63,17 @@ inum=$(stat -c "%i" $testdir/a)
echo "Corrupt filesystem"
_scratch_unmount
-_scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -- -1' >> $seqres.full
+
+# run two xfs_db commands to set core.size regardless of the special argument "--"
+# is needed or not.
+_scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -- -1' >> $seqres.full 2>&1
+_scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -1' >> $seqres.full 2>&1
+
+# check core.size and _notrun if it's not set correctly
+i_size=`_scratch_xfs_db -c "inode ${inum}" -c "print core.size" | $AWK_PROG '{print $3}'`
+if [ $i_size -ne -1 ]; then
+ _notrun "Could not set i_size to -1 successfully, skip test."
+fi
echo "Remount, try to append"
_scratch_mount
diff --git a/tests/xfs/134 b/tests/xfs/134
index 09466cd..f064476 100755
--- a/tests/xfs/134
+++ b/tests/xfs/134
@@ -58,14 +58,25 @@ _scratch_mkfs >> $seqres.full 2>&1
_scratch_mount
testdir=$SCRATCH_MNT
-echo m > $testdir/a
+# make sure the write offset is 512-aligned
+touch $testdir/a
inum=$(stat -c "%i" $testdir/a)
echo "Corrupt filesystem"
_scratch_unmount
-# Set the file size to the highest multiple of 512 below
-# -1 so that we can perform a dio write.
-_scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -- -512' >> $seqres.full
+
+# 1) Set the file size to the highest multiple of 512 below -1 so
+# that we can perform a dio write.
+# 2) run two xfs_db commands to set core.size regardless of the special
+# argument "--" is needed or not.
+_scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -- -512' >> $seqres.full 2>&1
+_scratch_xfs_db -x -c "inode ${inum}" -c 'write core.size -512' >> $seqres.full 2>&1
+
+# check core.size and _notrun if it's not set correctly
+i_size=`_scratch_xfs_db -c "inode ${inum}" -c "print core.size" | $AWK_PROG '{print $3}'`
+if [ $i_size -ne -512 ]; then
+ _notrun "Could not set i_size to -512 successfully, skip test."
+fi
echo "Remount, try to append"
_scratch_mount
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-02-23 3:36 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-13 11:21 [PATCH] shared/005,7: add check whether debugfs succeeds to set a negative i_size or not Xiao Yang
2017-02-20 2:13 ` Xiao Yang
2017-02-20 2:36 ` Eryu Guan
2017-02-20 5:02 ` [PATCH] shared/005,7: fix making sure debugfs sets negative i_size && add strict check for dd Xiao Yang
2017-02-22 4:20 ` Eryu Guan
2017-02-22 4:56 ` Xiao Yang
2017-02-22 6:53 ` Eryu Guan
2017-02-22 8:17 ` [PATCH v2] shared/005,7: fix making sure debugfs sets negative i_size Xiao Yang
2017-02-20 9:37 ` [PATCH] xfs/133,4: make sure xfs_db sets negative i_size && add strict check for dd Xiao Yang
2017-02-22 9:31 ` [PATCH v2] xfs/133,4: make sure xfs_db sets negative i_size Xiao Yang
2017-02-22 10:49 ` Eryu Guan
2017-02-23 3:23 ` Xiao Yang
2017-02-23 3:35 ` [PATCH v3] " Xiao Yang
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.