Skip to content

Commit

Permalink
boot: Skip checks of first partition offset for for gpt partition table
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
pirat89 committed Apr 23, 2024
1 parent 732203e commit 084d357
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,17 @@
],
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',
Expand All @@ -33,6 +44,12 @@
partitions=[PartitionInfo(part_device='/dev/vda1', start_offset=1024*1024)])
],
False
),
(
[
GRUBDevicePartitionLayout(device='/dev/vda', partitions=[])
],
False
)
]
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,8 @@ def consume_mocked(*args, **kwargs):
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

0 comments on commit 084d357

Please sign in to comment.