From b0da703f46c87b9fdf339b282139783ca5151ccf Mon Sep 17 00:00:00 2001
From: Ian Wienand <iwienand@redhat.com>
Date: Wed, 27 Jun 2018 13:51:04 +1000
Subject: [PATCH] Don't quote names with sgdisk

Our sgdisk calls are putting extra double-quotes around the names of
partitions.  This confuses sfdisk, which confuses growpart, which
confuses growroot ... and you don't get your partition grown for EFI
boot.

Ensure we just bunch arguments into the list directly (for Popen)
rather than string split and have to worry about quoting.  Add a check
for this to our GPT unit test, extending it to include a space in the
name of the root partition.

Change-Id: I0a8cb69bb4c9c0865fbaa63ba0d7210028da552e
---
 .../block_device/level1/partitioning.py             | 10 ++++++----
 .../block_device/tests/config/gpt_efi.yaml          |  4 +++-
 diskimage_builder/block_device/tests/test_gpt.py    | 13 ++++++-------
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/diskimage_builder/block_device/level1/partitioning.py b/diskimage_builder/block_device/level1/partitioning.py
index bc2a728ef..ca949076c 100644
--- a/diskimage_builder/block_device/level1/partitioning.py
+++ b/diskimage_builder/block_device/level1/partitioning.py
@@ -136,7 +136,7 @@ class Partitioning(PluginBase):
         for p in self.partitions:
             args = {}
             args['pnum'] = pnum
-            args['name'] = '"%s"' % p.get_name()
+            args['name'] = '%s' % p.get_name()
             args['type'] = '%s' % p.get_type()
 
             # convert from a relative/string size to bytes
@@ -149,9 +149,11 @@ class Partitioning(PluginBase):
             assert size <= disk_free
             args['size'] = size // (1024 * 1024)
 
-            new_cmd = ("-n {pnum}:0:+{size}M -t {pnum}:{type} "
-                       "-c {pnum}:{name}".format(**args))
-            cmd.extend(new_cmd.strip().split(' '))
+            new_cmd = ("-n", "{pnum}:0:+{size}M".format(**args),
+                       "-t", "{pnum}:{type}".format(**args),
+                       # Careful with this one, as {name} could have spaces
+                       "-c", "{pnum}:{name}".format(**args))
+            cmd.extend(new_cmd)
 
             # Fill the state; we mount all partitions with kpartx
             # below once we're done.  So the device this partition
diff --git a/diskimage_builder/block_device/tests/config/gpt_efi.yaml b/diskimage_builder/block_device/tests/config/gpt_efi.yaml
index 9ef2f682f..c5fd8aee2 100644
--- a/diskimage_builder/block_device/tests/config/gpt_efi.yaml
+++ b/diskimage_builder/block_device/tests/config/gpt_efi.yaml
@@ -20,7 +20,9 @@
       - name: BSP
         type: 'EF02'
         size: 8MiB
-      - name: root
+        # spaces are probably a bad idea for max compatability, but
+        # we're deliberatly testing it here.
+      - name: Root Part
         type: '8300'
         size: 100%
         mkfs:
diff --git a/diskimage_builder/block_device/tests/test_gpt.py b/diskimage_builder/block_device/tests/test_gpt.py
index bc2009800..db046282d 100644
--- a/diskimage_builder/block_device/tests/test_gpt.py
+++ b/diskimage_builder/block_device/tests/test_gpt.py
@@ -63,13 +63,12 @@ class TestGPT(tc.TestGraphGeneration):
                 node.create()
 
         # check the parted call looks right
-        parted_cmd = ('sgdisk %s '
-                      '-n 1:0:+8M -t 1:EF00 -c 1:"ESP" '
-                      '-n 2:0:+8M -t 2:EF02 -c 2:"BSP" '
-                      '-n 3:0:+1006M -t 3:8300 -c 3:"root"'
-                      % self.image_path)
+        parted_cmd = ['sgdisk', self.image_path,
+                      '-n', '1:0:+8M', '-t', '1:EF00', '-c', '1:ESP',
+                      '-n', '2:0:+8M', '-t', '2:EF02', '-c', '2:BSP',
+                      '-n', '3:0:+1006M', '-t', '3:8300', '-c', '3:Root Part']
         cmd_sequence = [
-            mock.call(parted_cmd.split(' ')),
+            mock.call(parted_cmd),
             mock.call(['sync']),
             mock.call(['kpartx', '-avs', '/dev/loopX'])
         ]
@@ -81,5 +80,5 @@ class TestGPT(tc.TestGraphGeneration):
                              {'device': '/dev/mapper/loopXp1'})
         self.assertDictEqual(state['blockdev']['BSP'],
                              {'device': '/dev/mapper/loopXp2'})
-        self.assertDictEqual(state['blockdev']['root'],
+        self.assertDictEqual(state['blockdev']['Root Part'],
                              {'device': '/dev/mapper/loopXp3'})