On Tue, Feb 18, 2014 at 05:52:20PM -0600, Eric Sandeen wrote:
> This is the current series I have leading up to verifier error
> differentiation,
> just resending them all.
>
> As I send this, I remember that EILSEQ is giving us an unusual perror()
> output, so the last patch which uses it may need to pick some (?)
> other error code...
Eric, this leaks EBADMSG to userspace in xfs/005 with a corrupted
superblock:
$ diff -u tests/xfs/005.out /home/dave/src/xfstests-dev/results//xfs/005.out.bad
--- tests/xfs/005.out 2014-02-24 09:58:09.000000000 +1100
+++ /home/dave/src/xfstests-dev/results//xfs/005.out.bad
2014-02-27 19:40:56.000000000 +1100
@@ -1,4 +1,4 @@
QA output created by 005
wrote 4/4 bytes at offset 224
XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-mount: Structure needs cleaning
+mount: Bad message
Dmesg contains this:
[ 8790.946956] XFS (vdb): Metadata CRC error detected at
xfs_sb_read_verify+0x10c/0x140, block 0x0
[ 8790.949984] XFS (vdb): Unmount and run xfs_repair
[ 8790.951725] XFS (vdb): First 64 bytes of corrupted metadata buffer:
[ 8790.953883] ffff880003eea000: 58 46 53 42 00 00 10 00 00 00 00 00 00 2c 00
00 XFSB.........,..
[ 8790.956452] ffff880003eea010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 ................
[ 8790.958518] ffff880003eea020: 0b 21 13 2a 5b 00 40 59 bd 7c 2b d4 5b 6c a4
0a .!.*[.@Y.|+.[l..
[ 8790.960487] ffff880003eea030: 00 00 00 00 00 20 00 04 00 00 00 00 00 00 00
40 ..... .........@
So that part is good. However, I suspect we should still be
returning EFSCORRUPTED to userspace. Most metadata buffer reads go
through xfs_trans_read_buf_map(), so there only a couple of places
we need to catch and convert the error being returned. The other two
I notice that are need conversion are xfs_buf_read() in
xfs_readlink_bmap(), and the cause of this regression which is
xfs_buf_read_uncached() in xfs_readsb()....
Can you look into writing up a followup patch that does this
catch-and-convert? It will also mean all the higher layers will
treat a CRC error as corrupt metadata, which is exactly what we want
it to do. :)
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
|