[U-Boot] [PATCH v3 0/6] solve issues in gpt management

[U-Boot] [PATCH v3 0/6] solve issues in gpt management

[Patrick Delaunay]

in the last version v2017.09, I see some regression for the command

$> gpt write mmc 0 "name=test,start=0x4400,size=0"
$> gpt write mmc 0 "name=test,size=0"

I use sandbox python test to verify if this issue is also present in
v2017.11-rc1 and when I check the log tests, I detect a other issue
for the swap / rename feature : the offset and the size is always 1MB
align, that cause issue if the partition wasn't initially 1MB align.
And it is the case of the test (the size of partition change after the
command gpt rename or swap)

I propose this patch-set with:
- updated gpt test to highlight the issues
- my proposed correction for the 2 issues

tests are ok on v2017.11-rc1


Changes in v3:
- update after Stephen Warren comments
- Add partition name in persistent data and test them
- split test_gpt.py update: commit to add the test write command
- tests are now OK for each commit
- Indicate LBA end error for rename command in test/py and commit message

Changes in v2:
- Split test to functional change

Patrick Delaunay (6):
  test/py: gpt: copy persistent file
  test/py: gpt: add test for sub-command read and verify
  disk: efi: correct the overlap check on GPT header and PTE
  test/py: gpt: add test for sub-command write
  test/py: gpt: test start LBA for sub-command rename and swap
  cmd: gpt: solve issue for swap and rename command

 cmd/gpt.c                 | 12 +++----
 disk/part_efi.c           |  4 +--
 test/py/tests/test_gpt.py | 82 +++++++++++++++++++++++++++++++++++++++--------
 3 files changed, 77 insertions(+), 21 deletions(-)

-- 
2.7.4

[U-Boot] [PATCH v3 1/6] test/py: gpt: copy persistent file

[Patrick Delaunay]

copy the persistent gpt binary file as it can be modified during the test
that avoid issue if the test fail: the test always restart with clean file

Acked-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Changes in v3: None
Changes in v2:
- Split test to functional change

 test/py/tests/test_gpt.py | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/test/py/tests/test_gpt.py b/test/py/tests/test_gpt.py
