[U-Boot] [PULL] efi patch queue 2018-07-25

Tom Rini trini at konsulko.com
Mon Jul 30 16:13:52 UTC 2018 

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 at 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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180730/fbec45c9/attachment.sig>

More information about the U-Boot mailing list