mbox series

[PULL] efi patch queue 2018-07-25

Message ID 20180725130427.60884-1-agraf@suse.de
State New
Headers show
Series [PULL] efi patch queue 2018-07-25 | expand

Pull-request

git://github.com/agraf/u-boot.git tags/signed-efi-next

Message

Alexander Graf July 25, 2018, 1:04 p.m. UTC
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)

----------------------------------------------------------------
Patch queue for efi - 2018-07-25

Highlights this time:

  - Many small fixes to improve spec compatibility (found by SCT)
  - Almost enough to run with sandbox target
  - GetTime() improvements
  - Enable EFI_LOADER and HYP entry on ARMv7 with NONSEC=y

----------------------------------------------------------------
Alexander Graf (11):
      efi_loader: Use compiler constants for image loader
      efi_loader: Use map_sysmem() in bootefi command
      efi_loader: Allow SMBIOS tables in highmem
      efi_loader: Disable miniapps on sandbox
      efi_loader: Introduce ms abi vararg helpers
      efi_loader: Move to compiler based target architecture determination
      elf: Move x86 reloc defines to common elf.h
      efi_loader: Use common elf.h reloc defines
      efi_loader: Expose U-Boot addresses in memory map for sandbox
      efi_loader: Rename sections to allow for implicit data
      x86: Add efi_loader bits to x86_64 linker script

Heinrich Schuchardt (29):
      efi_selftest: update .gitignore
      efi_loader: efi_allocate_pages is too restrictive
      efi_loader: check parameters of CreateEvent
      efi_loader: check parameters in memory allocation
      efi_loader: check parameters of GetMemoryMap
      efi_loader: check map_key in ExitBootServices
      fs: fat: cannot write to subdirectories
      efi_selftest: test writing to file
      efi_driver: set DM_FLAG_NAME_ALLOCED flag
      efi_loader: set revision in loaded image protocol
      efi_loader: EFI_SIMPLE_TEXT_INPUT_PROTOCOL.Reset()
      efi_loader: clear screen has to reset cursor position
      efi_loader: specify UEFI spec revision
      efi_loader: correct EFI_RUNTIME_SERVICES_SIGNATURE
      efi_loader: correct headersize EFI tables
      efi_loader: provide firmware revision
      efi_loader: calculate crc32 for EFI tables
      efi_loader: allocate configuration table array
      efi_selftest: test InstallConfigurationTable()
      efi_loader: correct signature of CalculateCrc32()
      efi_loader: update crc32 in InstallConfigurationTable
      efi_selftest: check crc32 for InstallConfigurationTable
      efi_selftest: unit test for CalculateCrc32()
      rtc: remove CONFIG_CMD_DATE dependency
      efi_loader: remove unused efi_get_time_init()
      efi_loader: complete implementation of GetTime()
      efi_selftest: support printing leading zeroes
      efi_selftest: unit test for GetTime()
      MAINTAINERS: assign lib/charset.c

Mark Kettenis (5):
      ARM: HYP/non-sec: migrate stack
      efi_loader: ARM: run EFI payloads non-secure
      efi_loader: ARM: don't attempt to enter non-secure mode twice
      ARM: HYP/non-sec: enable ARMV7_LPAE if HYP mode is supported
      Revert "efi_loader: no support for ARMV7_NONSEC=y"

