From 683176dbeeeff32cc6b04410b4f7e4715a3de8e0 Mon Sep 17 00:00:00 2001 From: Petr Stodulka Date: Wed, 24 Apr 2024 01:09:51 +0200 Subject: [PATCH 15/34] boot: Skip checks of first partition offset for for gpt partition table This is extension of the previous commit. The original problem that we are trying to resolve is to be sure the embedding area (MBR gap) has expected size. This is irrelevant in case of GPT partition table is used on a device. The fdisk output format is in case of GPT disk label different, which breaks the parsing, resulting in empty list of partitions in related GRUBDevicePartitionLayout msg. For now, let's skip produce of msgs for "GPT devices". As a seatbelt, ignore processing of messages with empty partitions field, expecting that such a device does not contain MBR. We want to prevent false positive inhibitors (and FP blocking errors). We expect that total number of machines with small embedding area is very minor in total numbers, so even if we would miss something (which is not expected now to our best knowledge) it's still good trade-off as the major goal is to reduce number of machines that have problems with the in-place upgrade. The solution can be updated in future if there is a reason for it. --- .../libraries/check_first_partition_offset.py | 7 +++++ .../test_check_first_partition_offset.py | 16 +++++++++++ .../libraries/scan_layout.py | 28 +++++++++++++++++++ .../tests/test_scan_partition_layout.py | 5 ++++ 4 files changed, 56 insertions(+) diff --git a/repos/system_upgrade/el7toel8/actors/checkfirstpartitionoffset/libraries/check_first_partition_offset.py b/repos/system_upgrade/el7toel8/actors/checkfirstpartitionoffset/libraries/check_first_partition_offset.py index fbd4e178..fca9c3ff 100644 --- a/repos/system_upgrade/el7toel8/actors/checkfirstpartitionoffset/libraries/check_first_partition_offset.py +++ b/repos/system_upgrade/el7toel8/actors/checkfirstpartitionoffset/libraries/check_first_partition_offset.py @@ -16,6 +16,13 @@ def check_first_partition_offset(): problematic_devices = [] for grub_dev in api.consume(GRUBDevicePartitionLayout): + if not grub_dev.partitions: + # NOTE(pstodulk): In case of empty partition list we have nothing to do. + # This can could happen when the fdisk output is different then expected. + # E.g. when GPT partition table is used on the disk. We are right now + # interested strictly about MBR only, so ignoring these cases. + # This is seatbelt, as the msg should not be produced for GPT at all. + continue first_partition = min(grub_dev.partitions, key=lambda partition: partition.start_offset) if first_partition.start_offset < SAFE_OFFSET_BYTES: problematic_devices.append(grub_dev.device) diff --git a/repos/system_upgrade/el7toel8/actors/checkfirstpartitionoffset/tests/test_check_first_partition_offset.py b/repos/system_upgrade/el7toel8/actors/checkfirstpartitionoffset/tests/test_check_first_partition_offset.py index e349ff7d..f925f7d4 100644 --- a/repos/system_upgrade/el7toel8/actors/checkfirstpartitionoffset/tests/test_check_first_partition_offset.py +++ b/repos/system_upgrade/el7toel8/actors/checkfirstpartitionoffset/tests/test_check_first_partition_offset.py @@ -20,6 +20,16 @@ from leapp.utils.report import is_inhibitor ], True ), + ( + [ + GRUBDevicePartitionLayout(device='/dev/vda', + partitions=[ + PartitionInfo(part_device='/dev/vda2', start_offset=1024*1025), + PartitionInfo(part_device='/dev/vda1', start_offset=32256) + ]) + ], + True + ), ( [ GRUBDevicePartitionLayout(device='/dev/vda', @@ -33,6 +43,12 @@ from leapp.utils.report import is_inhibitor partitions=[PartitionInfo(part_device='/dev/vda1', start_offset=1024*1024)]) ], False + ), + ( + [ + GRUBDevicePartitionLayout(device='/dev/vda', partitions=[]) + ], + False ) ] ) diff --git a/repos/system_upgrade/el7toel8/actors/scangrubdevpartitionlayout/libraries/scan_layout.py b/repos/system_upgrade/el7toel8/actors/scangrubdevpartitionlayout/libraries/scan_layout.py index bb2e6d9e..f51bcda4 100644 --- a/repos/system_upgrade/el7toel8/actors/scangrubdevpartitionlayout/libraries/scan_layout.py +++ b/repos/system_upgrade/el7toel8/actors/scangrubdevpartitionlayout/libraries/scan_layout.py @@ -31,6 +31,34 @@ def get_partition_layout(device): unit = int(unit.split(' ')[0].strip()) break # First line of the partition table header + # Discover disk label type: dos | gpt + for line in table_iter: + line = line.strip() + if not line.startswith('Disk label type'): + continue + disk_type = line.split(':')[1].strip() + break + + if disk_type == 'gpt': + api.current_logger().info( + 'Detected GPT partition table. Skipping produce of GRUBDevicePartitionLayout message.' + ) + # NOTE(pstodulk): The GPT table has a different output format than + # expected below, example (ignore start/end lines): + # --------------------------- start ---------------------------------- + # # Start End Size Type Name + # 1 2048 4095 1M BIOS boot + # 2 4096 2101247 1G Microsoft basic + # 3 2101248 41940991 19G Linux LVM + # ---------------------------- end ----------------------------------- + # But mainly, in case of GPT, we have nothing to actually check as + # we are gathering this data now mainly to get information about the + # actual size of embedding area (MBR gap). In case of GPT, there is + # bios boot / prep boot partition, which has always 1 MiB and fulfill + # our expectations. So skip in this case another processing and generation + # of the msg. Let's improve it in future if we find a reason for it. + return None + for line in table_iter: line = line.strip() if not line.startswith('Device'): diff --git a/repos/system_upgrade/el7toel8/actors/scangrubdevpartitionlayout/tests/test_scan_partition_layout.py b/repos/system_upgrade/el7toel8/actors/scangrubdevpartitionlayout/tests/test_scan_partition_layout.py index 37bb5bcf..54025379 100644 --- a/repos/system_upgrade/el7toel8/actors/scangrubdevpartitionlayout/tests/test_scan_partition_layout.py +++ b/repos/system_upgrade/el7toel8/actors/scangrubdevpartitionlayout/tests/test_scan_partition_layout.py @@ -76,3 +76,8 @@ def test_get_partition_layout(monkeypatch, devices): expected_part_name_to_start = {part.name: part.start_offset*dev.sector_size for part in dev.partitions} actual_part_name_to_start = {part.part_device: part.start_offset for part in message.partitions} assert expected_part_name_to_start == actual_part_name_to_start + + +def test_get_partition_layout_gpt(monkeypatch): + # TODO(pstodulk): skipping for now, due to time pressure. Testing for now manually. + pass -- 2.42.0