Re: [U-Boot] [PATCH 0/8] fs/fat: cleanups + readdir implementation

[Tom Rini]

On Mon, Aug 14, 2017 at 08:48:47AM -0400, Rob Clark wrote:
> On Sun, Aug 13, 2017 at 5:59 PM, Tom Rini  wrote:
> > On Sun, Aug 13, 2017 at 05:50:39PM -0400, Rob Clark wrote:
> >> On Sun, Aug 13, 2017 at 5:13 PM, Tom Rini  wrote:
> >> > On Sun, Aug 13, 2017 at 06:45:20AM -0400, Rob Clark wrote:
> >> >
> >> >> Introduce directory traversal iterators, and implement fs_readdir()
> >> >> which is needed by EFI_LOADER.
> >> >>
> >> >> The part re-working fat.c to use the directory iterators itself is
> >> >> nearly a 2:1 negative diffstat, and a pretty big cleanup.  I fixed
> >> >> one or two other small issues along the way.  It hasn't really been
> >> >> tested with a wide variaty of different fat filesystems (if someone
> >> >> has a collection of disk images to test with somewhere, let me know),
> >> >> but it seems at least not worse than what it is replacing.
> >> >
> >> > Did you run test/fs/fs-test.sh and confirm there's at least no
> >> > regressions?  Thanks!
> >>
> >> I've been having trouble getting fs-test.sh to pass even without my
> >> changes, so no..
> >
> > Pass, or provide expected results?  From the script, we expect:
> > # Total Summary: TOTAL PASS: 132 TOTAL FAIL: 6
> 
> Ok, I was missing special case handling for the "partition is whole
> disk" case (which I *guess* is only a sandbox thing?)..

FWIW, nope, I believe some atmel boards utilize whole disk as FAT, IIRC,
from when I was trying to come up with a good default value for the
ENV_IS_IN_FAT Kconfig options.

[snip]
> fixed (and excluding the ext4 tests which have unrelated problems on
> my setup):
> 
> Total Summary: TOTAL PASS: 63 TOTAL FAIL: 6
> 
> So if the 6 failed fat tests (below) are "normal", then I guess we're
> good.. I'll post a v2 shortly.

Great, thanks!

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 0/8] fs/fat: cleanups + readdir implementation

[Rob Clark]

On Sun, Aug 13, 2017 at 5:59 PM, Tom Rini  wrote:
> On Sun, Aug 13, 2017 at 05:50:39PM -0400, Rob Clark wrote:
>> On Sun, Aug 13, 2017 at 5:13 PM, Tom Rini  wrote:
>> > On Sun, Aug 13, 2017 at 06:45:20AM -0400, Rob Clark wrote:
>> >
>> >> Introduce directory traversal iterators, and implement fs_readdir()
>> >> which is needed by EFI_LOADER.
>> >>
>> >> The part re-working fat.c to use the directory iterators itself is
>> >> nearly a 2:1 negative diffstat, and a pretty big cleanup.  I fixed
>> >> one or two other small issues along the way.  It hasn't really been
>> >> tested with a wide variaty of different fat filesystems (if someone
>> >> has a collection of disk images to test with somewhere, let me know),
>> >> but it seems at least not worse than what it is replacing.
>> >
>> > Did you run test/fs/fs-test.sh and confirm there's at least no
>> > regressions?  Thanks!
>>
>> I've been having trouble getting fs-test.sh to pass even without my
>> changes, so no..
>
> Pass, or provide expected results?  From the script, we expect:
> # Total Summary: TOTAL PASS: 132 TOTAL FAIL: 6

Ok, I was missing special case handling for the "partition is whole
disk" case (which I *guess* is only a sandbox thing?).. but with that
fixed (and excluding the ext4 tests which have unrelated problems on
my setup):

Total Summary: TOTAL PASS: 63 TOTAL FAIL: 6

So if the 6 failed fat tests (below) are "normal", then I guess we're
good.. I'll post a v2 shortly.

** Start sandbox/test/fs/fs-test.nonfs.fat.out_clean
FAIL - TC12: 1MB write to . - write denied
FAIL - TC13: 1MB read from ./1MB.file.w2 - content verified
FAIL - TC13: 1MB read from 1MB.file.w2 - content verified

** Start sandbox/test/fs/fs-test.fs.fat.out_clean
FAIL - TC12: 1MB write to . - write denied
FAIL - TC13: 1MB read from ./1MB.file.w2 - content verified
FAIL - TC13: 1MB read from 1MB.file.w2 - content verified


