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

NAS-128891 / 24.10 / Remove the logic for creating swap partitions #13695

Merged
merged 9 commits into from
May 14, 2024
Merged

NAS-128891 / 24.10 / Remove the logic for creating swap partitions #13695

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
cdd0a4d
remove swap
yocalebo May 9, 2024
bbc9b95
remove swap logic from disk.format
yocalebo May 9, 2024
8e5e446
simplify disk.format
yocalebo May 10, 2024
af2f55b
fix test_disk_format.py
yocalebo May 11, 2024
06dab03
fix tests and a regression found after API test run
yocalebo May 11, 2024
b43b4b5
fix test_disk_format.py
yocalebo May 13, 2024
11dabd4
some touch-ups
yocalebo May 14, 2024
23fcbf1
address reviews
yocalebo May 14, 2024
a44ba45
remove swap arg in test_disk_wipe.py as well
yocalebo May 14, 2024
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
18 changes: 0 additions & 18 deletions debian/debian/ix-swap.service
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion debian/debian/rules
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ override_dh_installsystemd:
dh_installsystemd --no-start -r --no-restart-after-upgrade --name=ix-shutdown
dh_installsystemd --no-start -r --no-restart-after-upgrade --name=ix-ssh-keys
dh_installsystemd --no-start -r --no-restart-after-upgrade --name=ix-syncdisks
dh_installsystemd --no-start -r --no-restart-after-upgrade --name=ix-swap
dh_installsystemd --no-start -r --no-restart-after-upgrade --name=ix-wait-on-disks
dh_installsystemd --no-start -r --no-restart-after-upgrade --name=ix-zfs
dh_installsystemd --no-start -r --no-restart-after-upgrade --name=snmp-agent
Expand Down
3 changes: 0 additions & 3 deletions src/middlewared/middlewared/plugins/disk_/disk_events.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@ async def added_disk(middleware, disk_name):
async def remove_disk(middleware, disk_name):
await (await middleware.call('disk.sync_all')).wait()
await middleware.call('alert.oneshot_delete', 'SMART', disk_name)
# If a disk dies we need to reconfigure swaps so we are not left
# with a single disk mirror swap, which may be a point of failure.
middleware.create_task(middleware.call('disk.swaps_configure'))


async def udev_block_devices_hook(middleware, data):
Expand Down
210 changes: 36 additions & 174 deletions src/middlewared/middlewared/plugins/disk_/format.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import pathlib

import parted

from middlewared.service import CallError, private, Service
Expand All @@ -6,185 +8,45 @@
class DiskService(Service):

@private
def format(self, disk, swap_size_gb):
"""
Format a data drive with a maximized data partition
Rules:
- The min_data_size is 512MiB
i.e. the drive must be bigger than 512MiB + 2MiB (partition offsets)
NOTE: 512MiB is arbitrary, but allows for very small drives
- If swap_size_gb is not None, then
* The swap is sized in 1 GiB increments
* Drive partitioning will abort if requested swap cannot be accomodated
* A swap partition will be created only if the following is true:
swap_size < drive_size - (data_size + partition_gaps)
* The data partition will be reduced by swap_size_gb
- The drive is left unchanged if the drive cannot be partitioned according to the rules

The current config default requested swap is 2 GiB
A typical drive partition diagram (assuming 1 MiB partition gaps):

| - unused - | - partition 1 - | - unused -| - partition 2 - | - unused - |
|------------|-----------------|-----------|-----------------|------------|
| 1 MiB gap | 2 GiB swap | 1 MiB gap | N GiB data | 1 MiB gap |

"""
if swap_size_gb is not None and (swap_size_gb < 0 or not isinstance(swap_size_gb, int)):
raise CallError('Requested swap must be a non-negative integer')

dd = self.middleware.call_sync('device.get_disk', disk)
if not dd:
def format(self, disk, swap_size_gb=None):
"""Format a data drive with a maximized data partition"""
sysfs = pathlib.Path(f'/sys/class/block/{disk}')
if not sysfs.exists():
raise CallError(f'Unable to retrieve disk details for {disk!r}')

