-
Notifications
You must be signed in to change notification settings - Fork 148
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
boot: check first partition offset on GRUB devices
Check that the first partition starts at least at 1MiB (2048 cylinders), as too small first-partition offsets lead to failures when doing grub2-install. The limit (1MiB) has been chosen as it is a common value set by the disk formatting tools nowadays. jira: https://issues.redhat.com/browse/RHEL-3341
- Loading branch information
Showing
7 changed files
with
315 additions
and
0 deletions.
There are no files selected for viewing
24 changes: 24 additions & 0 deletions
24
repos/system_upgrade/el7toel8/actors/checkfirstpartitionoffset/actor.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
from leapp.actors import Actor | ||
from leapp.libraries.actor import check_first_partition_offset | ||
from leapp.models import FirmwareFacts, GRUBDevicePartitionLayout | ||
from leapp.reporting import Report | ||
from leapp.tags import ChecksPhaseTag, IPUWorkflowTag | ||
|
||
|
||
class CheckFirstPartitionOffset(Actor): | ||
""" | ||
Check whether the first partition starts at the offset >=1MiB. | ||
The alignment of the first partition plays role in disk access speeds. Older tools placed the start of the first | ||
partition at cylinder 63 (due to historical reasons connected to the INT13h BIOS API). However, grub core | ||
binary is placed before the start of the first partition, meaning that not enough space causes bootloader | ||
installation to fail. Modern partitioning tools place the first partition at >= 1MiB (cylinder 2048+). | ||
""" | ||
|
||
name = 'check_first_partition_offset' | ||
consumes = (FirmwareFacts, GRUBDevicePartitionLayout,) | ||
produces = (Report,) | ||
tags = (ChecksPhaseTag, IPUWorkflowTag,) | ||
|
||
def process(self): | ||
check_first_partition_offset.check_first_partition_offset() |
52 changes: 52 additions & 0 deletions
52
...grade/el7toel8/actors/checkfirstpartitionoffset/libraries/check_first_partition_offset.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
from leapp import reporting | ||
from leapp.libraries.common.config import architecture | ||
from leapp.libraries.stdlib import api | ||
from leapp.models import FirmwareFacts, GRUBDevicePartitionLayout | ||
|
||
SAFE_OFFSET_BYTES = 1024*1024 # 1MiB | ||
|
||
|
||
def check_first_partition_offset(): | ||
if architecture.matches_architecture(architecture.ARCH_S390X): | ||
return | ||
|
||
for fact in api.consume(FirmwareFacts): | ||
if fact.firmware == 'efi': | ||
return # Skip EFI system | ||
|
||
problematic_devices = [] | ||
for grub_dev in api.consume(GRUBDevicePartitionLayout): | ||
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) | ||
|
||
if problematic_devices: | ||
summary = ( | ||
'On the system booting by using BIOS, the in-place upgrade fails ' | ||
'when upgrading the GRUB2 bootloader if the boot disk\'s embedding area ' | ||
'does not contain enough space for the core image installation. ' | ||
'This results in a broken system, and can occur when the disk has been ' | ||
'partitioned manually, for example using the RHEL 6 fdisk utility.\n\n' | ||
|
||
'The list of devices with small embedding area:\n' | ||
'{0}.' | ||
) | ||
problematic_devices_fmt = ['- {0}'.format(dev) for dev in problematic_devices] | ||
|
||
hint = ( | ||
'We recommend to perform a fresh installation of the RHEL 8 system ' | ||
'instead of performing the in-place upgrade.\n' | ||
'Another possibility is to reformat the devices so that there is ' | ||
'at least {0} kiB space before the first partition. ' | ||
'Note that this operation is not supported and does not have to be ' | ||
'always possible.' | ||
) | ||
|
||
reporting.create_report([ | ||
reporting.Title('Found GRUB devices with too little space reserved before the first partition'), | ||
reporting.Summary(summary.format('\n'.join(problematic_devices_fmt))), | ||
reporting.Remediation(hint=hint.format(SAFE_OFFSET_BYTES // 1024)), | ||
reporting.Severity(reporting.Severity.HIGH), | ||
reporting.Groups([reporting.Groups.BOOT]), | ||
reporting.Groups([reporting.Groups.INHIBITOR]), | ||
]) |
51 changes: 51 additions & 0 deletions
51
...rade/el7toel8/actors/checkfirstpartitionoffset/tests/test_check_first_partition_offset.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
import pytest | ||
|
||
from leapp import reporting | ||
from leapp.libraries.actor import check_first_partition_offset | ||
from leapp.libraries.common import grub | ||
from leapp.libraries.common.testutils import create_report_mocked, CurrentActorMocked | ||
from leapp.libraries.stdlib import api | ||
from leapp.models import FirmwareFacts, GRUBDevicePartitionLayout, PartitionInfo | ||
from leapp.reporting import Report | ||
from leapp.utils.report import is_inhibitor | ||
|
||
|
||
@pytest.mark.parametrize( | ||
('devices', 'should_report'), | ||
[ | ||
( | ||
[ | ||
GRUBDevicePartitionLayout(device='/dev/vda', | ||
partitions=[PartitionInfo(part_device='/dev/vda1', start_offset=32256)]) | ||
], | ||
True | ||
), | ||
( | ||
[ | ||
GRUBDevicePartitionLayout(device='/dev/vda', | ||
partitions=[PartitionInfo(part_device='/dev/vda1', start_offset=1024*1025)]) | ||
], | ||
False | ||
), | ||
( | ||
[ | ||
GRUBDevicePartitionLayout(device='/dev/vda', | ||
partitions=[PartitionInfo(part_device='/dev/vda1', start_offset=1024*1024)]) | ||
], | ||
False | ||
) | ||
] | ||
) | ||
def test_bad_offset_reported(monkeypatch, devices, should_report): | ||
def consume_mocked(model_cls): | ||
if model_cls == FirmwareFacts: | ||
return [FirmwareFacts(firmware='bios')] | ||
return devices | ||
|
||
monkeypatch.setattr(api, 'consume', consume_mocked) | ||
monkeypatch.setattr(api, 'current_actor', CurrentActorMocked()) | ||
monkeypatch.setattr(reporting, 'create_report', create_report_mocked()) | ||
|
||
check_first_partition_offset.check_first_partition_offset() | ||
|
||
assert bool(reporting.create_report.called) == should_report |
18 changes: 18 additions & 0 deletions
18
repos/system_upgrade/el7toel8/actors/scangrubdevpartitionlayout/actor.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
from leapp.actors import Actor | ||
from leapp.libraries.actor import scan_layout as scan_layout_lib | ||
from leapp.models import GRUBDevicePartitionLayout, GrubInfo | ||
from leapp.tags import FactsPhaseTag, IPUWorkflowTag | ||
|
||
|
||
class ScanGRUBDevicePartitionLayout(Actor): | ||
""" | ||
Scan all identified GRUB devices for their partition layout. | ||
""" | ||
|
||
name = 'scan_grub_device_partition_layout' | ||
consumes = (GrubInfo,) | ||
produces = (GRUBDevicePartitionLayout,) | ||
tags = (FactsPhaseTag, IPUWorkflowTag,) | ||
|
||
def process(self): | ||
scan_layout_lib.scan_grub_device_partition_layout() |
64 changes: 64 additions & 0 deletions
64
repos/system_upgrade/el7toel8/actors/scangrubdevpartitionlayout/libraries/scan_layout.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
from leapp.libraries.stdlib import api, CalledProcessError, run | ||
from leapp.models import GRUBDevicePartitionLayout, GrubInfo, PartitionInfo | ||
|
||
SAFE_OFFSET_BYTES = 1024*1024 # 1MiB | ||
|
||
|
||
def split_on_space_segments(line): | ||
fragments = (fragment.strip() for fragment in line.split(' ')) | ||
return [fragment for fragment in fragments if fragment] | ||
|
||
|
||
def get_partition_layout(device): | ||
try: | ||
partition_table = run(['fdisk', '-l', '-u=sectors', device], split=True)['stdout'] | ||
except CalledProcessError as err: | ||
# Unlikely - if the disk has no partition table, `fdisk` terminates with 0 (no err). Fdisk exits with an err | ||
# when the device does not exists, or if it is too small to contain a partition table. | ||
|
||
err_msg = 'Failed to run `fdisk` to obtain the partition table of the device {0}. Full error: \'{1}\'' | ||
api.current_logger().error(err_msg.format(device, str(err))) | ||
return None | ||
|
||
table_iter = iter(partition_table) | ||
|
||
for line in table_iter: | ||
if not line.startswith('Units'): | ||
# We are still reading general device information and not the table itself | ||
continue | ||
|
||
unit = line.split('=')[2].strip() # Contains '512 bytes' | ||
unit = int(unit.split(' ')[0].strip()) | ||
break # First line of the partition table header | ||
|
||
for line in table_iter: | ||
line = line.strip() | ||
if not line.startswith('Device'): | ||
continue | ||
|
||
part_all_attrs = split_on_space_segments(line) | ||
break | ||
|
||
partitions = [] | ||
for partition_line in table_iter: | ||
# Fields: Device Boot Start End Sectors Size Id Type | ||
# The line looks like: `/dev/vda1 * 2048 2099199 2097152 1G 83 Linux` | ||
part_info = split_on_space_segments(partition_line) | ||
|
||
# If the partition is not bootable, the Boot column might be empty | ||
part_device = part_info[0] | ||
part_start = int(part_info[2]) if len(part_info) == len(part_all_attrs) else int(part_info[1]) | ||
partitions.append(PartitionInfo(part_device=part_device, start_offset=part_start*unit)) | ||
|
||
return GRUBDevicePartitionLayout(device=device, partitions=partitions) | ||
|
||
|
||
def scan_grub_device_partition_layout(): | ||
grub_devices = next(api.consume(GrubInfo), None) | ||
if not grub_devices: | ||
return | ||
|
||
for device in grub_devices.orig_devices: | ||
dev_info = get_partition_layout(device) | ||
if dev_info: | ||
api.produce(dev_info) |
78 changes: 78 additions & 0 deletions
78
...em_upgrade/el7toel8/actors/scangrubdevpartitionlayout/tests/test_scan_partition_layout.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
from collections import namedtuple | ||
|
||
import pytest | ||
|
||
from leapp.libraries.actor import scan_layout as scan_layout_lib | ||
from leapp.libraries.common import grub | ||
from leapp.libraries.common.testutils import create_report_mocked, produce_mocked | ||
from leapp.libraries.stdlib import api | ||
from leapp.models import GRUBDevicePartitionLayout, GrubInfo | ||
from leapp.utils.report import is_inhibitor | ||
|
||
Device = namedtuple('Device', ['name', 'partitions', 'sector_size']) | ||
Partition = namedtuple('Partition', ['name', 'start_offset']) | ||
|
||
|
||
@pytest.mark.parametrize( | ||
'devices', | ||
[ | ||
( | ||
Device(name='/dev/vda', sector_size=512, | ||
partitions=[Partition(name='/dev/vda1', start_offset=63), | ||
Partition(name='/dev/vda2', start_offset=1000)]), | ||
Device(name='/dev/vdb', sector_size=1024, | ||
partitions=[Partition(name='/dev/vdb1', start_offset=100), | ||
Partition(name='/dev/vdb2', start_offset=20000)]) | ||
), | ||
( | ||
Device(name='/dev/vda', sector_size=512, | ||
partitions=[Partition(name='/dev/vda1', start_offset=111), | ||
Partition(name='/dev/vda2', start_offset=1000)]), | ||
) | ||
] | ||
) | ||
def test_get_partition_layout(monkeypatch, devices): | ||
device_to_fdisk_output = {} | ||
for device in devices: | ||
fdisk_output = [ | ||
'Disk {0}: 42.9 GB, 42949672960 bytes, 83886080 sectors'.format(device.name), | ||
'Units = sectors of 1 * {sector_size} = {sector_size} bytes'.format(sector_size=device.sector_size), | ||
'Sector size (logical/physical): 512 bytes / 512 bytes', | ||
'I/O size (minimum/optimal): 512 bytes / 512 bytes', | ||
'Disk label type: dos', | ||
'Disk identifier: 0x0000000da', | ||
'', | ||
' Device Boot Start End Blocks Id System', | ||
] | ||
for part in device.partitions: | ||
part_line = '{0} * {1} 2099199 1048576 83 Linux'.format(part.name, part.start_offset) | ||
fdisk_output.append(part_line) | ||
|
||
device_to_fdisk_output[device.name] = fdisk_output | ||
|
||
def mocked_run(cmd, *args, **kwargs): | ||
assert cmd[:3] == ['fdisk', '-l', '-u=sectors'] | ||
device = cmd[3] | ||
output = device_to_fdisk_output[device] | ||
return {'stdout': output} | ||
|
||
def consume_mocked(*args, **kwargs): | ||
yield GrubInfo(orig_devices=[device.name for device in devices]) | ||
|
||
monkeypatch.setattr(scan_layout_lib, 'run', mocked_run) | ||
monkeypatch.setattr(api, 'produce', produce_mocked()) | ||
monkeypatch.setattr(api, 'consume', consume_mocked) | ||
|
||
scan_layout_lib.scan_grub_device_partition_layout() | ||
|
||
assert api.produce.called == len(devices) | ||
|
||
dev_name_to_desc = {dev.name: dev for dev in devices} | ||
|
||
for message in api.produce.model_instances: | ||
assert isinstance(message, GRUBDevicePartitionLayout) | ||
dev = dev_name_to_desc[message.device] | ||
|
||
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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
from leapp.models import fields, Model | ||
from leapp.topics import SystemInfoTopic | ||
|
||
|
||
class PartitionInfo(Model): | ||
""" | ||
Information about a single partition. | ||
""" | ||
topic = SystemInfoTopic | ||
|
||
part_device = fields.String() | ||
""" Partition device """ | ||
|
||
start_offset = fields.Integer() | ||
""" Partition start - offset from the start of the block device in bytes """ | ||
|
||
|
||
class GRUBDevicePartitionLayout(Model): | ||
""" | ||
Information about partition layout of a GRUB device. | ||
""" | ||
topic = SystemInfoTopic | ||
|
||
device = fields.String() | ||
""" GRUB device """ | ||
|
||
partitions = fields.List(fields.Model(PartitionInfo)) | ||
""" List of partitions present on the device """ |