| Message ID | 20180720025723.6736-1-takahiro.akashi@linaro.org |
|---|---|
| Headers | show |
| Series | fs: fat: extend FAT write operations | expand |
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote: > This patch series is an attempt to address FAT write related issues > in an effort of running UEFI SCT (Self-Certification Test) to verify > UEFI support on u-boot. > > SCT is a test platform as well as an extentisive collection of test > cases for UEFI specification. It can run all the tests automatically > and save test results to dedicated log files. > > AFAIK, what's missing in the current fat file system to safely run > SCT without errors (I don't mean test case failures) are: > * write a file located under sub-directories > * write a file with non-zero offset > * delete a file > * create a directory > > Patch#1 to patch#6 are some sort of preparatory ones. > Patch#7 implements write with sub-directories path. > Patch#8 to patch#10 implement write with non-zero offset. > Patch#11 to patch#15 are related to creating a directory. > Patch#16 provides delete, but it doesn't actually delete a file > but instead return 0 with purging a file content, which is good > enough to run SCT for now. > Finally, patch#17 fixes a minor bug in fs-test.sh. > > I applied this patch set on top of v2018.07 along with a couple of > yet-to--be-upstreamed UEFI-related patches, and could successfully > run unmodified SCT[1] on qemu-arm. > > [1] http://uefi.org/testtools Thanks Takahiro for getting this all implemented. I have some questions concerning code pages: The FAT file system contains a short name and a long name. The short name is in the 8-bit code page of the system. The long name is 16bit Unicode. Which local code page do you assume? I would suggest to use 437 as we do in some other drivers. With your patch fat_mkdir() and fat_file_write() do not yet support UTF-8 as file names and convert this to the 437 local page for the short name and to UTF-16 for the long name. Without this conversoin we will not be able to implement the EFI specification. normalize_longname() simply ignores codes 0x80-0xFF. Please, have a look at https://www.win.tue.nl/~aeb/linux/fs/fat/fatgen103.pdf Microsoft Extensible Firmware Initiative FAT32 File System Specification FAT: General Overview of On-Disk Format Version 1.03, December 6, 2000 Shouldn't we implement the base-name algorithm as described in the paper? Best regards Heinrich
Hello Tom, hello Alex, I have been testing the patches. They are working fine for ASCII file names. To support Unicode file names extra work will be needed. But probably we should postpone this to a later patch series. There are some dependencies with my work for correcting errors in Unicode handling for the EFI branch. Should the patches be passed via efi-next? Best regards Heinrich
On Sun, Jul 22, 2018 at 08:44:39AM +0200, Heinrich Schuchardt wrote: > Hello Tom, hello Alex, > > I have been testing the patches. They are working fine for ASCII file > names. To support Unicode file names extra work will be needed. But > probably we should postpone this to a later patch series. I absolutely agree. To be clear, the aim of my FAT write patchset is to successfully run SCT to evaluate and verify UEFI-related features rather than to make the implementation comply with UEFI specification. For the latter purpose, separate patches would be compiled as it might require time-consuming efforts. (I'm sure that SCT's file (media access) protocol test causes lots of test case failures.) Thanks, -Takahiro AKASHI > There are some dependencies with my work for correcting errors in > Unicode handling for the EFI branch. Should the patches be passed via > efi-next? > > Best regards > > Heinrich
On Sun, Jul 22, 2018 at 08:44:39AM +0200, Heinrich Schuchardt wrote: > Hello Tom, hello Alex, > > I have been testing the patches. They are working fine for ASCII file > names. To support Unicode file names extra work will be needed. But > probably we should postpone this to a later patch series. > > There are some dependencies with my work for correcting errors in > Unicode handling for the EFI branch. Should the patches be passed via > efi-next? Yes, a follow-up series makes sense, and yes, efi-next for the patches themselves sounds fine, thanks!
On 07/23/2018 04:46 PM, Tom Rini wrote: > On Sun, Jul 22, 2018 at 08:44:39AM +0200, Heinrich Schuchardt wrote: >> Hello Tom, hello Alex, >> >> I have been testing the patches. They are working fine for ASCII file >> names. To support Unicode file names extra work will be needed. But >> probably we should postpone this to a later patch series. >> >> There are some dependencies with my work for correcting errors in >> Unicode handling for the EFI branch. Should the patches be passed via >> efi-next? > > Yes, a follow-up series makes sense, and yes, efi-next for the patches > themselves sounds fine, thanks! > Hello Takahiro, could you, please, resubmit the series with the few changes needed so that Alex can consider them for efi-next. Cheers Heinrich
On Tue, Aug 07, 2018 at 12:34:28AM +0200, Heinrich Schuchardt wrote: > On 07/23/2018 04:46 PM, Tom Rini wrote: > > On Sun, Jul 22, 2018 at 08:44:39AM +0200, Heinrich Schuchardt wrote: > >> Hello Tom, hello Alex, > >> > >> I have been testing the patches. They are working fine for ASCII file > >> names. To support Unicode file names extra work will be needed. But > >> probably we should postpone this to a later patch series. > >> > >> There are some dependencies with my work for correcting errors in > >> Unicode handling for the EFI branch. Should the patches be passed via > >> efi-next? > > > > Yes, a follow-up series makes sense, and yes, efi-next for the patches > > themselves sounds fine, thanks! > > > > Hello Takahiro, > > could you, please, resubmit the series with the few changes needed so > that Alex can consider them for efi-next. Well, I'm sure that my patch set can be applied to v2018.09-rc1 w/o any changes if your patch (0dc1bfb7302 "fs: fat: cannot write to subdirectories") is reverted beforehand. As I said, my patch(#5/17) superseded it in terms of its function. What's more, I ran fs-test.sh again, and all tests passed, either against fat16 or fat32. Do we still want to update a test result in fs-test.sh? (removing it would be the easiest.) Thanks, -Takahiro AKASHI > Cheers > > Heinrich
On Tue, Aug 07, 2018 at 02:38:34PM +0900, AKASHI Takahiro wrote: > On Tue, Aug 07, 2018 at 12:34:28AM +0200, Heinrich Schuchardt wrote: > > On 07/23/2018 04:46 PM, Tom Rini wrote: > > > On Sun, Jul 22, 2018 at 08:44:39AM +0200, Heinrich Schuchardt wrote: > > >> Hello Tom, hello Alex, > > >> > > >> I have been testing the patches. They are working fine for ASCII file > > >> names. To support Unicode file names extra work will be needed. But > > >> probably we should postpone this to a later patch series. > > >> > > >> There are some dependencies with my work for correcting errors in > > >> Unicode handling for the EFI branch. Should the patches be passed via > > >> efi-next? > > > > > > Yes, a follow-up series makes sense, and yes, efi-next for the patches > > > themselves sounds fine, thanks! > > > > > > > Hello Takahiro, > > > > could you, please, resubmit the series with the few changes needed so > > that Alex can consider them for efi-next. > > Well, I'm sure that my patch set can be applied to v2018.09-rc1 w/o any > changes if your patch (0dc1bfb7302 "fs: fat: cannot write to subdirectories") > is reverted beforehand. As I said, my patch(#5/17) superseded it in terms of > its function. See: https://git.linaro.org/people/takahiro.akashi/u-boot.git fat_write > What's more, I ran fs-test.sh again, and all tests passed, either against > fat16 or fat32. Do we still want to update a test result in fs-test.sh? > (removing it would be the easiest.) I've almost finished to convert fs-test.sh to a Python script, but it will take some time to add more tests against my patch set as my priority right now is on examining a test result from SCT. -Takahiro AKASHI > Thanks, > -Takahiro AKASHI > > > > Cheers > > > > Heinrich
On 08/07/2018 07:38 AM, AKASHI Takahiro wrote: > On Tue, Aug 07, 2018 at 12:34:28AM +0200, Heinrich Schuchardt wrote: >> On 07/23/2018 04:46 PM, Tom Rini wrote: >>> On Sun, Jul 22, 2018 at 08:44:39AM +0200, Heinrich Schuchardt wrote: >>>> Hello Tom, hello Alex, >>>> >>>> I have been testing the patches. They are working fine for ASCII file >>>> names. To support Unicode file names extra work will be needed. But >>>> probably we should postpone this to a later patch series. >>>> >>>> There are some dependencies with my work for correcting errors in >>>> Unicode handling for the EFI branch. Should the patches be passed via >>>> efi-next? >>> >>> Yes, a follow-up series makes sense, and yes, efi-next for the patches >>> themselves sounds fine, thanks! >>> >> >> Hello Takahiro, >> >> could you, please, resubmit the series with the few changes needed so >> that Alex can consider them for efi-next. > > Well, I'm sure that my patch set can be applied to v2018.09-rc1 w/o any > changes if your patch (0dc1bfb7302 "fs: fat: cannot write to subdirectories") > is reverted beforehand. As I said, my patch(#5/17) superseded it in terms of > its function. Please, add a patch doing the reverting or rebase patch 5. > > What's more, I ran fs-test.sh again, and all tests passed, either against > fat16 or fat32. Do we still want to update a test result in fs-test.sh? > (removing it would be the easiest.) As long as we have no better tests in place updating those comments is appropriate. In my review of patch 01/17 I indicated an error in evaluating the sector number. Best regards Heinrich
This patch series is an attempt to address FAT write related issues in an effort of running UEFI SCT (Self-Certification Test) to verify UEFI support on u-boot. SCT is a test platform as well as an extentisive collection of test cases for UEFI specification. It can run all the tests automatically and save test results to dedicated log files. AFAIK, what's missing in the current fat file system to safely run SCT without errors (I don't mean test case failures) are: * write a file located under sub-directories * write a file with non-zero offset * delete a file * create a directory Patch#1 to patch#6 are some sort of preparatory ones. Patch#7 implements write with sub-directories path. Patch#8 to patch#10 implement write with non-zero offset. Patch#11 to patch#15 are related to creating a directory. Patch#16 provides delete, but it doesn't actually delete a file but instead return 0 with purging a file content, which is good enough to run SCT for now. Finally, patch#17 fixes a minor bug in fs-test.sh. I applied this patch set on top of v2018.07 along with a couple of yet-to--be-upstreamed UEFI-related patches, and could successfully run unmodified SCT[1] on qemu-arm. [1] http://uefi.org/testtools AKASHI Takahiro (17): fs: fat: extend get_fs_info() for write use fs: fat: handle "." and ".." of root dir correctly with fat_itr_resolve() fs: fat: make directory iterator global for write use fs: fat: assure iterator's ->dent belongs to ->clust fs: fat: check and normailze file name fs: fat: write returns error code instead of -1 fs: fat: support write with sub-directory path fs: fat: refactor write interface for a file offset fs: fat: support write with non-zero offset cmd: fat: add offset parameter to fatwrite fs: add mkdir interface fs: fat: remember the starting cluster number of directory fs: fat: support mkdir cmd: fat: add fatmkdir command efi_loader: file: support creating a directory efi_loader: implement a pseudo "file delete" fs-test: fix false positive error at Test Case 12 cmd/fat.c | 22 +- fs/fat/fat.c | 98 ++-- fs/fat/fat_write.c | 1051 +++++++++++++++++++++++-------------- fs/fs.c | 46 ++ include/fat.h | 37 ++ include/fs.h | 10 + lib/efi_loader/efi_file.c | 24 +- test/fs/fs-test.sh | 2 +- 8 files changed, 825 insertions(+), 465 deletions(-)