From 7e8c7adaace58d960763225b459a0fc3739f62ee Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Fri, 26 May 2023 15:09:49 -0700 Subject: [PATCH] Always prefer GPT disk labels on x86_64 (and clean up the logic) See: https://bugzilla.redhat.com/show_bug.cgi?id=2092091#c6 There was a Fedora 37 Change to prefer GPT disk labels on x86_64 BIOS installs, but for some reason, this was implemented in anaconda by having it ignore blivet's ordering of the disk label types and just universally prefer GPT if it's in the list at all, which resulted in a preference for GPT in more cases than just x86_64 BIOS. This is one step towards fixing that, by putting the 'always prefer GPT on x86_64' logic here in the blivet ordering code where it belongs. Step 2 will be to drop the anaconda code that overrides blivet's preference order. This also simplifies the logic a bit; it had gotten rather a lot of conditions piled on top of each other and was rather hard to read. This should achieve the same effect as before in a clearer and more concise way. Signed-off-by: Adam Williamson --- blivet/formats/disklabel.py | 7 ++----- tests/storage_tests/devices_test/partition_test.py | 2 +- tests/unit_tests/formats_tests/disklabel_test.py | 10 ++++++++++ 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/blivet/formats/disklabel.py b/blivet/formats/disklabel.py index 72df9d67..5b4b0a85 100644 --- a/blivet/formats/disklabel.py +++ b/blivet/formats/disklabel.py @@ -223,11 +223,8 @@ class DiskLabel(DeviceFormat): label_types = ["msdos", "gpt"] if arch.is_pmac(): label_types = ["mac"] - elif arch.is_aarch64(): - label_types = ["gpt", "msdos"] - elif arch.is_efi() and arch.is_arm(): - label_types = ["msdos", "gpt"] - elif arch.is_efi() and not arch.is_aarch64(): + # always prefer gpt on aarch64, x86_64, and EFI plats except 32-bit ARM + elif arch.is_aarch64() or arch.is_x86(bits=64) or (arch.is_efi() and not arch.is_arm()): label_types = ["gpt", "msdos"] elif arch.is_s390(): label_types += ["dasd"] diff --git a/tests/storage_tests/devices_test/partition_test.py b/tests/storage_tests/devices_test/partition_test.py index ba01c801..d3ff78a3 100644 --- a/tests/storage_tests/devices_test/partition_test.py +++ b/tests/storage_tests/devices_test/partition_test.py @@ -99,7 +99,7 @@ class PartitionDeviceTestCase(unittest.TestCase): def test_min_max_size_alignment(self): with sparsetmpfile("minsizetest", Size("10 MiB")) as disk_file: disk = DiskFile(disk_file) - disk.format = get_format("disklabel", device=disk.path) + disk.format = get_format("disklabel", device=disk.path, label_type="msdos") grain_size = Size(disk.format.alignment.grainSize) sector_size = Size(disk.format.parted_device.sectorSize) start = int(grain_size) diff --git a/tests/unit_tests/formats_tests/disklabel_test.py b/tests/unit_tests/formats_tests/disklabel_test.py index f514a778..a7f5e777 100644 --- a/tests/unit_tests/formats_tests/disklabel_test.py +++ b/tests/unit_tests/formats_tests/disklabel_test.py @@ -75,6 +75,7 @@ class DiskLabelTestCase(unittest.TestCase): arch.is_aarch64.return_value = False arch.is_arm.return_value = False arch.is_pmac.return_value = False + arch.is_x86.return_value = False self.assertEqual(disklabel_class.get_platform_label_types(), ["msdos", "gpt"]) @@ -96,6 +97,14 @@ class DiskLabelTestCase(unittest.TestCase): self.assertEqual(disklabel_class.get_platform_label_types(), ["msdos", "gpt"]) arch.is_arm.return_value = False + # this simulates x86_64 + arch.is_x86.return_value = True + self.assertEqual(disklabel_class.get_platform_label_types(), ["gpt", "msdos"]) + arch.is_efi.return_value = True + self.assertEqual(disklabel_class.get_platform_label_types(), ["gpt", "msdos"]) + arch.is_x86.return_value = False + arch.is_efi.return_value = False + arch.is_s390.return_value = True self.assertEqual(disklabel_class.get_platform_label_types(), ["msdos", "gpt", "dasd"]) arch.is_s390.return_value = False @@ -138,6 +147,7 @@ class DiskLabelTestCase(unittest.TestCase): arch.is_aarch64.return_value = False arch.is_arm.return_value = False arch.is_pmac.return_value = False + arch.is_x86.return_value = False with mock.patch.object(dl, '_label_type_size_check') as size_check: # size check passes for first type ("msdos") -- 2.40.1