Simon Glass (5):
      efi: sandbox: Adjust memory usage for sandbox
      vsprintf: Handle NULL with %pU
      efi_selftest: Clean up a few comments and messages
      efi: Tidy up device-tree-size calculation in copy_fdt()
      efi: Drop error return in efi_carve_out_dt_rsv()

 MAINTAINERS                                  |   1 +
 arch/arm/config.mk                           |   4 +-
 arch/arm/cpu/armv7/Kconfig                   |   2 +-
 arch/arm/cpu/armv7/nonsec_virt.S             |   2 +
 arch/arm/cpu/armv8/u-boot.lds                |  24 ++-
 arch/arm/cpu/u-boot.lds                      |  36 ++--
 arch/arm/mach-zynq/u-boot.lds                |  36 ++--
 arch/riscv/cpu/ax25/u-boot.lds               |  26 ++-
 arch/sandbox/config.mk                       |   3 +
 arch/sandbox/cpu/u-boot.lds                  |   9 +-
 arch/x86/config.mk                           |   2 +
 arch/x86/cpu/start.S                         |   2 +-
 arch/x86/cpu/start64.S                       |   2 +-
 arch/x86/cpu/u-boot-64.lds                   |  37 +++-
 arch/x86/cpu/u-boot.lds                      |  34 ++--
 arch/x86/include/asm/elf.h                   |  45 -----
 arch/x86/lib/reloc_ia32_efi.c                |   1 -
 arch/x86/lib/reloc_x86_64_efi.c              |   1 -
 board/qualcomm/dragonboard410c/u-boot.lds    |  17 +-
 board/qualcomm/dragonboard820c/u-boot.lds    |  24 ++-
 board/ti/am335x/u-boot.lds                   |  36 ++--
 cmd/bootefi.c                                |  90 +++++++--
 doc/README.uefi                              |   2 -
 drivers/rtc/at91sam9_rtt.c                   |   4 -
 drivers/rtc/davinci.c                        |   2 -
 drivers/rtc/ds1302.c                         |   4 -
 drivers/rtc/ds1306.c                         |   4 -
 drivers/rtc/ds1307.c                         |   4 -
 drivers/rtc/ds1337.c                         |   4 -
 drivers/rtc/ds1374.c                         |   3 -
 drivers/rtc/ds164x.c                         |   4 -
 drivers/rtc/ds174x.c                         |   4 -
 drivers/rtc/ds3231.c                         |   4 -
 drivers/rtc/imxdi.c                          |   4 -
 drivers/rtc/m41t11.c                         |   3 -
 drivers/rtc/m41t60.c                         |   3 -
 drivers/rtc/m41t62.c                         |   4 -
 drivers/rtc/m48t35ax.c                       |   4 -
 drivers/rtc/max6900.c                        |   4 -
 drivers/rtc/mc146818.c                       |   3 -
 drivers/rtc/mcfrtc.c                         |   4 -
 drivers/rtc/mk48t59.c                        |   4 -
 drivers/rtc/pcf8563.c                        |   4 -
 drivers/rtc/rs5c372.c                        |   3 -
 drivers/rtc/rx8025.c                         |   4 -
 drivers/rtc/s3c24x0_rtc.c                    |   4 -
 drivers/rtc/x1205.c                          |   4 -
 fs/fat/fat_write.c                           |  16 +-
 include/efi.h                                |   8 +
 include/efi_api.h                            |  16 +-
 include/efi_loader.h                         |  13 +-
 include/elf.h                                |  35 ++++
 lib/efi_driver/efi_block_device.c            |   2 +
 lib/efi_loader/Kconfig                       |   2 -
 lib/efi_loader/Makefile                      |   3 +
 lib/efi_loader/efi_boottime.c                | 169 +++++++++++------
 lib/efi_loader/efi_console.c                 |   9 +-
 lib/efi_loader/efi_image_loader.c            |  12 +-
 lib/efi_loader/efi_memory.c                  |  65 +++++--
 lib/efi_loader/efi_runtime.c                 |  83 ++++++---
 lib/efi_loader/efi_smbios.c                  |  11 +-
 lib/efi_selftest/.gitignore                  |   4 +-
 lib/efi_selftest/Makefile                    |   5 +-
 lib/efi_selftest/efi_selftest.c              |  14 +-
 lib/efi_selftest/efi_selftest_block_device.c |  70 +++++++
 lib/efi_selftest/efi_selftest_config_table.c | 266 +++++++++++++++++++++++++++
 lib/efi_selftest/efi_selftest_console.c      |  33 ++--
 lib/efi_selftest/efi_selftest_crc32.c        | 141 ++++++++++++++
 lib/efi_selftest/efi_selftest_rtc.c          |  67 +++++++
 lib/vsprintf.c                               |   5 +-
 70 files changed, 1172 insertions(+), 402 deletions(-)
 delete mode 100644 arch/x86/include/asm/elf.h
 create mode 100644 lib/efi_selftest/efi_selftest_config_table.c
 create mode 100644 lib/efi_selftest/efi_selftest_crc32.c
 create mode 100644 lib/efi_selftest/efi_selftest_rtc.c

