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

Sun Jul 29 07:42:14 UTC 2018

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

If this is the only patch you are worried about, couldn't you, please,
merge the rest of the EFI queue.

Best regards

Heinrich
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEbcT5xx8ppvoGt20zxIHbvCwFGsQFAltdb84ACgkQxIHbvCwF
GsQLdA/8Cxi0wuX6/CK/JiulQyBBPWhLYmQxHxJQs6es6JSm7mbq9RDddc4apsJG
XKembfUCrRJ6hyd653mWCaSknJewEp0z8EqEh0XinwCjL7nY0ZDbJdJmZu5nGxSc
3OIivCd3H7PQRDFDqn1zVm7dw+HMWEzLsXu3jsuyAClgOsnwfdzsQm0Bv1sfLoaD
TY/AFNJSe3XKFoMf2TOq4ROwEUSgbFJC+PIgTho++wqwtgHwgNv4yuaJo3Ces7p4
1hfaYmQRdWAhG25CrjiHmSHW7vypqYfuaCKryPgqBFY8qQqpYJHRiYHyZySYpJam
+PW1+qTrj8iZwRBx5mZyErzufrT50b2LOGsuVN9yfzVCht3jtHLYdF/UNlzyFv/v
tlZ+HWL+/Skn3lQ0Syhb2HyJMnFD9DyDU05rE/kyzmr1rVY8mCKmKQ39P9pn1/j3
mjex5cgVwQhpXsKv1hO7mZF55ZZxaQjp3OzrQq/84cjeCIAhILG+3lz7IgGcuOzk
AcZ01OndTYuNubaSJ/CwY+ipOTndiC8SDUhA4aUxIdsdmOKHpyGRFr38dzppcz/i
wxXDvBdSlQsWuqU0mFJIRsuiWftJwbV4KOB18v2OqvegQ7naj9Pl8DqX7yA1nSvz
xGxQrlNo/7Jmp5T6IdOGF12ioKbglok8h7XwRRYE1pupfk/XHkE=
=lS4a
-----END PGP SIGNATURE-----