Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make GPT default label type on all architectures #1317

Merged
merged 1 commit into from
Nov 19, 2024
Merged

Make GPT default label type on all architectures #1317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions blivet/formats/disklabel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,12 +220,13 @@ def parted_device(self):

@classmethod
def get_platform_label_types(cls):
label_types = ["msdos", "gpt"]
# always prefer gpt except for configurations below
label_types = ["gpt", "msdos"]
if arch.is_pmac():
label_types = ["mac"]
# 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"]
# prefet msdos on 32-bit ARM
vojtechtrefny marked this conversation as resolved.
Show resolved Hide resolved
elif arch.is_arm():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's confusing to me that is_arm means 32 bit, is this because the other arch string is aarch64?

Maybe worth a followup to rename is_arm to is_arm32 or so

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name is unfortunate, I had to check myself what is_arm actually means. We have arch.is_arm which means 32bit ARM and arch.is_aarch64 for 64bit ARM.

label_types = ["msdos", "gpt"]
elif arch.is_s390():
label_types += ["dasd"]

Expand Down Expand Up @@ -254,7 +255,7 @@ def _get_best_label_type(self):
if arch.is_s390():
if blockdev.s390.dasd_is_fba(self.device):
# the device is FBA DASD
return "msdos"
return "gpt"
elif self.parted_device.type == parted.DEVICE_DASD:
# the device is DASD
return "dasd"
Expand Down
20 changes: 10 additions & 10 deletions tests/unit_tests/formats_tests/disklabel_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def test_platform_label_types(self, arch):
arch.is_pmac.return_value = False
arch.is_x86.return_value = False

self.assertEqual(disklabel_class.get_platform_label_types(), ["msdos", "gpt"])
self.assertEqual(disklabel_class.get_platform_label_types(), ["gpt", "msdos"])

arch.is_pmac.return_value = True
self.assertEqual(disklabel_class.get_platform_label_types(), ["mac"])
Expand Down Expand Up @@ -100,7 +100,7 @@ def test_platform_label_types(self, arch):
arch.is_efi.return_value = False

arch.is_s390.return_value = True
self.assertEqual(disklabel_class.get_platform_label_types(), ["msdos", "gpt", "dasd"])
self.assertEqual(disklabel_class.get_platform_label_types(), ["gpt", "msdos", "dasd"])
arch.is_s390.return_value = False

def test_label_type_size_check(self):
Expand All @@ -121,14 +121,14 @@ def test_label_type_size_check(self):

with patch.object(blivet.formats.disklabel.DiskLabel, "parted_device", new=PropertyMock(return_value=None)):
# no parted device -> no passing size check
self.assertEqual(dl._label_type_size_check("msdos"), False)
self.assertEqual(dl._label_type_size_check("gpt"), False)

@patch("blivet.formats.disklabel.arch")
def test_best_label_type(self, arch):
"""
1. is always in _disklabel_types
2. is the default unless the device is too long for the default
3. is msdos for fba dasd on S390
3. is gpt for fba dasd on S390
4. is dasd for non-fba dasd on S390
"""
dl = blivet.formats.disklabel.DiskLabel()
Expand All @@ -144,17 +144,17 @@ def test_best_label_type(self, arch):
arch.is_x86.return_value = False

with patch.object(dl, '_label_type_size_check') as size_check:
# size check passes for first type ("msdos")
# size check passes for first type ("gpt")
size_check.return_value = True
self.assertEqual(dl._get_best_label_type(), "msdos")
self.assertEqual(dl._get_best_label_type(), "gpt")

# size checks all fail -> label type is None
size_check.return_value = False
self.assertEqual(dl._get_best_label_type(), None)

# size check passes on second call -> label type is "gpt" (second in platform list)
# size check passes on second call -> label type is "msdos" (second in platform list)
size_check.side_effect = [False, True]
self.assertEqual(dl._get_best_label_type(), "gpt")
self.assertEqual(dl._get_best_label_type(), "msdos")

arch.is_pmac.return_value = True
with patch.object(dl, '_label_type_size_check') as size_check:
Expand All @@ -175,10 +175,10 @@ def test_best_label_type(self, arch):
size_check.return_value = True
with patch("blivet.formats.disklabel.blockdev.s390") as _s390:
_s390.dasd_is_fba.return_value = False
self.assertEqual(dl._get_best_label_type(), "msdos")
self.assertEqual(dl._get_best_label_type(), "gpt")

_s390.dasd_is_fba.return_value = True
self.assertEqual(dl._get_best_label_type(), "msdos")
self.assertEqual(dl._get_best_label_type(), "gpt")

_s390.dasd_is_fba.return_value = False
dl._parted_device.type = parted.DEVICE_DASD
Expand Down