Comments

Tom Rini July 28, 2018, 3:55 p.m. UTC | #1
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

-- 
Tom
Heinrich Schuchardt July 28, 2018, 4:07 p.m. UTC | #2
-----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.

Best regards

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

iQIzBAEBCgAdFiEEbcT5xx8ppvoGt20zxIHbvCwFGsQFAltclK0ACgkQxIHbvCwF
GsTc9w/8Cqd6WakCF+v2WI95qSG2EF9wl0ruM900n/IeOsh22vCkzBd7L0L3Zqbj
hIiYUVmEfw7Q5DHWfo+nXXqEmjfsmewI9vcMQNYJtUbCeBq+uv0GPDjJupQ8wJyT
B8nnFTWt5jKSFmF+zOdcVa8JPesSD6kmQs0SysVIRb5532rpO0LKjRwCR1uJ4f12
7PRZ1d0Mf4JcY/3RlaaVS480zqo/GRlk7ViwFaq82DDg5MWzjql9AEbYZPb1c+r5
NAyFYEk2X3OPwkRxMvPpmlV5M8xiV58Ht9KnXeqa5aivKsUFBsBoy5Yb4eLQkPAe
H385U0yueZUsKil1cFYvOhkm/Nnj3JMa7eT4LovnvnGF23hu+hp2jSCamskyW06a
IPHdaUDU8Wnog9loHt4yko0nNHLJ4x9Ryi9WvsFE6P6rS/58t3UcNEhLOkR6oCiJ
44hpE1MxvOzVFkIWrw1ze/86ZzMz2Cj5I65bseBMHcJIOnsZhjTpU2VFYax6gWno
mNtSj8OZcwT1dd0XUB3FQofXYpIcgOlc1/cn4fhwQiH8/ZoCRlTUCseSOoa0EOCr
W1QgbbR9GTpYKYV6yEy2RmC4qteWB7PvUyfddiOvaTtHeqkp8R8dWeyAmevQxIh6
RFzsXW7EZA0qa95P/5MtrkMzDKzMoD8rbjv0/9Y08TQ2vsGF1vg=
=iTo4
-----END PGP SIGNATURE-----
Tom Rini July 28, 2018, 4:13 p.m. UTC | #3
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.

-- 
Tom
Heinrich Schuchardt July 28, 2018, 4:21 p.m. UTC | #4
-----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?

Best regards

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

iQIzBAEBCgAdFiEEbcT5xx8ppvoGt20zxIHbvCwFGsQFAltcmB8ACgkQxIHbvCwF
GsRfyg/7ByTTAsNNZOa1jXxqY4Swp+F495ybdjRlUMkiIF+zxgQJLCcLpkVO1HLR
4TP62LTq+gGdnHvEOjQmxVGCGXNjlFCyBDbGL7rB8eLZFIBgJ1343Q1V4V0FURir
HdCpIVFRHUNwMV3DU6iK04QwjldC7MurJG/yJaE6RySrRxdGSNxSP9kYDkDrolU+
BOibDleXhxaGT65Vb7V77dAmkGwGWYhvCD/3AgDxlztGKOCSIW1XotUyWrORQvhf
dUL3VddKaVqQoUc3MZJllc5II/qCdFO8e4jXlPMXvm6iHeqGBdt7oHssylbAZkuS
V+9Ocfi1/qz7c4ZR7MhC7Lqs8zmVBO8gnseLnt4nI3HbrSa4etR1Aqr+YY57id24
stCe7miK71DK7FiBVdgeJqRLiT82pcMZHSRCCW0DlJM70v4SPRZ/rNOyGnQnxSvo
/2SJQz/jVOMhk2SqjT9mjtEfHPjyt1EUllqjQUjEYtMvqCgx2b5sWKQUEkiGiJTB
Ydj+LqnRCBzKUVbS1JVU56jhuHA8zqM67QRaTRgj1MQqkrNEDqj4CHA7a3tol159
OWY2Sv7BcxXozHTVF5l0yImh2w1Bx3ot0XYY5u2L35eEQl60Xyi9TmSrC1l1qv29
PHVYLwynvecQP56RbqHpAws1TuiIPWpnW5Rc66/Y1j8hplD2F6w=
=g8NU
-----END PGP SIGNATURE-----
Tom Rini July 28, 2018, 4:32 p.m. UTC | #5
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
Heinrich Schuchardt July 28, 2018, 5:10 p.m. UTC | #6
-----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?