BR,
-R

>> Not entirely sure if I have newer kernel/etc compared to the last
>> person who ran fs-test.sh, or if there is something on u-boot side
>> which changed without corresponding change in fs-test.sh.
>
> Note that this script it part of my
> everything-my-lab-isn't-giving-me-fits loop.  I'm probably going to
> split that loop into pure-SW and HW, as the HW part is what's driving me
> crazy most of the time.
>
> --
> Tom
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 0/8] fs/fat: cleanups + readdir implementation

[Tom Rini]

On Sun, Aug 13, 2017 at 05:50:39PM -0400, Rob Clark wrote:
> On Sun, Aug 13, 2017 at 5:13 PM, Tom Rini  wrote:
> > On Sun, Aug 13, 2017 at 06:45:20AM -0400, Rob Clark wrote:
> >
> >> Introduce directory traversal iterators, and implement fs_readdir()
> >> which is needed by EFI_LOADER.
> >>
> >> The part re-working fat.c to use the directory iterators itself is
> >> nearly a 2:1 negative diffstat, and a pretty big cleanup.  I fixed
> >> one or two other small issues along the way.  It hasn't really been
> >> tested with a wide variaty of different fat filesystems (if someone
> >> has a collection of disk images to test with somewhere, let me know),
> >> but it seems at least not worse than what it is replacing.
> >
> > Did you run test/fs/fs-test.sh and confirm there's at least no
> > regressions?  Thanks!
> 
> I've been having trouble getting fs-test.sh to pass even without my
> changes, so no..

Pass, or provide expected results?  From the script, we expect:
# Total Summary: TOTAL PASS: 132 TOTAL FAIL: 6

> Not entirely sure if I have newer kernel/etc compared to the last
> person who ran fs-test.sh, or if there is something on u-boot side
> which changed without corresponding change in fs-test.sh.

Note that this script it part of my
everything-my-lab-isn't-giving-me-fits loop.  I'm probably going to
split that loop into pure-SW and HW, as the HW part is what's driving me
crazy most of the time.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 0/8] fs/fat: cleanups + readdir implementation

[Rob Clark]

On Sun, Aug 13, 2017 at 5:13 PM, Tom Rini  wrote:
> On Sun, Aug 13, 2017 at 06:45:20AM -0400, Rob Clark wrote:
>
>> Introduce directory traversal iterators, and implement fs_readdir()
>> which is needed by EFI_LOADER.
>>
>> The part re-working fat.c to use the directory iterators itself is
>> nearly a 2:1 negative diffstat, and a pretty big cleanup.  I fixed
>> one or two other small issues along the way.  It hasn't really been
>> tested with a wide variaty of different fat filesystems (if someone
>> has a collection of disk images to test with somewhere, let me know),
>> but it seems at least not worse than what it is replacing.
>
> Did you run test/fs/fs-test.sh and confirm there's at least no
> regressions?  Thanks!

I've been having trouble getting fs-test.sh to pass even without my
changes, so no..

Not entirely sure if I have newer kernel/etc compared to the last
person who ran fs-test.sh, or if there is something on u-boot side
which changed without corresponding change in fs-test.sh.

BR,
-R
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 0/8] fs/fat: cleanups + readdir implementation

[Tom Rini]

On Sun, Aug 13, 2017 at 06:45:20AM -0400, Rob Clark wrote:

> Introduce directory traversal iterators, and implement fs_readdir()
> which is needed by EFI_LOADER.
> 
> The part re-working fat.c to use the directory iterators itself is
> nearly a 2:1 negative diffstat, and a pretty big cleanup.  I fixed
> one or two other small issues along the way.  It hasn't really been
> tested with a wide variaty of different fat filesystems (if someone
> has a collection of disk images to test with somewhere, let me know),
> but it seems at least not worse than what it is replacing.

Did you run test/fs/fs-test.sh and confirm there's at least no
regressions?  Thanks!

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 0/8] fs/fat: cleanups + readdir implementation

[Rob Clark]

On Sun, Aug 13, 2017 at 7:28 AM, Heinrich Schuchardt
 wrote:
> On 08/13/2017 12:45 PM, Rob Clark wrote:
>> Introduce directory traversal iterators, and implement fs_readdir()
>> which is needed by EFI_LOADER.
>>
>> The part re-working fat.c to use the directory iterators itself is
>> nearly a 2:1 negative diffstat, and a pretty big cleanup.  I fixed
>> one or two other small issues along the way.  It hasn't really been
>> tested with a wide variaty of different fat filesystems (if someone
>> has a collection of disk images to test with somewhere, let me know),
>> but it seems at least not worse than what it is replacing.
>>
>> Rob Clark (8):
>>   fs/fat: split out helper to init fsdata
>>   fs/fat: introduce new director iterators
>>   fat/fs: convert to directory iterators
>>   fs: add fs_readdir()
>>   fs/fat: implement opendir/readdir/closedir
>>   fat/fs: remove a bunch of dead code
>>   fat/fs: move ls to generic implementation
>>   fs/fat: fix case for FAT shortnames
>>
>>  fs/fat/Makefile|4 -
>>  fs/fat/fat.c   | 1019 
>> +---
>>  fs/fat/fat_write.c |4 +-
>>  fs/fat/file.c  |  183 --
>>  fs/fs.c|  119 +-
>>  include/fat.h  |   35 +-
>>  include/fs.h   |   55 +++
>>  7 files changed, 590 insertions(+), 829 deletions(-)
>>  delete mode 100644 fs/fat/file.c
>>
> Hello Rob,
>
> I assume this patch series is meant to be merged before
> [PATCH v1 00/15] enough UEFI for standard distro boot
> and partially replaces it. Did I get it right?
>

correct, the branch ordering goes:  efi-next -> vsprintf -> readdir ->
enough-uefi-for-shim-2

You can find the latest version of the latter, w/ efi_file re-worked
for updated fs_readdir API, on my github tree.  I'll be re-sending it
soon but still have a few review comments to address first.

BR,
-R
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 0/8] fs/fat: cleanups + readdir implementation

[Heinrich Schuchardt]

On 08/13/2017 12:45 PM, Rob Clark wrote:
> Introduce directory traversal iterators, and implement fs_readdir()
> which is needed by EFI_LOADER.
> 
> The part re-working fat.c to use the directory iterators itself is
> nearly a 2:1 negative diffstat, and a pretty big cleanup.  I fixed
> one or two other small issues along the way.  It hasn't really been
> tested with a wide variaty of different fat filesystems (if someone
> has a collection of disk images to test with somewhere, let me know),
> but it seems at least not worse than what it is replacing.
> 
> Rob Clark (8):
>   fs/fat: split out helper to init fsdata
>   fs/fat: introduce new director iterators
>   fat/fs: convert to directory iterators
>   fs: add fs_readdir()
>   fs/fat: implement opendir/readdir/closedir
>   fat/fs: remove a bunch of dead code
>   fat/fs: move ls to generic implementation
>   fs/fat: fix case for FAT shortnames
> 
>  fs/fat/Makefile|4 -
>  fs/fat/fat.c   | 1019 
> +---
>  fs/fat/fat_write.c |4 +-
>  fs/fat/file.c  |  183 --
>  fs/fs.c|  119 +-
>  include/fat.h  |   35 +-
>  include/fs.h   |   55 +++
>  7 files changed, 590 insertions(+), 829 deletions(-)
>  delete mode 100644 fs/fat/file.c
> 
Hello Rob,

I assume this patch series is meant to be merged before
[PATCH v1 00/15] enough UEFI for standard distro boot
and partially replaces it. Did I get it right?

Regards

Heinrich
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 0/8] fs/fat: cleanups + readdir implementation

[Rob Clark]

Introduce directory traversal iterators, and implement fs_readdir()
which is needed by EFI_LOADER.

The part re-working fat.c to use the directory iterators itself is
nearly a 2:1 negative diffstat, and a pretty big cleanup.  I fixed
one or two other small issues along the way.  It hasn't really been
tested with a wide variaty of different fat filesystems (if someone
has a collection of disk images to test with somewhere, let me know),
but it seems at least not worse than what it is replacing.

Rob Clark (8):
  fs/fat: split out helper to init fsdata
  fs/fat: introduce new director iterators
  fat/fs: convert to directory iterators
  fs: add fs_readdir()
  fs/fat: implement opendir/readdir/closedir
  fat/fs: remove a bunch of dead code
  fat/fs: move ls to generic implementation
  fs/fat: fix case for FAT shortnames

 fs/fat/Makefile|4 -
 fs/fat/fat.c   | 1019 +---
 fs/fat/fat_write.c |4 +-
 fs/fat/file.c  |  183 --
 fs/fs.c|  119 +-
 include/fat.h  |   35 +-
 include/fs.h   |   55 +++
 7 files changed, 590 insertions(+), 829 deletions(-)
 delete mode 100644 fs/fat/file.c

-- 
2.13.0

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot