[U-Boot] [PATCH v2 0/5] fs: fat: fixes for write under root directory

[U-Boot] [PATCH v2 0/5] fs: fat: fixes for write under root directory

[AKASHI Takahiro]

This patch set contains three (independent) bug fixes relating to
write operations to root directory. See each commit message
for more details.

Without those fixes, you may lose files that you created under root
directory or will see the file system corrupted. This will happen
particularly when you create (or even delete) many files repeatedly
under root directory.
(Non-root directory won't be affected.)

I only tested my patches on limited number of file system instances,
and hope that other people will also try them.

Changes in v2 (May 24, 2019)
* remove a debug message modification (patch#1)
* clarify a package dependency for test_fs (adding patch#5)

-Takahiro Akashi

AKASHI Takahiro (5):
  fs: fat:  write to non-cluster-aligned root directory
  fs: fat: flush a directory cluster properly
  fs: fat: allocate a new cluster for root directory of fat32
  test/py: test_fs: add tests for creating/deleting many files
  test/py: clarify a package dependency for test_fs

 fs/fat/fat_write.c                | 117 +++++++++++++++++++-----------
 test/py/README.md                 |  19 ++---
 test/py/tests/test_fs/test_ext.py |  84 +++++++++++++++++++++
 3 files changed, 167 insertions(+), 53 deletions(-)

-- 
2.21.0

[U-Boot] [PATCH v2 1/5] fs: fat: write to non-cluster-aligned root directory

[AKASHI Takahiro]

With the commit below, fat now correctly handles a file read under
a non-cluster-aligned root directory of fat12/16.
Write operation should be fixed in the same manner.

Fixes: commit 9b18358dc05d ("fs: fat: fix reading non-cluster-aligned
       root directory")
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: Anssi Hannula <anssi.hannula@bitwise.fi>
Tested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 fs/fat/fat_write.c | 78 +++++++++++++++++++++++++++++++---------------
 1 file changed, 53 insertions(+), 25 deletions(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 852f874e5817..3bc0dd637521 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -388,29 +388,23 @@ static __u32 determine_fatent(fsdata *mydata, __u32 entry)
 }
 
 /**
- * set_cluster() - write data to cluster
+ * set_sectors() - write data to sectors
  *
- * Write 'size' bytes from 'buffer' into the specified cluster.
+ * Write 'size' bytes from 'buffer' into the specified sector.
  *
  * @mydata:	data to be written
- * @clustnum:	cluster to be written to
+ * @startsect:	sector to be written to
  * @buffer:	data to be written
  * @size:	bytes to be written (but not more than the size of a cluster)
  * Return:	0 on success, -1 otherwise
  */
 static int
-set_cluster(fsdata *mydata, u32 clustnum, u8 *buffer, u32 size)
+set_sectors(fsdata *mydata, u32 startsect, u8 *buffer, u32 size)
 {
-	u32 idx = 0;
-	u32 startsect;
+	u32 nsects = 0;
 	int ret;
 
-	if (clustnum > 0)
-		startsect = clust_to_sect(mydata, clustnum);
-	else
-		startsect = mydata->rootdir_sect;
-
-	debug("clustnum: %d, startsect: %d\n", clustnum, startsect);
+	debug("startsect: %d\n", startsect);
 
 	if ((unsigned long)buffer & (ARCH_DMA_MINALIGN - 1)) {
 		ALLOC_CACHE_ALIGN_BUFFER(__u8, tmpbuf, mydata->sect_size);
@@ -429,17 +423,16 @@ set_cluster(fsdata *mydata, u32 clustnum, u8 *buffer, u32 size)
 			size -= mydata->sect_size;
 		}
 	} else if (size >= mydata->sect_size) {
-		idx = size / mydata->sect_size;
-		ret = disk_write(startsect, idx, buffer);
-		if (ret != idx) {
+		nsects = size / mydata->sect_size;
+		ret = disk_write(startsect, nsects, buffer);
+		if (ret != nsects) {
 			debug("Error writing data (got %d)\n", ret);
 			return -1;
 		}
 
-		startsect += idx;
-		idx *= mydata->sect_size;
-		buffer += idx;
-		size -= idx;
+		startsect += nsects;
+		buffer += nsects * mydata->sect_size;
+		size -= nsects * mydata->sect_size;
 	}
 
 	if (size) {
@@ -457,6 +450,44 @@ set_cluster(fsdata *mydata, u32 clustnum, u8 *buffer, u32 size)
 	return 0;
 }
 
+/**
+ * set_cluster() - write data to cluster
+ *
+ * Write 'size' bytes from 'buffer' into the specified cluster.
+ *
+ * @mydata:	data to be written
+ * @clustnum:	cluster to be written to
+ * @buffer:	data to be written
+ * @size:	bytes to be written (but not more than the size of a cluster)
+ * Return:	0 on success, -1 otherwise
+ */
+static int
+set_cluster(fsdata *mydata, u32 clustnum, u8 *buffer, u32 size)
+{
+	return set_sectors(mydata, clust_to_sect(mydata, clustnum),
+			   buffer, size);
+}
+
+static int
+flush_dir(fat_itr *itr)
+{
+	fsdata *mydata = itr->fsdata;
+	u32 startsect, sect_offset, nsects;
+
+	if (!itr->is_root || mydata->fatsize == 32)
+		return set_cluster(mydata, itr->clust, itr->block,
+				   mydata->clust_size * mydata->sect_size);
+
+	sect_offset = itr->clust * mydata->clust_size;
+	startsect = mydata->rootdir_sect + sect_offset;
+	/* do not write past the end of rootdir */
+	nsects = min_t(u32, mydata->clust_size,
+		       mydata->rootdir_size - sect_offset);
+
+	return set_sectors(mydata, startsect, itr->block,
+			   nsects * mydata->sect_size);
+}
+
 static __u8 tmpbuf_cluster[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN);
 
 /*
@@ -1171,8 +1202,7 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer,
 	}
 
 	/* Write directory table to device */
-	ret = set_cluster(mydata, itr->clust, itr->block,
-			  mydata->clust_size * mydata->sect_size);
+	ret = flush_dir(itr);
 	if (ret) {
 		printf("Error: writing directory entry\n");
 		ret = -EIO;
@@ -1249,8 +1279,7 @@ static int delete_dentry(fat_itr *itr)
 	memset(dentptr, 0, sizeof(*dentptr));
 	dentptr->name[0] = 0xe5;
 
-	if (set_cluster(mydata, itr->clust, itr->block,
-			mydata->clust_size * mydata->sect_size) != 0) {
+	if (flush_dir(itr)) {
 		printf("error: writing directory entry\n");
 		return -EIO;
 	}
@@ -1452,8 +1481,7 @@ int fat_mkdir(const char *new_dirname)
 	}
 
 	/* Write directory table to device */
-	ret = set_cluster(mydata, itr->clust, itr->block,
-			  mydata->clust_size * mydata->sect_size);
+	ret = flush_dir(itr);
 	if (ret)
 		printf("Error: writing directory entry\n");
 
-- 
2.21.0

[U-Boot] [PATCH v2 2/5] fs: fat: flush a directory cluster properly

[AKASHI Takahiro]

When a long name directory entry is created, multiple directory entries
may be occupied across a directory cluster boundary. Since only one
directory cluster is cached in a directory iterator, a first cluster must
be written back to device before switching over a second cluster.

Without this patch, some added files may be lost even if you don't see
any failures on write operation.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Tested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 fs/fat/fat_write.c | 33 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 3bc0dd637521..da1753d545ac 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -209,7 +209,8 @@ name11_12:
 	return 1;
 }
 
-static int flush_dir_table(fat_itr *itr);
+static int new_dir_table(fat_itr *itr);
+static int flush_dir(fat_itr *itr);
 
 /*
  * Fill dir_slot entries with appropriate name, id, and attr
@@ -242,19 +243,15 @@ fill_dir_slot(fat_itr *itr, const char *l_name)
 		memcpy(itr->dent, slotptr, sizeof(dir_slot));
 		slotptr--;
 		counter--;
+
+		if (itr->remaining == 0)
+			flush_dir(itr);
+
 		if (!fat_itr_next(itr))
-			if (!itr->dent && !itr->is_root && flush_dir_table(itr))
+			if (!itr->dent && !itr->is_root && new_dir_table(itr))
 				return -1;
 	}
 
-	if (!itr->dent && !itr->is_root)
-		/*
-		 * don't care return value here because we have already
-		 * finished completing an entry with name, only ending up
-		 * no more entry left
-		 */
-		flush_dir_table(itr);
-
 	return 0;
 }
 
@@ -621,18 +618,14 @@ static int find_empty_cluster(fsdata *mydata)
 }
 
 /*
- * Write directory entries in itr's buffer to block device
+ * Allocate a cluster for additional directory entries
  */
-static int flush_dir_table(fat_itr *itr)
+static int new_dir_table(fat_itr *itr)
 {
 	fsdata *mydata = itr->fsdata;
 	int dir_newclust = 0;
 	unsigned int bytesperclust = mydata->clust_size * mydata->sect_size;
 
-	if (set_cluster(mydata, itr->clust, itr->block, bytesperclust) != 0) {
-		printf("error: writing directory entry\n");
-		return -1;
-	}
 	dir_newclust = find_empty_cluster(mydata);
 	set_fatent_value(mydata, itr->clust, dir_newclust);
 	if (mydata->fatsize == 32)
@@ -987,7 +980,7 @@ static dir_entry *find_directory_entry(fat_itr *itr, char *filename)
 			return itr->dent;
 	}
 
-	if (!itr->dent && !itr->is_root && flush_dir_table(itr))
+	if (!itr->dent && !itr->is_root && new_dir_table(itr))
 		/* indicate that allocating dent failed */
 		itr->dent = NULL;
 
@@ -1172,14 +1165,16 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer,
 
 		memset(itr->dent, 0, sizeof(*itr->dent));
 
-		/* Set short name to set alias checksum field in dir_slot */
+		/* Calculate checksum for short name */
 		set_name(itr->dent, filename);
+
+		/* Set long name entries */
 		if (fill_dir_slot(itr, filename)) {
 			ret = -EIO;
 			goto exit;
 		}
 
-		/* Set attribute as archive for regular file */
+		/* Set short name entry */
 		fill_dentry(itr->fsdata, itr->dent, filename, 0, size, 0x20);
 
 		retdent = itr->dent;
-- 
2.21.0

[U-Boot] [PATCH v2 3/5] fs: fat: allocate a new cluster for root directory of fat32

[AKASHI Takahiro]

Contrary to fat12/16, fat32 can have root directory at any location
and its size can be expanded.
Without this patch, root directory won't grow properly and so we will
eventually fail to add files under root directory. Please note that this
can happen even if you delete many files as deleted directory entries
are not reclaimed but just marked as "deleted" under the current
implementation.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Tested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 fs/fat/fat_write.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index da1753d545ac..3bb9eac097eb 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -247,8 +247,11 @@ fill_dir_slot(fat_itr *itr, const char *l_name)
 		if (itr->remaining == 0)
 			flush_dir(itr);
 
+		/* allocate a cluster for more entries */
 		if (!fat_itr_next(itr))
-			if (!itr->dent && !itr->is_root && new_dir_table(itr))
+			if (!itr->dent &&
+			    (!itr->is_root || itr->fsdata->fatsize == 32) &&
+			    new_dir_table(itr))
 				return -1;
 	}
 
@@ -980,7 +983,10 @@ static dir_entry *find_directory_entry(fat_itr *itr, char *filename)
 			return itr->dent;
 	}
 
-	if (!itr->dent && !itr->is_root && new_dir_table(itr))
+	/* allocate a cluster for more entries */
+	if (!itr->dent &&
+	    (!itr->is_root || itr->fsdata->fatsize == 32) &&
+	    new_dir_table(itr))
 		/* indicate that allocating dent failed */
 		itr->dent = NULL;
 
-- 
2.21.0

[U-Boot] [PATCH v2 4/5] test/py: test_fs: add tests for creating/deleting many files

[AKASHI Takahiro]

Two test cases are added under test_fs_ext:
    test case 10: for root directory
    test case 11: for non-root directory

Those will verify a behavior fixed by the commits related to
root directory
("fs: fat: allocate a new cluster for root directory of fat32" and
"fs: fat: flush a directory cluster properly"), and focus on
handling long-file-name directory entries under a directory.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 test/py/tests/test_fs/test_ext.py | 84 +++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/test/py/tests/test_fs/test_ext.py b/test/py/tests/test_fs/test_ext.py
index 2c47738b8df2..361f440dd6d4 100644
--- a/test/py/tests/test_fs/test_ext.py
+++ b/test/py/tests/test_fs/test_ext.py
@@ -233,3 +233,87 @@ class TestFsExt(object):
                     % (fs_type, ADDR, MIN_FILE)])
             assert('Unable to write "/dir1' in ''.join(output))
             assert_fs_integrity(fs_type, fs_img)