Best regards

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

iQIzBAEBCgAdFiEEbcT5xx8ppvoGt20zxIHbvCwFGsQFAltco4YACgkQxIHbvCwF
GsSTxhAAhPbNHg16YZg7DZE6pMPZ5v64eVrAPPiM6LoIb8rXyLtKP9lDh/V1tk33
t7nHRfPfS5Ow3KYjneBxoxnw3A4uxU4Rf16xb5KZX9UvLPUusumwjaRUW9/FytiQ
MkZeZQGdtAHLy9cFL3sRkIM4Vz3kWo5XBNgYjJaYQAN0/BlWt6oknTUKAYrciGCz
O+B7i6Vf7hXj1d1RdFjLfS4sinSibjUMd1Bn5HoAZvjLo3Gfmi+e/ZIPXKb75QfP
fQwH6oGuEOL6WqL+OyS5CVXiBeGbOtrJoH7o5C8TEixcPwp2cKGP7p/L8UBIOBbM
a/zXIgumFZhAyDhRdLcvevvUxfBRX4opCgGYk8aG8/QlpnoZO7eeQJxKU7+t/JxY
UCRQNZXzh6WbTDLkRkP5u3vIA0qGqpGNkH9D7lGU8W9Iq9awkr3gtLls1HGuyGI+
zdYpLeC9JYYRPPZSG/Guy58NphuYRG3vCj6D8tvLqMzE3dYCIBwxobWESV26s40h
do3oqPTpmFYWJNJG4C1x0H3JfMW32X5ibrk8Hn6AFvHn4DYsdD4c24Ofol9IcIqf
nbgsYwrf5rdyiqgK5MUVXoFztNkPot6BSI+sU/ft3CcxX+0/cpSYiZ3M3QOk8BGt
J5Q7/57oBxcLrZrlEmE9PE+rmGDpw6F60DqZpGo5EwyzZmb4nKM=
=V/cr
-----END PGP SIGNATURE-----
Tom Rini July 28, 2018, 6:33 p.m. UTC | #7
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.

-- 
Tom
Heinrich Schuchardt July 28, 2018, 9:32 p.m. UTC | #8
-----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.

Best regards

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