if dd['dif']:
is_dif = next(sysfs.glob('device/scsi_disk/*/protection_type'), None)
if is_dif is not None and is_dif.read_text().strip() != '0':
# 0 == disabled, > 0 enabled
raise CallError(f'Disk: {disk!r} is incorrectly formatted with Data Integrity Feature (DIF).')

# Get drive specs and size in sectors
device = parted.getDevice(f'/dev/{disk}')

# We rely on a valid 'grainSize', let's make sure before we proceed.
if device.optimumAlignment.grainSize <= 0:
raise CallError(f'Unable to format {disk!r}: grainSize = {device.optimumAlignment.grainSize}')

drive_size_s = parted.sizeToSectors(dd['size'], 'B', device.sectorSize)

# Allocate space for the requested swap size
leave_free_space = 0 if swap_size_gb is None else parted.sizeToSectors(swap_size_gb, 'GiB', device.sectorSize)

swap_gap = device.optimumAlignment.grainSize if leave_free_space > 0 else 0
partition_gaps = 2 * device.optimumAlignment.grainSize + swap_gap

# Minimum data partition size of 512 MiB is arbitrary
min_data_size = parted.sizeToSectors(512, 'MiB', device.sectorSize)
# Here we leave max_data_size possibly oversized to allow for parted
# to create the maximal sized partition
max_data_size = drive_size_s - leave_free_space

# For validation we should also account for the gaps
if (max_data_size - partition_gaps) <= 0:
emsg = f'Disk {disk!r} capacity is too small. Please use a larger capacity drive' + (
' or reduce swap.' if leave_free_space > 0 else '.'
)
raise CallError(emsg)

# At this point, the drive has passed validation. Proceed with drive clean and partitioning
job = self.middleware.call_sync('disk.wipe', disk, 'QUICK', False)
job.wait_sync()
if job.error:
raise CallError(f'Failed to wipe disk {disk}: {job.error}')

device.clobber()
parted_disk = parted.freshDisk(device, 'gpt')

# Sanity: make sure max is larger than min
if max_data_size <= min_data_size:
max_data_size = min_data_size + device.optimumAlignment.grainSize

data_geometry = self._get_largest_free_space_region(parted_disk)

# Place the data partition at the end of the disk. The swap is created at the beginning
start_range = parted.Geometry(
device,
# We need the partition gap _only if_ there is a swap partition
data_geometry.start + leave_free_space + (device.optimumAlignment.grainSize if leave_free_space > 0 else 0),
end=data_geometry.end,
)
data_constraint = parted.Constraint(
startAlign=device.optimumAlignment,
endAlign=device.optimumAlignment,
startRange=start_range,
endRange=data_geometry,
minSize=min_data_size,
maxSize=max_data_size,
)

def create_data_partition(constraint):
data_filesystem = parted.FileSystem(type='zfs', geometry=data_geometry)
data_partition = parted.Partition(
disk=parted_disk,
type=parted.PARTITION_NORMAL,
fs=data_filesystem,
geometry=data_geometry,
)
parted_disk.addPartition(data_partition, constraint=constraint)

try:
create_data_partition(data_constraint)
except parted.PartitionException as e:
emsg = f'Disk {disk!r} capacity might be too small. Try a larger capacity drive' + (
' or reduce swap.' if leave_free_space > 0 else '.'
)
raise CallError(f"{emsg}: {e}")