+
+    def test_fs_ext10(self, u_boot_console, fs_obj_ext):
+        """
+        'Test Case 10 - create/delete as many directories under root directory
+        as amount of directory entries goes beyond one cluster size)'
+        """
+        fs_type,fs_img,md5val = fs_obj_ext
+        with u_boot_console.log.section('Test Case 10 - create/delete (many)'):
+            # Test Case 10a - Create many files
+            #   Please note that the size of directory entry is 32 bytes.
+            #   So one typical cluster may holds 64 (2048/32) entries.
+            output = u_boot_console.run_command(
+                'host bind 0 %s' % fs_img)
+
+            for i in range(0, 66):
+                output = u_boot_console.run_command(
+                    '%swrite host 0:0 %x /FILE0123456789_%02x 100'
+                    % (fs_type, ADDR, i))
+            output = u_boot_console.run_command('%sls host 0:0 /' % fs_type)
+            assert('FILE0123456789_00' in output)
+            assert('FILE0123456789_41' in output)
+
+            # Test Case 10b - Delete many files
+            for i in range(0, 66):
+                output = u_boot_console.run_command(
+                    '%srm host 0:0 /FILE0123456789_%02x'
+                    % (fs_type, i))
+            output = u_boot_console.run_command('%sls host 0:0 /' % fs_type)
+            assert(not 'FILE0123456789_00' in output)
+            assert(not 'FILE0123456789_41' in output)
+
+            # Test Case 10c - Create many files again
+            # Please note no.64 and 65 are intentionally re-created
+            for i in range(64, 128):
+                output = u_boot_console.run_command(
+                    '%swrite host 0:0 %x /FILE0123456789_%02x 100'
+                    % (fs_type, ADDR, i))
+            output = u_boot_console.run_command('%sls host 0:0 /' % fs_type)
+            assert('FILE0123456789_40' in output)
+            assert('FILE0123456789_79' in output)
+
+            assert_fs_integrity(fs_type, fs_img)
+
+    def test_fs_ext11(self, u_boot_console, fs_obj_ext):
+        """
+        'Test Case 11 - create/delete as many directories under non-root
+        directory as amount of directory entries goes beyond one cluster size)'
+        """
+        fs_type,fs_img,md5val = fs_obj_ext
+        with u_boot_console.log.section('Test Case 10 - create/delete (many)'):
+            # Test Case 11a - Create many files
+            #   Please note that the size of directory entry is 32 bytes.
+            #   So one typical cluster may holds 64 (2048/32) entries.
+            output = u_boot_console.run_command(
+                'host bind 0 %s' % fs_img)
+
+            for i in range(0, 66):
+                output = u_boot_console.run_command(
+                    '%swrite host 0:0 %x /dir1/FILE0123456789_%02x 100'
+                    % (fs_type, ADDR, i))
+            output = u_boot_console.run_command('%sls host 0:0 /dir1' % fs_type)
+            assert('FILE0123456789_00' in output)
+            assert('FILE0123456789_41' in output)
+
+            # Test Case 11b - Delete many files
+            for i in range(0, 66):
+                output = u_boot_console.run_command(
+                    '%srm host 0:0 /dir1/FILE0123456789_%02x'
+                    % (fs_type, i))
+            output = u_boot_console.run_command('%sls host 0:0 /dir1' % fs_type)
+            assert(not 'FILE0123456789_00' in output)
+            assert(not 'FILE0123456789_41' in output)
+
+            # Test Case 11c - Create many files again
+            # Please note no.64 and 65 are intentionally re-created
+            for i in range(64, 128):
+                output = u_boot_console.run_command(
+                    '%swrite host 0:0 %x /dir1/FILE0123456789_%02x 100'
+                    % (fs_type, ADDR, i))
+            output = u_boot_console.run_command('%sls host 0:0 /dir1' % fs_type)
+            assert('FILE0123456789_40' in output)
+            assert('FILE0123456789_79' in output)
+
+            assert_fs_integrity(fs_type, fs_img)
-- 
2.21.0

