Message ID | 20190213111527.1525-4-jjhiblot@ti.com |
---|---|
State | Accepted |
Delegated to: | Tom Rini |
Headers | show |
Series | Add support for symlink creation in EXT4 | expand |
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@ti.com> > Reviewed-by: Tom Rini <trini@konsulko.com>
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@ti.com> > Reviewed-by: Tom Rini <trini@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] So... I'm inclined to say that to start with, I bring this patch in and then disable FAT fsck (as I cannot see how to mark these as xfail with a comment that we need to fix them, only for FAT). But we should get these FAT problems fixed.
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@ti.com> >> Reviewed-by: Tom Rini <trini@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. Best regards Heinrich > > So... I'm inclined to say that to start with, I bring this patch in and > then disable FAT fsck (as I cannot see how to mark these as xfail with > a comment that we need to fix them, only for FAT). But we should get > these FAT problems fixed. >
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@ti.com> > >> Reviewed-by: Tom Rini <trini@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.
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@ti.com> > > >> Reviewed-by: Tom Rini <trini@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. # like the case of ext4, we might have to turn off # some option at fsck? Thanks, -Takahiro Akashi > -- > Tom
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@ti.com> > > > >> Reviewed-by: Tom Rini <trini@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 > # 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.
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@ti.com> > > > > >> Reviewed-by: Tom Rini <trini@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. > > # 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
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@ti.com> > > > > > >> Reviewed-by: Tom Rini <trini@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 > >
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@ti.com> > Reviewed-by: Tom Rini <trini@konsulko.com> Applied to u-boot/master, thanks!
diff --git a/test/py/tests/test_fs/fstest_helpers.py b/test/py/tests/test_fs/fstest_helpers.py new file mode 100644 index 0000000000..637bc30a91 --- /dev/null +++ b/test/py/tests/test_fs/fstest_helpers.py @@ -0,0 +1,15 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2019, Texas Instrument +# Author: JJ Hiblot <jjhiblot@ti.com> +# + +from subprocess import check_call, CalledProcessError + +def assert_fs_integrity(fs_type, fs_img): + try: + if fs_type == 'ext4': + check_call('fsck.ext4 -n -f %s' % fs_img, shell=True) + if fs_type == 'fat': + check_call('fsck.fat -n %s' % fs_img, shell=True) + except CalledProcessError: + raise diff --git a/test/py/tests/test_fs/test_basic.py b/test/py/tests/test_fs/test_basic.py index 140ca29ac7..71f3e86fb1 100644 --- a/test/py/tests/test_fs/test_basic.py +++ b/test/py/tests/test_fs/test_basic.py @@ -11,6 +11,7 @@ This test verifies basic read/write operation on file system. import pytest import re from fstest_defs import * +from fstest_helpers import assert_fs_integrity @pytest.mark.boardspec('sandbox') @pytest.mark.slow @@ -237,6 +238,7 @@ class TestFsBasic(object): 'md5sum %x $filesize' % ADDR, 'setenv filesize']) assert(md5val[0] in ''.join(output)) + assert_fs_integrity(fs_type, fs_img) def test_fs12(self, u_boot_console, fs_obj_basic): """ @@ -252,6 +254,7 @@ class TestFsBasic(object): 'host bind 0 %s' % fs_img, '%swrite host 0:0 %x /. 0x10' % (fs_type, ADDR)]) assert('Unable to write' in ''.join(output)) + assert_fs_integrity(fs_type, fs_img) def test_fs13(self, u_boot_console, fs_obj_basic): """ @@ -286,3 +289,4 @@ class TestFsBasic(object): 'md5sum %x $filesize' % ADDR, 'setenv filesize']) assert(md5val[0] in ''.join(output)) + assert_fs_integrity(fs_type, fs_img) diff --git a/test/py/tests/test_fs/test_ext.py b/test/py/tests/test_fs/test_ext.py index 06cad5516d..2c47738b8d 100644 --- a/test/py/tests/test_fs/test_ext.py +++ b/test/py/tests/test_fs/test_ext.py @@ -11,6 +11,7 @@ This test verifies extended write operation on file system. import pytest import re from fstest_defs import * +from fstest_helpers import assert_fs_integrity @pytest.mark.boardspec('sandbox') @pytest.mark.slow @@ -36,6 +37,7 @@ class TestFsExt(object): 'md5sum %x $filesize' % ADDR, 'setenv filesize']) assert(md5val[0] in ''.join(output)) + assert_fs_integrity(fs_type, fs_img) def test_fs_ext2(self, u_boot_console, fs_obj_ext): """ @@ -58,6 +60,7 @@ class TestFsExt(object): 'md5sum %x $filesize' % ADDR, 'setenv filesize']) assert(md5val[0] in ''.join(output)) + assert_fs_integrity(fs_type, fs_img) def test_fs_ext3(self, u_boot_console, fs_obj_ext): """ @@ -72,6 +75,7 @@ class TestFsExt(object): '%swrite host 0:0 %x /dir1/none/%s.w3 $filesize' % (fs_type, ADDR, MIN_FILE)]) assert('Unable to write "/dir1/none/' in ''.join(output)) + assert_fs_integrity(fs_type, fs_img) def test_fs_ext4(self, u_boot_console, fs_obj_ext): """ @@ -104,6 +108,7 @@ class TestFsExt(object): 'md5sum %x $filesize' % ADDR, 'setenv filesize']) assert(md5val[1] in ''.join(output)) + assert_fs_integrity(fs_type, fs_img) def test_fs_ext5(self, u_boot_console, fs_obj_ext): """ @@ -136,6 +141,7 @@ class TestFsExt(object): 'md5sum %x $filesize' % ADDR, 'setenv filesize']) assert(md5val[2] in ''.join(output)) + assert_fs_integrity(fs_type, fs_img) def test_fs_ext6(self, u_boot_console, fs_obj_ext): """ @@ -160,6 +166,7 @@ class TestFsExt(object): 'printenv filesize', 'setenv filesize']) assert('filesize=0' in ''.join(output)) + assert_fs_integrity(fs_type, fs_img) def test_fs_ext7(self, u_boot_console, fs_obj_ext): """ @@ -192,6 +199,7 @@ class TestFsExt(object): 'md5sum %x $filesize' % ADDR, 'setenv filesize']) assert(md5val[3] in ''.join(output)) + assert_fs_integrity(fs_type, fs_img) def test_fs_ext8(self, u_boot_console, fs_obj_ext): """ @@ -209,6 +217,7 @@ class TestFsExt(object): '%swrite host 0:0 %x /dir1/%s.w8 0x1400 %x' % (fs_type, ADDR, MIN_FILE, 0x100000 + 0x1400)) assert('Unable to write "/dir1' in output) + assert_fs_integrity(fs_type, fs_img) def test_fs_ext9(self, u_boot_console, fs_obj_ext): """ @@ -223,3 +232,4 @@ class TestFsExt(object): '%swrite host 0:0 %x /dir1/%s.w9 0x1400 0x1400' % (fs_type, ADDR, MIN_FILE)]) assert('Unable to write "/dir1' in ''.join(output)) + assert_fs_integrity(fs_type, fs_img) diff --git a/test/py/tests/test_fs/test_mkdir.py b/test/py/tests/test_fs/test_mkdir.py index b3fe11cf3b..fa9561ec35 100644 --- a/test/py/tests/test_fs/test_mkdir.py +++ b/test/py/tests/test_fs/test_mkdir.py @@ -9,6 +9,7 @@ This test verifies mkdir operation on file system. """ import pytest +from fstest_helpers import assert_fs_integrity @pytest.mark.boardspec('sandbox') @pytest.mark.slow @@ -29,6 +30,8 @@ class TestMkdir(object): '%sls host 0:0 dir1' % fs_type) assert('./' in output) assert('../' in output) + assert_fs_integrity(fs_type, fs_img) + def test_mkdir2(self, u_boot_console, fs_obj_mkdir): """ @@ -46,6 +49,7 @@ class TestMkdir(object): '%sls host 0:0 dir1/dir2' % fs_type) assert('./' in output) assert('../' in output) + assert_fs_integrity(fs_type, fs_img) def test_mkdir3(self, u_boot_console, fs_obj_mkdir): """ @@ -58,6 +62,7 @@ class TestMkdir(object): 'host bind 0 %s' % fs_img, '%smkdir host 0:0 none/dir3' % fs_type]) assert('Unable to create a directory' in ''.join(output)) + assert_fs_integrity(fs_type, fs_img) def test_mkdir4(self, u_boot_console, fs_obj_mkdir): """ @@ -69,6 +74,7 @@ class TestMkdir(object): 'host bind 0 %s' % fs_img, '%smkdir host 0:0 .' % fs_type]) assert('Unable to create a directory' in ''.join(output)) + assert_fs_integrity(fs_type, fs_img) def test_mkdir5(self, u_boot_console, fs_obj_mkdir): """ @@ -80,6 +86,7 @@ class TestMkdir(object): 'host bind 0 %s' % fs_img, '%smkdir host 0:0 ..' % fs_type]) assert('Unable to create a directory' in ''.join(output)) + assert_fs_integrity(fs_type, fs_img) def test_mkdir6(self, u_boot_console, fs_obj_mkdir): """ @@ -111,3 +118,4 @@ class TestMkdir(object): '%sls host 0:0 dir6/0123456789abcdef13/..' % fs_type) assert('0123456789abcdef00/' in output) assert('0123456789abcdef13/' in output) + assert_fs_integrity(fs_type, fs_img) diff --git a/test/py/tests/test_fs/test_unlink.py b/test/py/tests/test_fs/test_unlink.py index 2b817468ed..97aafc63bb 100644 --- a/test/py/tests/test_fs/test_unlink.py +++ b/test/py/tests/test_fs/test_unlink.py @@ -10,6 +10,7 @@ on file system. """ import pytest +from fstest_helpers import assert_fs_integrity @pytest.mark.boardspec('sandbox') @pytest.mark.slow @@ -30,6 +31,7 @@ class TestUnlink(object): '%sls host 0:0 dir1/' % fs_type) assert(not 'file1' in output) assert('file2' in output) + assert_fs_integrity(fs_type, fs_img) def test_unlink2(self, u_boot_console, fs_obj_unlink): """ @@ -48,6 +50,7 @@ class TestUnlink(object): output = u_boot_console.run_command( '%sls host 0:0 dir2' % fs_type) assert('0 file(s), 2 dir(s)' in output) + assert_fs_integrity(fs_type, fs_img) def test_unlink3(self, u_boot_console, fs_obj_unlink): """ @@ -59,6 +62,7 @@ class TestUnlink(object): 'host bind 0 %s' % fs_img, '%srm host 0:0 dir1/nofile' % fs_type]) assert('nofile: doesn\'t exist' in ''.join(output)) + assert_fs_integrity(fs_type, fs_img) def test_unlink4(self, u_boot_console, fs_obj_unlink): """ @@ -71,9 +75,10 @@ class TestUnlink(object): '%srm host 0:0 dir4' % fs_type]) assert('' == ''.join(output)) - output = u_boot_console.run_command( - '%sls host 0:0 /' % fs_type) - assert(not 'dir4' in output) + output = u_boot_console.run_command( + '%sls host 0:0 /' % fs_type) + assert(not 'dir4' in output) + assert_fs_integrity(fs_type, fs_img) def test_unlink5(self, u_boot_console, fs_obj_unlink): """ @@ -86,6 +91,7 @@ class TestUnlink(object): 'host bind 0 %s' % fs_img, '%srm host 0:0 dir5' % fs_type]) assert('directory is not empty' in ''.join(output)) + assert_fs_integrity(fs_type, fs_img) def test_unlink6(self, u_boot_console, fs_obj_unlink): """ @@ -97,6 +103,7 @@ class TestUnlink(object): 'host bind 0 %s' % fs_img, '%srm host 0:0 dir5/.' % fs_type]) assert('directory is not empty' in ''.join(output)) + assert_fs_integrity(fs_type, fs_img) def test_unlink7(self, u_boot_console, fs_obj_unlink): """ @@ -108,3 +115,4 @@ class TestUnlink(object): 'host bind 0 %s' % fs_img, '%srm host 0:0 dir5/..' % fs_type]) assert('directory is not empty' in ''.join(output)) + assert_fs_integrity(fs_type, fs_img)
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@ti.com> --- Changes in v5: - Added filesystem integrity checks Changes in v4: None Changes in v3: None Changes in v2: None test/py/tests/test_fs/fstest_helpers.py | 15 +++++++++++++++ test/py/tests/test_fs/test_basic.py | 4 ++++ test/py/tests/test_fs/test_ext.py | 10 ++++++++++ test/py/tests/test_fs/test_mkdir.py | 8 ++++++++ test/py/tests/test_fs/test_unlink.py | 14 +++++++++++--- 5 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 test/py/tests/test_fs/fstest_helpers.py