# If requested, add a swap partition
if swap_size_gb > 0:
min_swap_size = parted.sizeToSectors(1, 'GiB', device.sectorSize)
# Select the free space region that we've left previously
swap_geometries = [
geometry
for geometry in parted_disk.getFreeSpaceRegions()
if geometry.length >= min_swap_size
]
if swap_geometries:
swap_geometry = swap_geometries[0]
swap_constraint = parted.Constraint(
startAlign=device.optimumAlignment,
endAlign=device.optimumAlignment,
startRange=swap_geometry,
endRange=swap_geometry,
minSize=min_swap_size,
maxSize=(
parted.sizeToSectors(swap_size_gb, 'GiB', device.sectorSize) + device.optimumAlignment.grainSize
),
)
swap_filesystem = parted.FileSystem(type='linux-swap(v1)', geometry=swap_geometry)
swap_partition = parted.Partition(
disk=parted_disk,
type=parted.PARTITION_NORMAL,
fs=swap_filesystem,
geometry=swap_geometry,
)
try:
parted_disk.addPartition(swap_partition, constraint=swap_constraint)
except parted.PartitionException as e:
self.logger.warning('Unable to fit a swap partition on disk %r: %r', disk, e)

# Reorder the partitions so that they logical order matched their physical order
partitions = parted_disk.partitions[:]
# Unfortunately this can only be achieved by first removing all partitions (this happens in the RAM, no
# real disk changes are made yet)
for partition in partitions:
parted_disk.removePartition(partition)
# And then re-creating them in the correct order
partitions.sort(key=lambda partition: partition.geometry.start)
for partition in partitions:
partition.resetNumber()
constraint = parted.Constraint(exactGeom=partition.geometry)
geometry = self._get_largest_free_space_region(parted_disk)
new_partition = parted.Partition(
disk=parted_disk,
type=parted.PARTITION_NORMAL,
fs=parted.FileSystem(type=partition.fileSystem.type, geometry=geometry),
geometry=geometry,
)
# Add a human readable name
if partition.fileSystem.type == 'zfs':
new_partition.name = 'data'
elif 'swap' in partition.fileSystem.type:
new_partition.name = 'swap'

parted_disk.addPartition(partition=new_partition, constraint=constraint)

dev = parted.getDevice(f'/dev/{disk}')
for i in range(2):
if not dev.clobber():
# clobber() wipes partition label info from disk but during testing
# on an m40 HA system, the disk had to be clobber()'ed twice before
# fdisk -l wouldn't show any partitions. Only doing it once showed
# the following output
# Disk /dev/sda: 10.91 TiB, 12000138625024 bytes, 2929721344 sectors
# Disk model: HUH721212AL4200
# Units: sectors of 1 * 4096 = 4096 bytes
# Sector size (logical/physical): 4096 bytes / 4096 bytes
# I/O size (minimum/optimal): 4096 bytes / 4096 bytes
# Disklabel type: dos
# Disk identifier: 0x00000000
#
# Device Boot Start End Sectors Size Id Type
# /dev/sda1 1 2929721343 2929721343 10.9T ee GPT
raise CallError(f'Failed on attempt #{i} clearing partition labels for {disk!r}')

parted_disk = parted.freshDisk(dev, 'gpt')
regions = sorted(parted_disk.getFreeSpaceRegions(), key=lambda x: x.length)[-1]
geom = parted.Geometry(start=regions.start, end=regions.end, device=dev)
fs = parted.FileSystem(type='zfs', geometry=geom)
part = parted.Partition(disk=parted_disk, type=parted.PARTITION_NORMAL, fs=fs, geometry=geom)
part.name = 'data' # give a human readable name to the label
parted_disk.addPartition(part, constraint=dev.optimalAlignedConstraint)
parted_disk.commit()

# TODO: Install a dummy boot block so system gives meaningful message if booting from a zpool data disk.

self.middleware.call_sync('device.settle_udev_events')

if len(self.middleware.call_sync('disk.list_partitions', disk)) != len(parted_disk.partitions):
if len(self.middleware.call_sync('disk.get_partitions_quick', disk)) != len(parted_disk.partitions):
# In some rare cases udev does not re-read the partition table correctly; force it
self.middleware.call_sync('device.trigger_udev_events', f'/dev/{disk}')
self.middleware.call_sync('device.settle_udev_events')

def _get_largest_free_space_region(self, disk):
return sorted(disk.getFreeSpaceRegions(), key=lambda geometry: geometry.length)[-1]
Loading
Loading