[U-Boot] [PATCH v2 5/5] test/py: clarify a package dependency for test_fs

[AKASHI Takahiro]

File system tests, test_fs, relies on guestmount so that it can be
executed in a user mode. Describe this in README.md.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 test/py/README.md | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/test/py/README.md b/test/py/README.md
index 2156661d6c42..50b174b51960 100644
--- a/test/py/README.md
+++ b/test/py/README.md
@@ -25,15 +25,16 @@ On Debian or Debian-like distributions, the following packages are required.
 Some packages are required to execute any test, and others only for specific
 tests. Similar package names should exist in other distributions.
 
-| Package        | Version tested (Ubuntu 14.04) |
-| -------------- | ----------------------------- |
-| python         | 2.7.5-5ubuntu3                |
-| python-pytest  | 2.5.1-1                       |
-| python-subunit | -                             |
-| gdisk          | 0.8.8-1ubuntu0.1              |
-| dfu-util       | 0.5-1                         |
-| dtc            | 1.4.0+dfsg-1                  |
-| openssl        | 1.0.1f-1ubuntu2.22            |
+| Package         | Version tested (Ubuntu 14.04) |
+| --------------- | ----------------------------- |
+| python          | 2.7.5-5ubuntu3                |
+| python-pytest   | 2.5.1-1                       |
+| python-subunit  | -                             |
+| gdisk           | 0.8.8-1ubuntu0.1              |
+| dfu-util        | 0.5-1                         |
+| dtc             | 1.4.0+dfsg-1                  |
+| openssl         | 1.0.1f-1ubuntu2.22            |
+| libguestfs-tools| 1:1.32.2-4ubuntu2.2           |
 
 The test script supports either:
 
