On Sun, Jul 29, 2018 at 09:42:14AM +0200, Heinrich Schuchardt wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512
On 07/29/2018 03:33 AM, Tom Rini wrote:
On Sat, Jul 28, 2018 at 11:32:56PM +0200, Heinrich Schuchardt wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512
On 07/28/2018 08:33 PM, Tom Rini wrote:
On Sat, Jul 28, 2018 at 07:10:39PM +0200, Heinrich Schuchardt wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512
On 07/28/2018 06:32 PM, Tom Rini wrote:
On Sat, Jul 28, 2018 at 06:21:58PM +0200, Heinrich Schuchardt wrote: > -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 > > On 07/28/2018 06:13 PM, Tom Rini wrote: >> On Sat, Jul 28, 2018 at 06:07:20PM +0200, Heinrich >> Schuchardt wrote: >> >>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 >>> >>> On 07/28/2018 05:55 PM, Tom Rini wrote: >>>> On Wed, Jul 25, 2018 at 03:04:27PM +0200, >>>> Alexander Graf wrote: >>>> >>>>> Hi Tom, >>>>> >>>>> This is my current patch queue for efi. Please >>>>> pull. >>>>> >>>>> Alex >>>>> >>>>> >>>>> The following changes since commit >>>>> 323a73adc9a1bf2de43fe03bdd9c3038ce7c2784: >>>>> >>>>> mtd: nand: add new enum for storing ECC algorithm >>>>> (2018-07-23 14:33:21 -0400) >>>>> >>>>> are available in the git repository at: >>>>> >>>>> git://github.com/agraf/u-boot.git >>>>> tags/signed-efi-next >>>>> >>>>> for you to fetch changes up to >>>>> 0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e: >>>>> >>>>> MAINTAINERS: assign lib/charset.c (2018-07-25 >>>>> 15:00:24 +0200) >>>>> >>>> >>>> NAK, this breaks one of the filesystem tests. >>>> Specifically: commit >>>> 0dc1bfb7302d220a48364263d5632d6d572b069b Author: >>>> Heinrich Schuchardt xypron.glpk@gmx.de Date: >>>> Mon Jul 2 02:41:23 2018 +0200 >>>> >>>> fs: fat: cannot write to subdirectories >>>> >>>> Breaks TC13: 1MB write to ./1MB.file.w2 >>>> >>> >>> Hello Tom, >>> >>> please, provide the link to the Travis log with the >>> failure. >> >> It's actually not in travis. Running >> test/fs/fs-test.sh is annoying to automate: >> FSTST=`./test/fs/fs-test.sh 2>&1 | tail -n 3 | head -n >> 1` echo $FSTST | grep -q "TOTAL PASS: 204 TOTAL FAIL: >> 12" && exit 0 || exit 1 >> >> but I should see if I can get that into .travis.yml. >> > > ./test/fs/fs-test.sh Missing mkfs binary. Exiting! > > You wouldn't run tests as root? Is this test meant to be > run with fakeroot?
It requires sudo to work along with various utilities to make the various filesystems.
Tom please, have a look at the files created by the tests w/o my patch.
This is what the find command returns:
sandbox/test/fs/mnt sandbox/test/fs/mnt/SUBDIR sandbox/test/fs/mnt/2.5GB.file sandbox/test/fs/mnt/1MB.file sandbox/test/fs/mnt/1MB.file.w sandbox/test/fs/mnt/1MB.file.w2 sandbox/test/fs/mnt/./1MB.file.w2
You observe that the last file has an illegal file name (yes, the filename itself is "./1MB.file.w2". It should never have been created.
Without my patch this illegal file is not created.
Why should this be a reason to dismiss my patch?
Ah, OK, thanks for looking. Please submit a patch that updates the tests.
With Takahiro's patch series
fs: fat: extend FAT write operations https://patchwork.ozlabs.org/project/uboot/list/?series=56580 https://lists.denx.de/pipermail/u-boot/2018-July/335683.html
the FAT driver will finally correctly support paths with subdirectories.
With that patch series the created files are:
sandbox/test/fs/mnt sandbox/test/fs/mnt/SUBDIR sandbox/test/fs/mnt/2.5GB.file sandbox/test/fs/mnt/1MB.file sandbox/test/fs/mnt/1MB.file.w sandbox/test/fs/mnt/1MB.file.w2
There is nothing wrong with the TC13 test. After writing it tries to do the verification with (b) and without (c) a relative path. If both subtests are passed the file system is working as expected. And as you already will have observed TC13b and TC13c are not passed without Takahiro's patch series.
Then I guess the answer is an update to fs-test.sh to note that the expected, for now, results should be 200/16 and to make sure that Takahiro's series also updates fs-test.sh results. Thanks!
@Tom: I am not able to reproduce that 200/16 result, I get 189/27. The difference are probably the 11 fails on ext4.
Creating files in ext4 image if not already present. mount: /home/user/u-boot/sandbox/test/fs/mnt: cannot mount; probably corrupted filesystem on /dev/loop0. umount: sandbox/test/fs/mnt: not mounted. rmdir: failed to remove 'sandbox/test/fs/mnt': Directory not empty ** Start sandbox/test/fs/fs-test.fs.ext4.out_clean pass - TC1: ls of 2.5GB.file pass - TC1: ls of 1MB.file pass - TC2: size of 1MB.file pass - TC2: size of 1MB.file via a path using '..' pass - TC3: size of 2.5GB.file pass - TC4: load of 1MB.file size FAIL - TC4: load from 1MB.file pass - TC5: load of 1st MB from 2.5GB.file size FAIL - TC5: load of 1st MB from 2.5GB.file pass - TC6: load of last MB from 2.5GB.file size FAIL - TC6: load of last MB from 2.5GB.file pass - TC7: load of last 1mb chunk of 2GB from 2.5GB.file size FAIL - TC7: load of last 1mb chunk of 2GB from 2.5GB.file pass - TC8: load 1st MB chunk after 2GB from 2.5GB.file size FAIL - TC8: load 1st MB chunk after 2GB from 2.5GB.file pass - TC9: load 1MB chunk crossing 2GB boundary from 2.5GB.file size FAIL - TC9: load 1MB chunk crossing 2GB boundary from 2.5GB.file pass - TC10: load 2MB from the last 1MB of 2.5GB.file loads 1MB FAIL - TC11: 1MB write to 1MB.file.w - write succeeded FAIL - TC11: 1MB write to 1MB.file.w - content verified pass - TC12: 1MB write to . - write denied FAIL - TC13: 1MB write to ./1MB.file.w2 - write succeeded FAIL - TC13: 1MB read from ./1MB.file.w2 - content verified FAIL - TC13: 1MB read from 1MB.file.w2 - content verified ** End sandbox/test/fs/fs-test.fs.ext4.out_clean Summary: PASS: 13 FAIL: 11
When rerunning the test scripts I get "Total Summary: TOTAL PASS: 178 TOTAL FAIL: 38".
I think this test script is a mess:
- The file systems used for testing are not delivered with U-Boot. So
they might differ between host systems. I would prefer gzipped file systems to be delivered with the source. Then on each test run unzip the original file system.
- The test requires running as root. Running software as root is a big
security issue. And it is totally unnecessary. File systems can be mounted in user space using fuse.
- The test misses to clean up what it has changed. It cannot be rerun
without manually deleting the /sandbox directory or you get different results on the 2nd run.
- Each individual test that is expected to fail should not write FAIL
but TODO, so that we know what is an unexpected failure.
- The test should be integrated in Travis.
Yes. It's been a long standing ask if anyone can improve the fs tests as there are various headaches in running them. It is also however what we have today and with a little prep work the expected results can be replicated.
If this is the only patch you are worried about, couldn't you, please, merge the rest of the EFI queue.
I'll send the patch then to update the expected outputs/results for now then.