index ec25fbb..e58bf61 100644
--- a/test/py/tests/test_gpt.py
+++ b/test/py/tests/test_gpt.py
@@ -28,26 +28,31 @@ class GptTestDiskImage(object):
         """
 
         filename = 'test_gpt_disk_image.bin'
-        self.path = u_boot_console.config.persistent_data_dir + '/' + filename
 
-        if os.path.exists(self.path):
-            u_boot_console.log.action('Disk image file ' + self.path +
+        persistent = u_boot_console.config.persistent_data_dir + '/' + filename
+        self.path = u_boot_console.config.result_dir  + '/' + filename
+
+        if os.path.exists(persistent):
+            u_boot_console.log.action('Disk image file ' + persistent +
                 ' already exists')
         else:
-            u_boot_console.log.action('Generating ' + self.path)
-            fd = os.open(self.path, os.O_RDWR | os.O_CREAT)
+            u_boot_console.log.action('Generating ' + persistent)
+            fd = os.open(persistent, os.O_RDWR | os.O_CREAT)
             os.ftruncate(fd, 4194304)
             os.close(fd)
             cmd = ('sgdisk', '-U', '375a56f7-d6c9-4e81-b5f0-09d41ca89efe',
-                self.path)
+                persistent)
             u_boot_utils.run_and_log(u_boot_console, cmd)
-            cmd = ('sgdisk', '--new=1:2048:2560', self.path)
+            cmd = ('sgdisk', '--new=1:2048:2560', persistent)
             u_boot_utils.run_and_log(u_boot_console, cmd)
-            cmd = ('sgdisk', '--new=2:4096:4608', self.path)
+            cmd = ('sgdisk', '--new=2:4096:4608', persistent)
             u_boot_utils.run_and_log(u_boot_console, cmd)
-            cmd = ('sgdisk', '-l', self.path)
+            cmd = ('sgdisk', '-l', persistent)
             u_boot_utils.run_and_log(u_boot_console, cmd)
 
+        cmd = ('cp', persistent, self.path)
+        u_boot_utils.run_and_log(u_boot_console, cmd)
+
 gtdi = None
 @pytest.fixture(scope='function')
 def state_disk_image(u_boot_console):
-- 
2.7.4

[U-Boot] [U-Boot,v3,1/6] test/py: gpt: copy persistent file

[Tom Rini]

On Wed, Oct 18, 2017 at 03:11:03PM +0200, Patrick Delaunay wrote:

> copy the persistent gpt binary file as it can be modified during the test
> that avoid issue if the test fail: the test always restart with clean file
> 
> Acked-by: Stephen Warren <swarren@nvidia.com>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>

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/20171024/241a4a35/attachment.sig>

[U-Boot] [PATCH v3 2/6] test/py: gpt: add test for sub-command read and verify

[Patrick Delaunay]

add sandbox test for some gpt sub-command
- gpt read / part list : read the gpt partition created by sgdisk on host
  test start, size, LBA and name output
- gpt verify : verify the gpt partition create by sgdisk on host

PS: persistent data test_gpt_disk_image.bin are udpated

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---
warning: need to erase previously create file
./build-sandbox/persistent-data/test_gpt_disk_image.bin

gpt test are OK on v2017.11-rc2:
./test/py/test.py -k gpt --build

test/py/tests/test_gpt.py ......

=> 6 passed, 229 deselected in 5.12 seconds

Changes in v3:
- update after Stephen Warren comments
- Add partition name in persistent data and test them

Changes in v2: None

 test/py/tests/test_gpt.py | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/test/py/tests/test_gpt.py b/test/py/tests/test_gpt.py
index e58bf61..5cfbf1f 100644
--- a/test/py/tests/test_gpt.py
+++ b/test/py/tests/test_gpt.py
@@ -43,9 +43,9 @@ class GptTestDiskImage(object):
             cmd = ('sgdisk', '-U', '375a56f7-d6c9-4e81-b5f0-09d41ca89efe',
                 persistent)
             u_boot_utils.run_and_log(u_boot_console, cmd)
-            cmd = ('sgdisk', '--new=1:2048:2560', persistent)
+            cmd = ('sgdisk', '--new=1:2048:2560', '-c 1:part1', persistent)
             u_boot_utils.run_and_log(u_boot_console, cmd)
-            cmd = ('sgdisk', '--new=2:4096:4608', persistent)
+            cmd = ('sgdisk', '--new=2:4096:4608', '-c 2:part2', persistent)
             u_boot_utils.run_and_log(u_boot_console, cmd)
             cmd = ('sgdisk', '-l', persistent)
             u_boot_utils.run_and_log(u_boot_console, cmd)
@@ -68,6 +68,33 @@ def state_disk_image(u_boot_console):
 
 @pytest.mark.boardspec('sandbox')
 @pytest.mark.buildconfigspec('cmd_gpt')
+ at pytest.mark.buildconfigspec('cmd_part')
+ at pytest.mark.requiredtool('sgdisk')
+def test_gpt_read(state_disk_image, u_boot_console):
+    """Test the gpt read command."""
+
+    u_boot_console.run_command('host bind 0 ' + state_disk_image.path)
+    output = u_boot_console.run_command('gpt read host 0')
+    assert 'Start 1MiB, size 0MiB' in output
+    assert 'Block size 512, name part1' in output
+    assert 'Start 2MiB, size 0MiB' in output
+    assert 'Block size 512, name part2' in output
+    output = u_boot_console.run_command('part list host 0')
+    assert '0x00000800	0x00000a00	"part1"' in output
+    assert '0x00001000	0x00001200	"part2"' in output
+
+ at pytest.mark.boardspec('sandbox')
+ at pytest.mark.buildconfigspec('cmd_gpt')
+ at pytest.mark.requiredtool('sgdisk')
+def test_gpt_verify(state_disk_image, u_boot_console):
+    """Test the gpt verify command."""
+
+    u_boot_console.run_command('host bind 0 ' + state_disk_image.path)
+    output = u_boot_console.run_command('gpt verify host 0')
+    assert 'Verify GPT: success!' in output
+
+ at pytest.mark.boardspec('sandbox')
+ at pytest.mark.buildconfigspec('cmd_gpt')
 @pytest.mark.requiredtool('sgdisk')
 def test_gpt_guid(state_disk_image, u_boot_console):
     """Test the gpt guid command."""
-- 
2.7.4

[U-Boot] [U-Boot, v3, 2/6] test/py: gpt: add test for sub-command read and verify

[Tom Rini]

On Wed, Oct 18, 2017 at 03:11:04PM +0200, Patrick Delaunay wrote:

> add sandbox test for some gpt sub-command
> - gpt read / part list : read the gpt partition created by sgdisk on host
>   test start, size, LBA and name output
> - gpt verify : verify the gpt partition create by sgdisk on host
> 
> PS: persistent data test_gpt_disk_image.bin are udpated
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>

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/20171024/84cc2124/attachment.sig>

[U-Boot] [PATCH v3 3/6] disk: efi: correct the overlap check on GPT header and PTE

[Patrick Delaunay]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 1832 bytes --]

the partition starting at 0x4400 is refused with overlap error:
  $> gpt write mmc 0 "name=test,start=0x4400,size=0"
  Writing GPT: Partition overlap
  error!

even if the 0x4400 is the first available offset for LBA35 with default
value:
- MBR=LBA1
- GPT header=LBA2
- PTE= 32 LBAs (128 entry), 3 to 34

And the command to have one partition for all the disk failed also :
  $> gpt write mmc 0 "name=test,size=0"

After the patch :

  $> gpt write mmc 0 "name=test,size=0"
  Writing GPT: success!
  $> part list mmc 0

  Partition Map for MMC device 0  --   Partition Type: EFI

  Part	Start LBA	End LBA		Name
	Attributes
	Type GUID
	Partition GUID
  1	0x00000022	0x01ce9fde	"test"
	attrs:	0x0000000000000000
	type:	ebd0a0a2-b9e5-4433-87c0-68b6b72699c7
	type:	data
	guid:	b4b84b8a-04e3-4000-0036-aff5c9c495b1

And 0x22 = 34 LBA => offset = 0x4400 is accepted as expected

Reviewed-by: Łukasz Majewski <lukma@denx.de>
Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---
gpt test are now OK
./test/py/test.py -k gpt --build

test/py/tests/test_gpt.py .......
=> 7 passed, 228 deselected in 1.11 seconds

Changes in v3: None
Changes in v2: None

 disk/part_efi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/disk/part_efi.c b/disk/part_efi.c
index 782f8be..7862bee 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -469,8 +469,8 @@ int gpt_fill_pte(struct blk_desc *dev_desc,
 		 * If our partition overlaps with either the GPT
 		 * header, or the partition entry, reject it.
 		 */
-		if (((start <= hdr_end && hdr_start <= (start + size)) ||
-		     (start <= pte_end && pte_start <= (start + size)))) {
+		if (((start < hdr_end && hdr_start < (start + size)) ||
+		     (start < pte_end && pte_start < (start + size)))) {
 			printf("Partition overlap\n");
 			return -1;
 		}
-- 
2.7.4

[U-Boot] [U-Boot, v3, 3/6] disk: efi: correct the overlap check on GPT header and PTE

[Tom Rini]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 1416 bytes --]

On Wed, Oct 18, 2017 at 03:11:05PM +0200, Patrick Delaunay wrote:

> the partition starting at 0x4400 is refused with overlap error:
>   $> gpt write mmc 0 "name=test,start=0x4400,size=0"
>   Writing GPT: Partition overlap
>   error!
> 
> even if the 0x4400 is the first available offset for LBA35 with default
> value:
> - MBR=LBA1
> - GPT header=LBA2
> - PTE= 32 LBAs (128 entry), 3 to 34
> 
> And the command to have one partition for all the disk failed also :
>   $> gpt write mmc 0 "name=test,size=0"
> 
> After the patch :
> 
>   $> gpt write mmc 0 "name=test,size=0"
>   Writing GPT: success!
>   $> part list mmc 0
> 
>   Partition Map for MMC device 0  --   Partition Type: EFI
> 
>   Part	Start LBA	End LBA		Name
> 	Attributes
> 	Type GUID
> 	Partition GUID
>   1	0x00000022	0x01ce9fde	"test"
> 	attrs:	0x0000000000000000
> 	type:	ebd0a0a2-b9e5-4433-87c0-68b6b72699c7
> 	type:	data
> 	guid:	b4b84b8a-04e3-4000-0036-aff5c9c495b1
> 
> And 0x22 = 34 LBA => offset = 0x4400 is accepted as expected
> 
> Reviewed-by: Łukasz Majewski <lukma@denx.de>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>

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/20171024/0a992428/attachment.sig>

[U-Boot] [PATCH v3 4/6] test/py: gpt: add test for sub-command write

[Patrick Delaunay]

+ test write for one partition on all the device (size=0)
+ test write with disk uuid and 2 partitions

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---
This write test failed without the previous patch with the error:
"Writing GPT: Partition overlap"

gpt test are now OK on v2017.11-rc2:
./test/py/test.py -k gpt --build

test/py/tests/test_gpt.py .......

=> 7 passed, 229 deselected in 5.23 seconds


Changes in v3:
- split test_gpt.py update: commit to add the test write command
- tests are now OK for each commit

Changes in v2: None

 test/py/tests/test_gpt.py | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/test/py/tests/test_gpt.py b/test/py/tests/test_gpt.py
index 5cfbf1f..2eb07a1 100644
--- a/test/py/tests/test_gpt.py
+++ b/test/py/tests/test_gpt.py
@@ -147,3 +147,23 @@ def test_gpt_swap_partitions(state_disk_image, u_boot_console):
     output = u_boot_console.run_command('part list host 0')
     assert '0x000007ff	"second"' in output
     assert '0x000017ff	"first"' in output
+
+ at pytest.mark.boardspec('sandbox')
+ at pytest.mark.buildconfigspec('cmd_gpt')
+ at pytest.mark.buildconfigspec('cmd_part')
+ at pytest.mark.requiredtool('sgdisk')
+def test_gpt_write(state_disk_image, u_boot_console):
+    """Test the gpt write command."""
+
+    u_boot_console.run_command('host bind 0 ' + state_disk_image.path)
+    output = u_boot_console.run_command('gpt write host 0 "name=all,size=0"')
+    assert 'Writing GPT: success!' in output
+    output = u_boot_console.run_command('part list host 0')
+    assert '0x00000022	0x00001fde	"all"' in output
+    output = u_boot_console.run_command('gpt write host 0 "uuid_disk=375a56f7-d6c9-4e81-b5f0-09d41ca89efe;name=first,start=0x100000,size=0x40200;name=second,start=0x200000,size=0x40200;"')
+    assert 'Writing GPT: success!' in output
+    output = u_boot_console.run_command('part list host 0')
+    assert '0x00000800	0x00000a00	"first"' in output
+    assert '0x00001000	0x00001200	"second"' in output
+    output = u_boot_console.run_command('gpt guid host 0')
+    assert '375a56f7-d6c9-4e81-b5f0-09d41ca89efe' in output
-- 
2.7.4

[U-Boot] [U-Boot, v3, 4/6] test/py: gpt: add test for sub-command write

[Tom Rini]

On Wed, Oct 18, 2017 at 03:11:06PM +0200, Patrick Delaunay wrote:

> + test write for one partition on all the device (size=0)
> + test write with disk uuid and 2 partitions
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>

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/20171024/dec5c084/attachment.sig>

[U-Boot] [PATCH v3 5/6] test/py: gpt: test start LBA for sub-command rename and swap

[Patrick Delaunay]

Add test of first and last LBA in gpt for rename and swap.
Only the name is expected to change, so test 3 columns
for part command
1: first LBA (start)
2: last LBA (end)
3: partition name

After rename, the last LBA change and it is a error in current U-Boot code
+ "first" = 0x7ff : invalid value (<start)
+ "second" = 0x17ff => size increasing !

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Before rename
0x00000800  0x00000a00      "part1"
0x00001000  0x00001200      "part2"

And after rename last LBA are invalid
0x00000800	0x000007ff "first"
0x00001000	0x000017ff	"second"

this issue will be corrected in next commit of the patchset

Changes in v3:
- Indicate LBA end error for rename command in test/py and commit message

Changes in v2: None

 test/py/tests/test_gpt.py | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/test/py/tests/test_gpt.py b/test/py/tests/test_gpt.py
index 2eb07a1..b7adc10 100644
--- a/test/py/tests/test_gpt.py
+++ b/test/py/tests/test_gpt.py
@@ -119,6 +119,7 @@ def test_gpt_save_guid(state_disk_image, u_boot_console):
 @pytest.mark.boardspec('sandbox')
 @pytest.mark.buildconfigspec('cmd_gpt')
 @pytest.mark.buildconfigspec('cmd_gpt_rename')
+ at pytest.mark.buildconfigspec('cmd_part')
 @pytest.mark.requiredtool('sgdisk')
 def test_gpt_rename_partition(state_disk_image, u_boot_console):
     """Test the gpt rename command to write partition names."""
@@ -130,6 +131,13 @@ def test_gpt_rename_partition(state_disk_image, u_boot_console):
     u_boot_console.run_command('gpt rename host 0 2 second')
     output = u_boot_console.run_command('gpt read host 0')
     assert 'name second' in output
+    output = u_boot_console.run_command('part list host 0')
+    assert '0x00000800	0x000007ff	"first"' in output
+    assert '0x00001000	0x000017ff	"second"' in output
+    # command error here because 'end LBA' (column 2) change after rename
+    # (previous value can be found in test_gpt_read)
+    # "first" 0xa00 => 0x7ff : it is an invalid value < start LBA !
+    # "seconf" 0x1200 => 0x17ff : size is increasing !
 
 @pytest.mark.boardspec('sandbox')
 @pytest.mark.buildconfigspec('cmd_gpt')
@@ -141,12 +149,12 @@ def test_gpt_swap_partitions(state_disk_image, u_boot_console):
 
     u_boot_console.run_command('host bind 0 ' + state_disk_image.path)
     output = u_boot_console.run_command('part list host 0')
-    assert '0x000007ff	"first"' in output
-    assert '0x000017ff	"second"' in output
+    assert '0x00000800	0x000007ff	"first"' in output
+    assert '0x00001000	0x000017ff	"second"' in output
     u_boot_console.run_command('gpt swap host 0 first second')
     output = u_boot_console.run_command('part list host 0')
-    assert '0x000007ff	"second"' in output
-    assert '0x000017ff	"first"' in output
+    assert '0x00000800	0x000007ff	"second"' in output
+    assert '0x00001000	0x000017ff	"first"' in output
 
 @pytest.mark.boardspec('sandbox')
 @pytest.mark.buildconfigspec('cmd_gpt')
-- 
2.7.4

[U-Boot] [U-Boot, v3, 5/6] test/py: gpt: test start LBA for sub-command rename and swap

[Tom Rini]

On Wed, Oct 18, 2017 at 03:11:07PM +0200, Patrick Delaunay wrote:

> Add test of first and last LBA in gpt for rename and swap.
> Only the name is expected to change, so test 3 columns
> for part command
> 1: first LBA (start)
> 2: last LBA (end)
> 3: partition name
> 
> After rename, the last LBA change and it is a error in current U-Boot code
> + "first" = 0x7ff : invalid value (<start)
> + "second" = 0x17ff => size increasing !
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>

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/20171024/dd523f2f/attachment.sig>

[U-Boot] [PATCH v3 6/6] cmd: gpt: solve issue for swap and rename command

[Patrick Delaunay]

don't use prettyprint_part_size() in create_gpt_partitions_list()
that avoid to align offset and size to 1 MiB and increase precision for
start and size.
This patch avoid the risk to change partition size and lost data during
rename or swap.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Changes in v3: None
Changes in v2: None

 cmd/gpt.c                 | 12 ++++++------
 test/py/tests/test_gpt.py | 16 ++++++----------
 2 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/cmd/gpt.c b/cmd/gpt.c
index 27dd987..707d861 100644
--- a/cmd/gpt.c
+++ b/cmd/gpt.c
@@ -282,14 +282,14 @@ static int create_gpt_partitions_list(int numparts, const char *guid,
 		strcat(partitions_list, "name=");
 		strncat(partitions_list, (const char *)curr->gpt_part_info.name,
 			PART_NAME_LEN + 1);
-		strcat(partitions_list, ",start=");
-		prettyprint_part_size(partstr, (unsigned long)curr->gpt_part_info.start,
-				      (unsigned long) curr->gpt_part_info.blksz);
+		sprintf(partstr, ",start=0x%llx",
+			(unsigned long long)curr->gpt_part_info.start *
+					    curr->gpt_part_info.blksz);
 		/* one extra byte for NULL */
 		strncat(partitions_list, partstr, PART_NAME_LEN + 1);
-		strcat(partitions_list, ",size=");
-		prettyprint_part_size(partstr, curr->gpt_part_info.size,
-				      curr->gpt_part_info.blksz);
+		sprintf(partstr, ",size=0x%llx",
+			(unsigned long long)curr->gpt_part_info.size *
+					    curr->gpt_part_info.blksz);
 		strncat(partitions_list, partstr, PART_NAME_LEN + 1);
 
 		strcat(partitions_list, ",uuid=");
diff --git a/test/py/tests/test_gpt.py b/test/py/tests/test_gpt.py
index b7adc10..b9b5e5f 100644
--- a/test/py/tests/test_gpt.py
+++ b/test/py/tests/test_gpt.py
@@ -132,12 +132,8 @@ def test_gpt_rename_partition(state_disk_image, u_boot_console):
     output = u_boot_console.run_command('gpt read host 0')
     assert 'name second' in output
     output = u_boot_console.run_command('part list host 0')
-    assert '0x00000800	0x000007ff	"first"' in output
-    assert '0x00001000	0x000017ff	"second"' in output
-    # command error here because 'end LBA' (column 2) change after rename
-    # (previous value can be found in test_gpt_read)
-    # "first" 0xa00 => 0x7ff : it is an invalid value < start LBA !
-    # "seconf" 0x1200 => 0x17ff : size is increasing !
+    assert '0x00000800	0x00000a00	"first"' in output
+    assert '0x00001000	0x00001200	"second"' in output
 
 @pytest.mark.boardspec('sandbox')
 @pytest.mark.buildconfigspec('cmd_gpt')
@@ -149,12 +145,12 @@ def test_gpt_swap_partitions(state_disk_image, u_boot_console):
 
     u_boot_console.run_command('host bind 0 ' + state_disk_image.path)
     output = u_boot_console.run_command('part list host 0')
-    assert '0x00000800	0x000007ff	"first"' in output
-    assert '0x00001000	0x000017ff	"second"' in output
+    assert '0x00000800	0x00000a00	"first"' in output
+    assert '0x00001000	0x00001200	"second"' in output
     u_boot_console.run_command('gpt swap host 0 first second')
     output = u_boot_console.run_command('part list host 0')
-    assert '0x00000800	0x000007ff	"second"' in output
-    assert '0x00001000	0x000017ff	"first"' in output
+    assert '0x00000800	0x00000a00	"second"' in output
+    assert '0x00001000	0x00001200	"first"' in output
 
 @pytest.mark.boardspec('sandbox')
 @pytest.mark.buildconfigspec('cmd_gpt')
-- 
2.7.4

[U-Boot] [PATCH v3 6/6] cmd: gpt: solve issue for swap and rename command

[Stephen Warren]

On 10/18/2017 07:11 AM, Patrick Delaunay wrote:
> don't use prettyprint_part_size() in create_gpt_partitions_list()
> that avoid to align offset and size to 1 MiB and increase precision for
> start and size.
> This patch avoid the risk to change partition size and lost data during
> rename or swap.

All the test/py changes in this series,
Acked-by: Stephen Warren <swarren@nvidia.com>

The series,
Tested-by: Stephen Warren <swarren@nvidia.com>

If you're looking for more things to change(!), perhaps change the 
sgdisk commands so that the partitions are big enough that 'gpt read 
host 0' says something other than 'size 0MiB' for the partitions; when I 
saw that, I was initially rather confused until I realized that the 
partitions were less than 1MiB!

[U-Boot] [PATCH v3 6/6] cmd: gpt: solve issue for swap and rename command

[Patrick DELAUNAY]

Hi,

> -----Original Message-----
> From: Stephen Warren [mailto:swarren at wwwdotorg.org]
> 
> On 10/18/2017 07:11 AM, Patrick Delaunay wrote:
> > don't use prettyprint_part_size() in create_gpt_partitions_list() that
> > avoid to align offset and size to 1 MiB and increase precision for
> > start and size.
> > This patch avoid the risk to change partition size and lost data
> > during rename or swap.
> 
> All the test/py changes in this series,
> Acked-by: Stephen Warren <swarren@nvidia.com>
> 
> The series,
> Tested-by: Stephen Warren <swarren@nvidia.com>
> 
> If you're looking for more things to change(!), perhaps change the sgdisk
> commands so that the partitions are big enough that 'gpt read host 0' says
> something other than 'size 0MiB' for the partitions; when I saw that, I was
> initially rather confused until I realized that the partitions were less than 1MiB!

I don't really looking...
but I agree: size 0MiB is confusing, I had the same reaction the first time.

So I will propose something when this serie will be accepted,
just update the test to have 2 partitions with different size and > 1MiB.

Regards
Patrick

[U-Boot] [U-Boot, v3, 6/6] cmd: gpt: solve issue for swap and rename command

[Tom Rini]

On Wed, Oct 18, 2017 at 03:11:08PM +0200, Patrick Delaunay wrote:

> don't use prettyprint_part_size() in create_gpt_partitions_list()
> that avoid to align offset and size to 1 MiB and increase precision for
> start and size.
> This patch avoid the risk to change partition size and lost data during
> rename or swap.
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> Acked-by: Stephen Warren <swarren@nvidia.com>
> Tested-by: Stephen Warren <swarren@nvidia.com>

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/20171024/f67690d3/attachment.sig>