-- 
2.21.0

[U-Boot] [PATCH v2 4/5] test/py: test_fs: add tests for creating/deleting many files

[Heinrich Schuchardt]

As both Tom and I wrote to you in reply of to the first version of the
patch it is not clear how these tests can be run.

make tests

simply skips them.

Could you, please, provide a description in test/py/README.md.

Best regards

Heinrich

On 5/24/19 7:10 AM, AKASHI Takahiro wrote:
> Two test cases are added under test_fs_ext:
>      test case 10: for root directory
>      test case 11: for non-root directory
>
> Those will verify a behavior fixed by the commits related to
> root directory
> ("fs: fat: allocate a new cluster for root directory of fat32" and
> "fs: fat: flush a directory cluster properly"), and focus on
> handling long-file-name directory entries under a directory.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   test/py/tests/test_fs/test_ext.py | 84 +++++++++++++++++++++++++++++++
>   1 file changed, 84 insertions(+)
>
> diff --git a/test/py/tests/test_fs/test_ext.py b/test/py/tests/test_fs/test_ext.py
> index 2c47738b8df2..361f440dd6d4 100644
> --- a/test/py/tests/test_fs/test_ext.py
> +++ b/test/py/tests/test_fs/test_ext.py
> @@ -233,3 +233,87 @@ class TestFsExt(object):
>                       % (fs_type, ADDR, MIN_FILE)])
>               assert('Unable to write "/dir1' in ''.join(output))
>               assert_fs_integrity(fs_type, fs_img)
> +
> +    def test_fs_ext10(self, u_boot_console, fs_obj_ext):
> +        """
> +        'Test Case 10 - create/delete as many directories under root directory
> +        as amount of directory entries goes beyond one cluster size)'
> +        """
> +        fs_type,fs_img,md5val = fs_obj_ext
> +        with u_boot_console.log.section('Test Case 10 - create/delete (many)'):
> +            # Test Case 10a - Create many files
> +            #   Please note that the size of directory entry is 32 bytes.
> +            #   So one typical cluster may holds 64 (2048/32) entries.
> +            output = u_boot_console.run_command(
> +                'host bind 0 %s' % fs_img)
> +
> +            for i in range(0, 66):
> +                output = u_boot_console.run_command(
> +                    '%swrite host 0:0 %x /FILE0123456789_%02x 100'
> +                    % (fs_type, ADDR, i))
> +            output = u_boot_console.run_command('%sls host 0:0 /' % fs_type)
> +            assert('FILE0123456789_00' in output)
> +            assert('FILE0123456789_41' in output)
> +
> +            # Test Case 10b - Delete many files
> +            for i in range(0, 66):
> +                output = u_boot_console.run_command(
> +                    '%srm host 0:0 /FILE0123456789_%02x'
> +                    % (fs_type, i))
> +            output = u_boot_console.run_command('%sls host 0:0 /' % fs_type)
> +            assert(not 'FILE0123456789_00' in output)
> +            assert(not 'FILE0123456789_41' in output)
> +
> +            # Test Case 10c - Create many files again
> +            # Please note no.64 and 65 are intentionally re-created
> +            for i in range(64, 128):
> +                output = u_boot_console.run_command(
> +                    '%swrite host 0:0 %x /FILE0123456789_%02x 100'
> +                    % (fs_type, ADDR, i))
> +            output = u_boot_console.run_command('%sls host 0:0 /' % fs_type)
> +            assert('FILE0123456789_40' in output)
> +            assert('FILE0123456789_79' in output)
> +
> +            assert_fs_integrity(fs_type, fs_img)
> +
> +    def test_fs_ext11(self, u_boot_console, fs_obj_ext):
> +        """
> +        'Test Case 11 - create/delete as many directories under non-root
> +        directory as amount of directory entries goes beyond one cluster size)'
> +        """
> +        fs_type,fs_img,md5val = fs_obj_ext
> +        with u_boot_console.log.section('Test Case 10 - create/delete (many)'):
> +            # Test Case 11a - Create many files
> +            #   Please note that the size of directory entry is 32 bytes.
> +            #   So one typical cluster may holds 64 (2048/32) entries.
> +            output = u_boot_console.run_command(
> +                'host bind 0 %s' % fs_img)
> +
> +            for i in range(0, 66):
> +                output = u_boot_console.run_command(
> +                    '%swrite host 0:0 %x /dir1/FILE0123456789_%02x 100'
> +                    % (fs_type, ADDR, i))
> +            output = u_boot_console.run_command('%sls host 0:0 /dir1' % fs_type)
> +            assert('FILE0123456789_00' in output)
> +            assert('FILE0123456789_41' in output)
> +
> +            # Test Case 11b - Delete many files
> +            for i in range(0, 66):
> +                output = u_boot_console.run_command(
> +                    '%srm host 0:0 /dir1/FILE0123456789_%02x'
> +                    % (fs_type, i))
> +            output = u_boot_console.run_command('%sls host 0:0 /dir1' % fs_type)
> +            assert(not 'FILE0123456789_00' in output)
> +            assert(not 'FILE0123456789_41' in output)
> +
> +            # Test Case 11c - Create many files again
> +            # Please note no.64 and 65 are intentionally re-created
> +            for i in range(64, 128):
> +                output = u_boot_console.run_command(
> +                    '%swrite host 0:0 %x /dir1/FILE0123456789_%02x 100'
> +                    % (fs_type, ADDR, i))
> +            output = u_boot_console.run_command('%sls host 0:0 /dir1' % fs_type)
> +            assert('FILE0123456789_40' in output)
> +            assert('FILE0123456789_79' in output)
> +
> +            assert_fs_integrity(fs_type, fs_img)
>