iQIzBAEBCgAdFiEEbcT5xx8ppvoGt20zxIHbvCwFGsQFAltc4P8ACgkQxIHbvCwF
GsSfXg/9G6RqPbMGb3wl3SOZSnPM2VzTv9tvnnpqZq8Bmaao4I7sgnI3w/Ts6r/k
iBU0c4iOrcytOiVWiQU9FPJ3L8zKOMqfK7vE9186NUZFh1u10sJrmLfESV6pbKF8
+pxKy6xM2y8F7AWE0VhgAf7lxFbA2tnhFAQPbP687mJaFKYKG48CQ0r7GwXO/iEl
YvE5KflSOEIuTJ6VD7sVuOdEWw8v9yXZTevxcjYSMQppvGg2JIpILtUc89aMlNNC
rFTesESdkqcm5V8NUaaMLv404QiW1z93ya+za7IwoUiXAlxYFuXCWJtOqWXxz4Mu
EbGOXTYXUq0xXqNJDZDjtwzOQilUANvFGAwXUJc1F6q3F0XJNLOStC7Vr6SrBjij
ibiJAj1XOgEbNip6m1yDB0ycyCjKjASr+l6RrfWzFli1AiQon4Fk6P38Rb27RAiU
DGTMKp6gqhj1tTLaNQTEzwdgcr19GvRfJQpfDYkZX0ujESETwMCyO+1i4VuM7bJo
v2h7C+1m7uk7qVDbwJAtTS0YiL7k7ZqyFv55AmGtRH0ZNVNIgIFByfXYchukrHqo
q89l1mYtMba05ZZ4JVp/21AcEFH94WbtlPvkizXhpcxeH2ZpGR1GhmZEXNJ/a9uP
d9eDngVapglWzJCya+ojtMh+RZi4Rn01Y3Qv45PfRptmzxJn2CI=
=aYo7
-----END PGP SIGNATURE-----
Tom Rini July 29, 2018, 1:33 a.m. UTC | #9
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
Heinrich Schuchardt July 29, 2018, 7:42 a.m. UTC | #10
-----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.

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-----
Simon Glass July 30, 2018, 4:05 p.m. UTC | #11
Hi,

On 28 July 2018 at 09:55, Tom Rini <trini@konsulko.com> 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

This is getting a little painful for me :-)

The EFI tree contains a few of the patches needed to make sandbox
support EFI, but not all. A rebase on this tree provides me with a
segfault, so it needs more work. But we are right at RC1.

Looking at the state of things it is clear that I cannot just pull in
my previously applied patches since there are various changes. I'm
going to have to send a new set of patches.

Perhaps as a way forward we could drop this one patch and apply to
master? Then I can do my side of things while the patch is worked on?

Regards,
Simon
Tom Rini July 30, 2018, 4:13 p.m. UTC | #12
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.

-- 
Tom
Alexander Graf July 30, 2018, 4:14 p.m. UTC | #13
On 07/30/2018 06:05 PM, Simon Glass wrote:
> Hi,
>
> On 28 July 2018 at 09:55, Tom Rini <trini@konsulko.com> 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
> This is getting a little painful for me :-)
>
> The EFI tree contains a few of the patches needed to make sandbox
> support EFI, but not all. A rebase on this tree provides me with a
> segfault, so it needs more work. But we are right at RC1.
>
> Looking at the state of things it is clear that I cannot just pull in
> my previously applied patches since there are various changes. I'm
> going to have to send a new set of patches.
>
> Perhaps as a way forward we could drop this one patch and apply to
> master? Then I can do my side of things while the patch is worked on?

What is really missing to just make the test pass? All we need to do is 
adapt the fail/success rate of the fat test, right?

How do I quickly run the test to cook up a patch?


Alex
Tom Rini July 30, 2018, 4:16 p.m. UTC | #14
On Mon, Jul 30, 2018 at 06:14:16PM +0200, Alexander Graf wrote:
> On 07/30/2018 06:05 PM, Simon Glass wrote:

> >Hi,

> >

> >On 28 July 2018 at 09:55, Tom Rini <trini@konsulko.com> 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

> >This is getting a little painful for me :-)

> >

> >The EFI tree contains a few of the patches needed to make sandbox

> >support EFI, but not all. A rebase on this tree provides me with a

> >segfault, so it needs more work. But we are right at RC1.

> >

> >Looking at the state of things it is clear that I cannot just pull in

> >my previously applied patches since there are various changes. I'm

> >going to have to send a new set of patches.

> >

> >Perhaps as a way forward we could drop this one patch and apply to

> >master? Then I can do my side of things while the patch is worked on?

> 

> What is really missing to just make the test pass? All we need to do is

> adapt the fail/success rate of the fat test, right?

> 

> How do I quickly run the test to cook up a patch?


