[U-Boot] [U-Boot, RESEND, v5, 3/7] test: fs: Add filesystem integrity checks

[Takahiro Akashi]

On Wed, Apr 10, 2019 at 11:51:20AM +0900, Takahiro Akashi wrote:
> On Tue, Apr 09, 2019 at 10:25:14PM -0400, Tom Rini wrote:
> > On Wed, Apr 10, 2019 at 10:37:42AM +0900, Takahiro Akashi wrote:
> > > On Tue, Apr 09, 2019 at 08:19:40PM -0400, Tom Rini wrote:
> > > > On Wed, Apr 10, 2019 at 02:10:12AM +0200, Heinrich Schuchardt wrote:
> > > > > On 4/9/19 10:03 PM, Tom Rini wrote:
> > > > > > On Wed, Feb 13, 2019 at 12:15:23PM +0100, Jean-Jacques Hiblot wrote:
> > > > > >
> > > > > >> We need to make sure that file writes,file creation, etc. are properly
> > > > > >> performed and do not corrupt the filesystem.
> > > > > >> To help with this, introduce the assert_fs_integrity() function that
> > > > > >> executes the appropriate fsck tool. It should be called at the end of any
> > > > > >> test that modify the content/organization of the filesystem.
> > > > > >> Currently only supports FATs and EXT4.
> > > > > >>
> > > > > >> Signed-off-by: Jean-Jacques Hiblot <jjhiblot at ti.com>
> > > > > >> Reviewed-by: Tom Rini <trini at konsulko.com>
> > > > > >
> > > > > > OK, I'm adding in a bunch of people to CC here.  The issue with this
> > > > > > patch is that by adding fsck to our tests we see 34 FAT16/FAT32
> > > > > > failures:
> > > > > > TestFsBasic.test_fs13[fat16]
> > > > > > TestFsBasic.test_fs11[fat32]
> > > > > > TestFsBasic.test_fs12[fat32]
> > > > > > TestFsBasic.test_fs13[fat32]
> > > > > > TestFsExt.test_fs_ext1[fat32]
> > > > > > TestFsExt.test_fs_ext2[fat32]
> > > > > > TestFsExt.test_fs_ext3[fat32]
> > > > > > TestFsExt.test_fs_ext4[fat32]
> > > > > > TestFsExt.test_fs_ext5[fat32]
> > > > > > TestFsExt.test_fs_ext6[fat32]
> > > > > > TestFsExt.test_fs_ext7[fat32]
> > > > > > TestFsExt.test_fs_ext8[fat32]
> > > > > > TestFsExt.test_fs_ext9[fat32]
> > > > > > TestMkdir.test_mkdir6[fat16]
> > > > > > TestMkdir.test_mkdir1[fat32]
> > > > > > TestMkdir.test_mkdir2[fat32]
> > > > > > TestMkdir.test_mkdir3[fat32]
> > > > > > TestMkdir.test_mkdir4[fat32]
> > > > > > TestMkdir.test_mkdir5[fat32]
> > > > > > TestMkdir.test_mkdir6[fat32]
> > > > > > TestUnlink.test_unlink1[fat16]
> > > > > > TestUnlink.test_unlink2[fat16]
> > > > > > TestUnlink.test_unlink3[fat16]
> > > > > > TestUnlink.test_unlink4[fat16]
> > > > > > TestUnlink.test_unlink5[fat16]
> > > > > > TestUnlink.test_unlink6[fat16]
> > > > > > TestUnlink.test_unlink7[fat16]
> > > > > > TestUnlink.test_unlink1[fat32]
> > > > > > TestUnlink.test_unlink2[fat32]
> > > > > > TestUnlink.test_unlink3[fat32]
> > > > > > TestUnlink.test_unlink4[fat32]
> > > > > > TestUnlink.test_unlink5[fat32]
> > > > > > TestUnlink.test_unlink6[fat32]
> > > > > > TestUnlink.test_unlink7[fat32]
> > > > > 
> > > > > I appreciate that we get tests for file system functions.
> > > > > 
> > > > > Unfortunately the test output is rudimentary. Can we have something more
> > > > > expressive than unlink1 - unlink7?
> > > > > 
> > > > > CCing Takahiro as he was contributing recently to FAT.
> > > > 
> > > > Sorry, yes, kind of?  I pasted that in for brevity, but it's basically
> > > > that for example all of test/py/tests/test_fs/test_unlink.py fails if
> > > > you fsck the image in question after each test.  If you apply
> > > > https://patchwork.ozlabs.org/patch/1041186/ (to avoid spurious ext4
> > > > failures) and then https://patchwork.ozlabs.org/patch/1041181/ and run
> > > > 'make tests' you'll see the full output.
> > > 
> > > I have no time to dig into this issue right now,
> > > but if you give me a log from fsck, particularly
> > > why fsck failed here, it would help me to understand
> > > the problem.
> > 
> > The raw log can be seen here:
> > https://gist.github.com/trini/b68799ed9880a31a3289e9bea033831d
> 
> Thanks. I can find error messages like:
> Free cluster summary wrong (144636 vs. really 144380)
> 
> So there seems to be a leak in reclaiming freed clusters.

A count of free clusters, along with other info, is kept in an "info sector"
on a file system (only for fat32), but in U-Boot fat, none of information
in that sector is currently maintained. So this error would be inevitable.
I don't know any fsck option to suppress this kind of check.

-Takahiro Akashi

> > > # like the case of ext4, we might have to turn off
> > > # some option at fsck?
> > 
> > Note that for ext4 we're turning off the metadata csum feature of the
> > filesystem as we do not support it.
> 
> I'm afraid that this is not the case.
> 
> -Takahiro Akashi
> 
> > -- 
> > Tom
> 
>