[U-Boot] [PATCH v2 5/5] test/py: clarify a package dependency for test_fs

[Heinrich Schuchardt]

On 5/24/19 7:10 AM, AKASHI Takahiro wrote:
> File system tests, test_fs, relies on guestmount so that it can be
> executed in a user mode. Describe this in README.md.

It is unclear where commands from package libguestfs-tools are used. I
could not find any script in directory test/ using it.

As already mentioned in reply to patch 4 of the series, please, provide
clear instructions how to invoke the file system tests in this README.md.

make qemu_arm64_defconfig
make
python3 ./test/py/test.py --bd=qemu-arm64 --build-dir=.

is simply skipping all of them.

Best regards

Heinrich

>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   test/py/README.md | 19 ++++++++++---------
>   1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/test/py/README.md b/test/py/README.md
> index 2156661d6c42..50b174b51960 100644
> --- a/test/py/README.md
> +++ b/test/py/README.md
> @@ -25,15 +25,16 @@ On Debian or Debian-like distributions, the following packages are required.
>   Some packages are required to execute any test, and others only for specific
>   tests. Similar package names should exist in other distributions.
>
> -| Package        | Version tested (Ubuntu 14.04) |
> -| -------------- | ----------------------------- |
> -| python         | 2.7.5-5ubuntu3                |
> -| python-pytest  | 2.5.1-1                       |
> -| python-subunit | -                             |
> -| gdisk          | 0.8.8-1ubuntu0.1              |
> -| dfu-util       | 0.5-1                         |
> -| dtc            | 1.4.0+dfsg-1                  |
> -| openssl        | 1.0.1f-1ubuntu2.22            |
> +| Package         | Version tested (Ubuntu 14.04) |
> +| --------------- | ----------------------------- |
> +| python          | 2.7.5-5ubuntu3                |
> +| python-pytest   | 2.5.1-1                       |
> +| python-subunit  | -                             |
> +| gdisk           | 0.8.8-1ubuntu0.1              |
> +| dfu-util        | 0.5-1                         |
> +| dtc             | 1.4.0+dfsg-1                  |
> +| openssl         | 1.0.1f-1ubuntu2.22            |
> +| libguestfs-tools| 1:1.32.2-4ubuntu2.2           |
>
>   The test script supports either:
>
>

