curtin-dev team mailing list archive

[Olivier Gayot <mp+481790@xxxxxxxxxxxxxxxxxx>]

Olivier Gayot has proposed merging ~ogayot/curtin:gpt-partname into curtin:master.

Commit message:
do not squash

Requested reviews:
  curtin developers (curtin-dev)
Related bugs:
  Bug #2099893 in curtin: "Support reading the GPT partition name in Subiquity"
  https://bugs.launchpad.net/curtin/+bug/2099893

For more details, see:
https://code.launchpad.net/~ogayot/curtin/+git/curtin/+merge/481790

Add support for reading the GPT partition name in storage_config.

We read the ID_PART_ENTRY_NAME value, which is set by libblkid [1] and encoded using the blkid_encode_string function [2]

[1] https://github.com/util-linux/util-linux/blob/3564de4d10aa0f4b51ab8b7d8b5fa268fa770720/libblkid/src/partitions/partitions.c#L774
[2] https://github.com/util-linux/util-linux/blob/3564de4d10aa0f4b51ab8b7d8b5fa268fa770720/libblkid/src/encode.c#L176
-- 
Your team curtin developers is requested to review the proposed merge of ~ogayot/curtin:gpt-partname into curtin:master.

diff --git a/curtin/block/schemas.py b/curtin/block/schemas.py
index 503e870..8e76d28 100644
--- a/curtin/block/schemas.py
+++ b/curtin/block/schemas.py
@@ -329,6 +329,7 @@ PARTITION = {
                                {'pattern': r'^0x[0-9a-fA-F]{1,2}$'},
                                {'$ref': '#/definitions/uuid'},
                                ]},
