From 2e2c9c43d5bbb156ef2b5c16503b2c65dcc6ff86 Mon Sep 17 00:00:00 2001 From: caleb Date: Thu, 27 Jun 2024 11:54:51 -0400 Subject: [PATCH 1/3] code hygiene (cherry picked from commit d0675a3989d3a671412aefbe0159a0d88412bce3) --- .../middlewared/plugins/disk_/disk_info.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/middlewared/middlewared/plugins/disk_/disk_info.py b/src/middlewared/middlewared/plugins/disk_/disk_info.py index e92e578f372b7..ee8e2fd042307 100644 --- a/src/middlewared/middlewared/plugins/disk_/disk_info.py +++ b/src/middlewared/middlewared/plugins/disk_/disk_info.py @@ -25,21 +25,15 @@ def get_dev_size(self, device): def list_partitions(self, disk): parts = [] try: - block_device = pyudev.Devices.from_name(pyudev.Context(), 'block', disk) + bd = pyudev.Devices.from_name(pyudev.Context(), 'block', disk) except pyudev.DeviceNotFoundByNameError: return parts - if not block_device.children: + if not bd.children: return parts - for p in filter( - lambda p: all( - p.get(k) for k in ( - 'ID_PART_ENTRY_TYPE', 'ID_PART_ENTRY_UUID', 'ID_PART_ENTRY_NUMBER', 'ID_PART_ENTRY_SIZE' - ) - ), - block_device.children - ): + req_keys = ('ID_PART_' + i for i in ('TYPE', 'UUID', 'NUMBER', 'SIZE')) + for p in filter(lambda p: all(p.get(k) for k in req_keys), bd.children): part_name = self.get_partition_for_disk(disk, p['ID_PART_ENTRY_NUMBER']) start_sector = int(p['ID_PART_ENTRY_OFFSET']) end_sector = int(p['ID_PART_ENTRY_OFFSET']) + int(p['ID_PART_ENTRY_SIZE']) - 1 From 61d02bd4b67d6da79864eff596e99e919532a8e3 Mon Sep 17 00:00:00 2001 From: caleb Date: Fri, 28 Jun 2024 07:49:14 -0400 Subject: [PATCH 2/3] fix start/end_sector keys in partition methods (cherry picked from commit 6a48fc96fa17d65a784dc1d943bce0649a7400ce) --- .../plugins/device_/device_info.py | 14 ++++--- .../middlewared/plugins/disk_/disk_info.py | 39 ++++++++++++++++--- 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/src/middlewared/middlewared/plugins/device_/device_info.py b/src/middlewared/middlewared/plugins/device_/device_info.py index cdd46ac841093..f6b9d9e22f795 100644 --- a/src/middlewared/middlewared/plugins/device_/device_info.py +++ b/src/middlewared/middlewared/plugins/device_/device_info.py @@ -5,6 +5,7 @@ import libsgio from middlewared.plugins.disk_.enums import DISKS_TO_IGNORE +from middlewared.plugins.disk_.disk_info import get_partition_size_info from middlewared.schema import Dict, returns from middlewared.service import Service, accepts, private from middlewared.utils.functools import cache @@ -100,6 +101,9 @@ def get_disk_partitions(self, dev): for i in filter(lambda x: all(x.get(k) for k in keys), dev.children): part_num = int(i['ID_PART_ENTRY_NUMBER']) part_name = self.middleware.call_sync('disk.get_partition_for_disk', parent, part_num) + lbs, start_sector, end_sector, sect_size = get_partition_size_info( + parent, int(i['ID_PART_ENTRY_OFFSET']), int(i['ID_PART_ENTRY_SIZE']) + ) part = { 'name': part_name, 'id': part_name, @@ -109,13 +113,13 @@ def get_disk_partitions(self, dev): 'partition_type': i['ID_PART_ENTRY_TYPE'], 'partition_number': part_num, 'partition_uuid': i['ID_PART_ENTRY_UUID'], - 'start_sector': int(i['ID_PART_ENTRY_OFFSET']), - 'end_sector': int(i['ID_PART_ENTRY_OFFSET']) + int(i['ID_PART_ENTRY_SIZE']) - 1, + 'start_sector': start_sector, + 'end_sector': end_sector, 'encrypted_provider': None, } - part['start'] = part['start_sector'] * 512 - part['end'] = part['end_sector'] * 512 - part['size'] = int(i['ID_PART_ENTRY_SIZE']) * 512 + part['start'] = start_sector * lbs # bytes + part['end'] = end_sector * lbs # bytes + part['size'] = sect_size * lbs # bytes for attr in filter(lambda x: x.startswith('holders/md'), i.attributes.available_attributes): # looks like `holders/md123` diff --git a/src/middlewared/middlewared/plugins/disk_/disk_info.py b/src/middlewared/middlewared/plugins/disk_/disk_info.py index ee8e2fd042307..097f2697ad281 100644 --- a/src/middlewared/middlewared/plugins/disk_/disk_info.py +++ b/src/middlewared/middlewared/plugins/disk_/disk_info.py @@ -1,3 +1,4 @@ +import contextlib import glob import os import pathlib @@ -7,6 +8,31 @@ from middlewared.service import CallError, private, Service +def get_partition_size_info(disk_name, s_offset, s_size): + """Kernel sysfs reports most disk files related to "size" in 512 bytes. + To properly calculate the starting SECTOR of partitions, you must + look at logical_block_size (again, reported in 512 bytes) and + do some calculations. It is _very_ important to do this properly + since almost all userspace tools that format disks expect partition + positions to be in sectors.""" + lbs = start_sect = end_sect = sect_size = 0 + with contextlib.suppress(FileNotFoundError, ValueError): + with open(f'/sys/block/{disk_name}/queue/logical_block_size') as f: + lbs = int(f.read().strip()) + + if not lbs: + # this should never happen + return lbs, start_sect, end_sect, sect_size + + # important when dealing with 4kn drives + divisor = lbs // 512 + start_sect = s_offset // divisor + sect_size = s_size // divisor + end_sect = sect_size + start_sect - 1 + + return lbs, start_sect, end_sect, sect_size + + class DiskService(Service): @private @@ -32,11 +58,12 @@ def list_partitions(self, disk): if not bd.children: return parts - req_keys = ('ID_PART_' + i for i in ('TYPE', 'UUID', 'NUMBER', 'SIZE')) + req_keys = ('ID_PART_ENTRY_' + i for i in ('TYPE', 'UUID', 'NUMBER', 'SIZE')) for p in filter(lambda p: all(p.get(k) for k in req_keys), bd.children): part_name = self.get_partition_for_disk(disk, p['ID_PART_ENTRY_NUMBER']) - start_sector = int(p['ID_PART_ENTRY_OFFSET']) - end_sector = int(p['ID_PART_ENTRY_OFFSET']) + int(p['ID_PART_ENTRY_SIZE']) - 1 + lbs, start_sector, end_sector, sect_size = get_partition_size_info( + disk, int(p['ID_PART_ENTRY_OFFSET']), int(p['ID_PART_ENTRY_SIZE']) + ) part = { 'name': part_name, 'partition_type': p['ID_PART_ENTRY_TYPE'], @@ -44,10 +71,10 @@ def list_partitions(self, disk): 'partition_uuid': p['ID_PART_ENTRY_UUID'], 'disk': disk, 'start_sector': start_sector, - 'start': start_sector * 512, + 'start': start_sector * lbs, # bytes 'end_sector': end_sector, - 'end': end_sector * 512, - 'size': int(p['ID_PART_ENTRY_SIZE']) * 512, + 'end': end_sector * lbs, # bytes + 'size': sect_size * lbs, # bytes 'id': part_name, 'path': os.path.join('/dev', part_name), 'encrypted_provider': None, From 61e21f4a5146c130e0123fca12d141699623e7ae Mon Sep 17 00:00:00 2001 From: caleb Date: Fri, 28 Jun 2024 08:49:33 -0400 Subject: [PATCH 3/3] used namedtuple (cherry picked from commit d7d9d889384106ab6e9bc2d4b5dea5fe53023c50) --- .../plugins/device_/device_info.py | 14 ++-- .../middlewared/plugins/disk_/disk_info.py | 67 ++++++++++++++----- 2 files changed, 56 insertions(+), 25 deletions(-) diff --git a/src/middlewared/middlewared/plugins/device_/device_info.py b/src/middlewared/middlewared/plugins/device_/device_info.py index f6b9d9e22f795..952cb1de7c10d 100644 --- a/src/middlewared/middlewared/plugins/device_/device_info.py +++ b/src/middlewared/middlewared/plugins/device_/device_info.py @@ -101,9 +101,7 @@ def get_disk_partitions(self, dev): for i in filter(lambda x: all(x.get(k) for k in keys), dev.children): part_num = int(i['ID_PART_ENTRY_NUMBER']) part_name = self.middleware.call_sync('disk.get_partition_for_disk', parent, part_num) - lbs, start_sector, end_sector, sect_size = get_partition_size_info( - parent, int(i['ID_PART_ENTRY_OFFSET']), int(i['ID_PART_ENTRY_SIZE']) - ) + pinfo = get_partition_size_info(parent, int(i['ID_PART_ENTRY_OFFSET']), int(i['ID_PART_ENTRY_SIZE'])) part = { 'name': part_name, 'id': part_name, @@ -113,13 +111,13 @@ def get_disk_partitions(self, dev): 'partition_type': i['ID_PART_ENTRY_TYPE'], 'partition_number': part_num, 'partition_uuid': i['ID_PART_ENTRY_UUID'], - 'start_sector': start_sector, - 'end_sector': end_sector, + 'start_sector': pinfo.start_sector, + 'end_sector': pinfo.end_sector, + 'start': pinfo.start_byte, + 'end': pinfo.end_byte, + 'size': pinfo.total_bytes, 'encrypted_provider': None, } - part['start'] = start_sector * lbs # bytes - part['end'] = end_sector * lbs # bytes - part['size'] = sect_size * lbs # bytes for attr in filter(lambda x: x.startswith('holders/md'), i.attributes.available_attributes): # looks like `holders/md123` diff --git a/src/middlewared/middlewared/plugins/disk_/disk_info.py b/src/middlewared/middlewared/plugins/disk_/disk_info.py index 097f2697ad281..ec2acd8005cb1 100644 --- a/src/middlewared/middlewared/plugins/disk_/disk_info.py +++ b/src/middlewared/middlewared/plugins/disk_/disk_info.py @@ -1,3 +1,4 @@ +import collections import contextlib import glob import os @@ -8,6 +9,32 @@ from middlewared.service import CallError, private, Service +# The basic unit of a block I/O is a sector. A sector is +# 512 (2 ** 9) bytes. In sysfs, the files (sector_t type) +# `//start` and `//size` are +# shown as a multiple of 512 bytes. Most user-space +# tools (fdisk, parted, sfdisk, etc) treat the partition +# offsets in sectors. +BYTES_512 = 512 +PART_INFO_FIELDS = ( + # queue/logical_block_size (reported as a multiple of BYTES_512) + 'lbs', + # starting offset of partition in sectors + 'start_sector', + # ending offset of partition in sectors + 'end_sector', + # total partition size in sectors + 'total_sectors', + # starting offset of partition in bytes + 'start_byte', + # ending offset of partition in bytes + 'end_byte', + # total size of partition in bytes + 'total_bytes', +) +PART_INFO = collections.namedtuple('part_info', PART_INFO_FIELDS, defaults=(0,) * len(PART_INFO_FIELDS)) + + def get_partition_size_info(disk_name, s_offset, s_size): """Kernel sysfs reports most disk files related to "size" in 512 bytes. To properly calculate the starting SECTOR of partitions, you must @@ -15,22 +42,30 @@ def get_partition_size_info(disk_name, s_offset, s_size): do some calculations. It is _very_ important to do this properly since almost all userspace tools that format disks expect partition positions to be in sectors.""" - lbs = start_sect = end_sect = sect_size = 0 + lbs = 0 with contextlib.suppress(FileNotFoundError, ValueError): with open(f'/sys/block/{disk_name}/queue/logical_block_size') as f: lbs = int(f.read().strip()) if not lbs: # this should never happen - return lbs, start_sect, end_sect, sect_size + return PART_INFO() # important when dealing with 4kn drives - divisor = lbs // 512 - start_sect = s_offset // divisor - sect_size = s_size // divisor - end_sect = sect_size + start_sect - 1 - - return lbs, start_sect, end_sect, sect_size + divisor = lbs // BYTES_512 + # sectors + start_sector = s_offset // divisor + total_sectors = s_size // divisor + end_sector = total_sectors + start_sector - 1 + # bytes + start_byte = start_sector * lbs + end_byte = end_sector * lbs + total_bytes = total_sectors * lbs + + return PART_INFO(*( + lbs, start_sector, end_sector, total_sectors, + start_byte, end_byte, total_bytes, + )) class DiskService(Service): @@ -45,7 +80,7 @@ def get_dev_size(self, device): if dev.get('DEVTYPE') not in ('disk', 'partition'): return - return dev.attributes.asint('size') * 512 + return dev.attributes.asint('size') * BYTES_512 @private def list_partitions(self, disk): @@ -61,20 +96,18 @@ def list_partitions(self, disk): req_keys = ('ID_PART_ENTRY_' + i for i in ('TYPE', 'UUID', 'NUMBER', 'SIZE')) for p in filter(lambda p: all(p.get(k) for k in req_keys), bd.children): part_name = self.get_partition_for_disk(disk, p['ID_PART_ENTRY_NUMBER']) - lbs, start_sector, end_sector, sect_size = get_partition_size_info( - disk, int(p['ID_PART_ENTRY_OFFSET']), int(p['ID_PART_ENTRY_SIZE']) - ) + pinfo = get_partition_size_info(disk, int(p['ID_PART_ENTRY_OFFSET']), int(p['ID_PART_ENTRY_SIZE'])) part = { 'name': part_name, 'partition_type': p['ID_PART_ENTRY_TYPE'], 'partition_number': int(p['ID_PART_ENTRY_NUMBER']), 'partition_uuid': p['ID_PART_ENTRY_UUID'], 'disk': disk, - 'start_sector': start_sector, - 'start': start_sector * lbs, # bytes - 'end_sector': end_sector, - 'end': end_sector * lbs, # bytes - 'size': sect_size * lbs, # bytes + 'start_sector': pinfo.start_sector, + 'start': pinfo.start_byte, + 'end_sector': pinfo.end_sector, + 'end': pinfo.end_byte, + 'size': pinfo.total_bytes, 'id': part_name, 'path': os.path.join('/dev', part_name), 'encrypted_provider': None,