[U-Boot] [PATCH v2 4/5] test/py: test_fs: add tests for creating/deleting many files

[Tom Rini]

On Tue, May 28, 2019 at 06:46:36PM +0200, Heinrich Schuchardt wrote:

> As both Tom and I wrote to you in reply of to the first version of the
> patch it is not clear how these tests can be run.
> 
> make tests
> 
> simply skips them.
> 
> Could you, please, provide a description in test/py/README.md.

To be a bit more clear, https://patchwork.ozlabs.org/patch/1104600/
documents the requirement to install libguestfs-tools but not how to
configure things further.  Just installing that package locally changes
my existing test/py/tests/test_fs/test_basic.py run from
'..........................sssssssssssss' to full skip.

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

[U-Boot] [PATCH v2 5/5] test/py: clarify a package dependency for test_fs

[Heinrich Schuchardt]

On 5/24/19 7:10 AM, AKASHI Takahiro wrote:
> File system tests, test_fs, relies on guestmount so that it can be
> executed in a user mode. Describe this in README.md.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   test/py/README.md | 19 ++++++++++---------
>   1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/test/py/README.md b/test/py/README.md
> index 2156661d6c42..50b174b51960 100644
> --- a/test/py/README.md
> +++ b/test/py/README.md
> @@ -25,15 +25,16 @@ On Debian or Debian-like distributions, the following packages are required.
>   Some packages are required to execute any test, and others only for specific
>   tests. Similar package names should exist in other distributions.
>
> -| Package        | Version tested (Ubuntu 14.04) |
> -| -------------- | ----------------------------- |
> -| python         | 2.7.5-5ubuntu3                |
> -| python-pytest  | 2.5.1-1                       |
> -| python-subunit | -                             |
> -| gdisk          | 0.8.8-1ubuntu0.1              |
> -| dfu-util       | 0.5-1                         |
> -| dtc            | 1.4.0+dfsg-1                  |
> -| openssl        | 1.0.1f-1ubuntu2.22            |
> +| Package         | Version tested (Ubuntu 14.04) |