Run ./test/fs/fs-test.sh and see what you get for totals, as the
starting point.  And then you can also join in on cringing at how bad
our current filesystem related tests are :)

-- 
Tom
Tom Rini July 30, 2018, 11:56 p.m. UTC | #15
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.


OK, I'm coming back in here.  The log file from running the test is
sandbox/test/fs/fs-test.fs.fat32.out_clean and if we look for TC13, this
is what we see:
=> # Write it via "same directory", i.e. "." dirent
=> # Test Case 13a - Check directory traversal
=> save host 0:0 0x01000008 ./1MB.file.w2 $filesize
FAT: illegal filename (./1MB.file.w2)

So the change is leading to us not being able to write "./1MB.file.w2"
at all, rather than (as the test was intended to catch!) writing
"1MB.file.w2" to ".".  Yes, the current code is wrong as it writes the
illegal file "./1MB.file.w2" but the new behavior is wrong too.  I am
going to take the EFI PR as-is and I'm going to push a commit that
updates the expected results and explains why.  I'm going to leave it to
you and Takahiro-san's series to address the problem and allow for
"./file" to write "file" to "." as intended.

-- 
Tom
Tom Rini July 31, 2018, 1:36 a.m. UTC | #16
On Sat, Jul 28, 2018 at 11:55:08AM -0400, 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


I've applied this PR along with a patch that changes the expected
results of fs-test.sh as we've changed from writing an illegal file to
not allowing a valid path + file to be written, which is a step in the
right general direction at least.

-- 
Tom
Alexander Graf July 31, 2018, 8:19 a.m. UTC | #17
On 07/31/2018 03:36 AM, Tom Rini wrote:
> On Sat, Jul 28, 2018 at 11:55:08AM -0400, 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
> I've applied this PR along with a patch that changes the expected
> results of fs-test.sh as we've changed from writing an illegal file to
> not allowing a valid path + file to be written, which is a step in the
> right general direction at least.

Thanks :)


Alex
Simon Glass Aug. 7, 2018, 5:12 p.m. UTC | #18
Hi Tom,

On 31 July 2018 at 02:19, Alexander Graf <agraf@suse.de> wrote:
> On 07/31/2018 03:36 AM, Tom Rini wrote:
>>
>> On Sat, Jul 28, 2018 at 11:55:08AM -0400, 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
>>
>> I've applied this PR along with a patch that changes the expected
>> results of fs-test.sh as we've changed from writing an illegal file to
>> not allowing a valid path + file to be written, which is a step in the
>> right general direction at least.
>
>
> Thanks :)

Thanks Tom.

I'm going to have to send a v9 series for sandbox on EFI since there
are various changes. I'll include a partial revert as well since there
is a sandbox change that I don't think we want.

Once people have had a change to review it, we can see whether we can
get sandbox EFI support into this release. But it might be too late.

Regards,
Simon
Tom Rini Aug. 7, 2018, 5:41 p.m. UTC | #19
On Tue, Aug 07, 2018 at 11:12:31AM -0600, Simon Glass wrote:
> Hi Tom,

> 

> On 31 July 2018 at 02:19, Alexander Graf <agraf@suse.de> wrote:

> > On 07/31/2018 03:36 AM, Tom Rini wrote:

> >>

> >> On Sat, Jul 28, 2018 at 11:55:08AM -0400, 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

> >>

> >> I've applied this PR along with a patch that changes the expected

> >> results of fs-test.sh as we've changed from writing an illegal file to

> >> not allowing a valid path + file to be written, which is a step in the

> >> right general direction at least.

> >

> >

> > Thanks :)

> 

> Thanks Tom.

> 

> I'm going to have to send a v9 series for sandbox on EFI since there

> are various changes. I'll include a partial revert as well since there

> is a sandbox change that I don't think we want.

> 

> Once people have had a change to review it, we can see whether we can

> get sandbox EFI support into this release. But it might be too late.


OK, thanks for the update!

-- 
Tom