+        'partition_name': {'$ref': '#definitions/name'},
         'grub_device': {
             'type': ['boolean', 'integer'],
             'minimum': 0,
diff --git a/curtin/storage_config.py b/curtin/storage_config.py
index bff7ea0..56bcb6e 100644
--- a/curtin/storage_config.py
+++ b/curtin/storage_config.py
@@ -1,5 +1,6 @@
 # This file is part of curtin. See LICENSE file for copyright and license info.
 from collections import namedtuple, OrderedDict
+import contextlib
 import copy
 import operator
 import os
@@ -403,6 +404,14 @@ def extract_storage_ordered_dict(config):
     return OrderedDict((d["id"], d) for d in scfg)
 
 
+def decode_libblkid_string(encoded: str) -> str:
+    r""" Decode a string encoded using blkid_encode_string() - where unsafe
+       characters are replaced by \x{...} sequences.
+       Note that this is a different encoding than what is used when running
+       the command "$ blkid". The latter is based on M- and ^ sequences. """
+    return encoded.encode('raw-unicode-escape').decode('unicode-escape')
+
+
 class ProbertParser(object):
     """ Base class for parsing probert storage configuration.
 
@@ -817,6 +826,20 @@ class BlockdevParser(ProbertParser):
         if entry['type'] == 'partition':
             if devname:
                 entry['path'] = devname
+            with contextlib.suppress(KeyError):
+                # For the name of the partition, there are two properties we
+                # can consider using:
+                # * PARTNAME: At least for GPT, if the partition name is not
+                # ASCII, the value will get mangled by the kernel, see LP:
+                # #2017862.  In some scenarios, it will even make probert choke
+                # and omit the PARTNAME property entirely.
+                # * ID_PART_ENTRY_NAME: This one is added by libblkid and
+                # should not be omitted by probert. However, it is encoded
+                # using blkid_encode_string, which turns spaces and other
+                # characters into \x{...} sequences.
+                entry['partition_name'] = decode_libblkid_string(
+                    blockdev_data['ID_PART_ENTRY_NAME']
+                )
             attrs = blockdev_data['attrs']
             if self.is_mpath_partition(blockdev_data):
                 entry['number'] = int(blockdev_data['DM_PART'])
diff --git a/tests/unittests/test_storage_config.py b/tests/unittests/test_storage_config.py
index 3ce1086..d86a282 100644
--- a/tests/unittests/test_storage_config.py
+++ b/tests/unittests/test_storage_config.py
@@ -3,12 +3,15 @@ import copy
 import json
 from unittest import mock
 
+from parameterized import parameterized
+
 from .helpers import CiTestCase, skipUnlessJsonSchema
 from curtin import storage_config
 from curtin.storage_config import ProbertParser as baseparser
 from curtin.storage_config import (BcacheParser, BlockdevParser, DasdParser,
                                    DmcryptParser, FilesystemParser, LvmParser,
                                    RaidParser, MountParser, ZfsParser)
+from curtin.storage_config import decode_libblkid_string
 from curtin.storage_config import ptable_part_type_to_flag, select_configs
 from curtin.storage_config import LOG as SCLogger
 from curtin import util
@@ -107,6 +110,34 @@ class TestStorageConfigSchema(CiTestCase):
             storage_config.validate_config(config)
 
 
+class TestDecodeLibblkidString(CiTestCase):
+    def test_easy(self):
+        self.assertEqual("Hello", decode_libblkid_string("Hello"))
+
+    def test_ascii_with_symbols(self):
+        self.assertEqual("Hello, World!",
+                         decode_libblkid_string(r"Hello\x2c\x20World\x21"))
+
+    @parameterized.expand((
+        ('¡Mi partición!', '¡Mi\\x20partición\\x21'),
+        ('Мой раздел!', 'Мой\\x20раздел\\x21'),
+        ('我的分区！', '我的分区！'),
+        ('私のパーティション！', '私のパーティション！'),
+        ('내 파티션!', '내\\x20파티션\\x21'),
+        ('قسمتي!', 'قسمتي\\x21'),
+        ('Mój partycja!', 'Mój\\x20partycja\\x21'),
+        ('मेरी पार्टीशन!', 'मेरी\\x20पार्टीशन\\x21'),
+        ('جزءي!', 'جزءي\\x21'),
+        ('Таман!', 'Таман\\x21'),
+        ('我的磁盘分区！', '我的磁盘分区！'),
+        ('मेरी विभाजन!', 'मेरी\\x20विभाजन\\x21'),
+        ('我的磁碟分區!', '我的磁碟分區\\x21'),
+    ))
+    def test_multibyte_utf8(self, name: str, encoded_name: str):
+        # Multibyte characters are typically not escaped
+        self.assertEqual(name, decode_libblkid_string(encoded_name))
+
+
 class TestProbertParser(CiTestCase):
 
     invalid_inputs = [None, '', {}, [], set()]
@@ -468,6 +499,27 @@ class TestBlockdevParser(CiTestCase):
         self.assertDictEqual(expected_dict,
                              self.bdevp.asdict(blockdev))
 
+    def test_blockdev_asdict_partition_with_name(self):
+        """ BlockdevParser creates dictionary of DEVTYPE=partition. """
+
+        blockdev = self.bdevp.blockdev_data['/dev/nvme0n1p2']
+        expected_dict = {
+            'id': 'partition-nvme0n1p2',
+            'type': 'partition',
+            'device': 'disk-nvme0n1',
+            'path': '/dev/nvme0n1p2',
+            'number': 2,
+            'offset': 234375168 * 512,
+            'size': 419430400 * 512,
+            'flag': 'linux',
+            'partition_name': 'Linux filesystem',
+            'partition_type': '0fc63daf-8483-4772-8e79-3d69d8477de4',
+            'uuid': '789835f9-0ca5-451a-af2d-d642934d83a6',
+        }
+        self.assertDictEqual(expected_dict,
+                             self.bdevp.asdict(blockdev))
+
+
     # XXX: Parameterize me
     def test_blockdev_asdict_not_disk_or_partition(self):
         """ BlockdevParser ignores DEVTYPE not in 'disk, partition'. """