Ubuntu 14.04 reached end of life in April.

> +| --------------- | ----------------------------- |
> +| python          | 2.7.5-5ubuntu3                |
> +| python-pytest   | 2.5.1-1                       |
> +| python-subunit  | -                             |
> +| gdisk           | 0.8.8-1ubuntu0.1              |
> +| dfu-util        | 0.5-1                         |
> +| dtc             | 1.4.0+dfsg-1                  |

A package called dtc does not exist in Ubuntu. Should this be
device-tree-compiler?

> +| openssl         | 1.0.1f-1ubuntu2.22            |
> +| libguestfs-tools| 1:1.32.2-4ubuntu2.2           |

This is a version from Ubuntu 16.04). But the table header refers to
Ubuntu 14.04. Please, provide consistent information.

Best regards

Heinrich

>
>   The test script supports either:
>
>

[U-Boot] [PATCH v2 5/5] test/py: clarify a package dependency for test_fs

[Tom Rini]

On Tue, May 28, 2019 at 07:18:34PM +0200, Heinrich Schuchardt wrote:
> On 5/24/19 7:10 AM, AKASHI Takahiro wrote:
> >File system tests, test_fs, relies on guestmount so that it can be
> >executed in a user mode. Describe this in README.md.
> >
> >Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >---
> >  test/py/README.md | 19 ++++++++++---------
> >  1 file changed, 10 insertions(+), 9 deletions(-)
> >
> >diff --git a/test/py/README.md b/test/py/README.md
> >index 2156661d6c42..50b174b51960 100644
> >--- a/test/py/README.md
> >+++ b/test/py/README.md
> >@@ -25,15 +25,16 @@ On Debian or Debian-like distributions, the following packages are required.
> >  Some packages are required to execute any test, and others only for specific
> >  tests. Similar package names should exist in other distributions.
> >
> >-| Package        | Version tested (Ubuntu 14.04) |
> >-| -------------- | ----------------------------- |
> >-| python         | 2.7.5-5ubuntu3                |
> >-| python-pytest  | 2.5.1-1                       |
> >-| python-subunit | -                             |
> >-| gdisk          | 0.8.8-1ubuntu0.1              |
> >-| dfu-util       | 0.5-1                         |
> >-| dtc            | 1.4.0+dfsg-1                  |
> >-| openssl        | 1.0.1f-1ubuntu2.22            |
> >+| Package         | Version tested (Ubuntu 14.04) |
> 
> Ubuntu 14.04 reached end of life in April.

FWIW, I had a local patch to try and update this list to 16.04 a while
ago, but never finished.  We should probably avoid spelling out a host
distribution and version and just say what we need installed.

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

[U-Boot] [PATCH v2 1/5] fs: fat: write to non-cluster-aligned root directory

[Tom Rini]

On Fri, May 24, 2019 at 02:10:35PM +0900, AKASHI Takahiro wrote:

> With the commit below, fat now correctly handles a file read under
> a non-cluster-aligned root directory of fat12/16.
> Write operation should be fixed in the same manner.
> 
> Fixes: commit 9b18358dc05d ("fs: fat: fix reading non-cluster-aligned
>        root directory")
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Cc: Anssi Hannula <anssi.hannula@bitwise.fi>
> Tested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Applied to u-boot/master, thanks!

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

[U-Boot] [PATCH v2 2/5] fs: fat: flush a directory cluster properly

[Tom Rini]

On Fri, May 24, 2019 at 02:10:36PM +0900, AKASHI Takahiro wrote:

> When a long name directory entry is created, multiple directory entries
> may be occupied across a directory cluster boundary. Since only one
> directory cluster is cached in a directory iterator, a first cluster must
> be written back to device before switching over a second cluster.
> 
> Without this patch, some added files may be lost even if you don't see
> any failures on write operation.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Tested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Applied to u-boot/master, thanks!

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

[U-Boot] [PATCH v2 3/5] fs: fat: allocate a new cluster for root directory of fat32

[Tom Rini]

On Fri, May 24, 2019 at 02:10:37PM +0900, AKASHI Takahiro wrote:

> Contrary to fat12/16, fat32 can have root directory at any location
> and its size can be expanded.
> Without this patch, root directory won't grow properly and so we will
> eventually fail to add files under root directory. Please note that this
> can happen even if you delete many files as deleted directory entries
> are not reclaimed but just marked as "deleted" under the current
> implementation.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Tested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Applied to u-boot/master, thanks!

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