From fa7f67a89841bae4c1ede5f06b17ec8727c55f47 Mon Sep 17 00:00:00 2001 From: themylogin Date: Thu, 5 Aug 2021 20:32:16 +0200 Subject: [PATCH 1/2] Improve smartd config generation performance --- src/middlewared/middlewared/common/smart/smartctl.py | 7 ++++--- src/middlewared/middlewared/etc_files/smartd.py | 7 ++++++- src/middlewared/middlewared/plugins/system.py | 4 ++++ 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/middlewared/middlewared/common/smart/smartctl.py b/src/middlewared/middlewared/common/smart/smartctl.py index 89f18a9a02ca..7d6266e8135e 100644 --- a/src/middlewared/middlewared/common/smart/smartctl.py +++ b/src/middlewared/middlewared/common/smart/smartctl.py @@ -85,9 +85,10 @@ async def get_smartctl_args(middleware, devices, disk): return [f"/dev/{driver}{controller_id}", "-d", f"3ware,{port}"] args = [f"/dev/{disk}"] - p = await smartctl(args + ["-i"], stderr=subprocess.STDOUT, check=False, encoding="utf8", errors="ignore") - if "Unknown USB bridge" in p.stdout: - args = args + ["-d", "sat"] + if disk.startswith("da") and not await middleware.call("system.is_enterprise_ix_hardware"): + p = await smartctl(args + ["-i"], stderr=subprocess.STDOUT, check=False, encoding="utf8", errors="ignore") + if "Unknown USB bridge" in p.stdout: + args = args + ["-d", "sat"] return args diff --git a/src/middlewared/middlewared/etc_files/smartd.py b/src/middlewared/middlewared/etc_files/smartd.py index f668fe29086b..f86432ec3342 100644 --- a/src/middlewared/middlewared/etc_files/smartd.py +++ b/src/middlewared/middlewared/etc_files/smartd.py @@ -12,7 +12,12 @@ async def annotate_disk_for_smart(middleware, devices, disk): args = await get_smartctl_args(middleware, devices, disk) if args: - if await ensure_smart_enabled(args): + if ( + # On Enterprise hardware we only use S.M.A.R.T.-enabled disks, there is no need to check for this + # every time. + await middleware.call('system.is_enterprise_ix_hardware') or + await ensure_smart_enabled(args) + ): args.extend(["-a"]) args.extend(["-d", "removable"]) return disk, dict(smartctl_args=args) diff --git a/src/middlewared/middlewared/plugins/system.py b/src/middlewared/middlewared/plugins/system.py index a2c4576c9e9c..488e3e27bcb9 100644 --- a/src/middlewared/middlewared/plugins/system.py +++ b/src/middlewared/middlewared/plugins/system.py @@ -726,6 +726,10 @@ async def is_ix_hardware(self): product = (await self.middleware.call('system.dmidecode_info'))['system-product-name'] return product is not None and product.startswith(('FREENAS-', 'TRUENAS-')) + @private + async def is_enterprise_ix_hardware(self): + return await self.middleware.call('truenas.get_chassis_hardware') != 'TRUENAS-UNKNOWN' + # Sync the clock @private def sync_clock(self): From e033a03099b5af3190097be1343583762be990f9 Mon Sep 17 00:00:00 2001 From: themylogin Date: Thu, 5 Aug 2021 20:34:32 +0200 Subject: [PATCH 2/2] Improve pool.transform_topology performance --- .../plugins/disk_/disk_info_base.py | 4 +- .../plugins/disk_/disk_info_freebsd.py | 36 ++++- .../plugins/disk_/disk_info_linux.py | 130 ------------------ src/middlewared/middlewared/plugins/pool.py | 4 +- 4 files changed, 34 insertions(+), 140 deletions(-) delete mode 100644 src/middlewared/middlewared/plugins/disk_/disk_info_linux.py diff --git a/src/middlewared/middlewared/plugins/disk_/disk_info_base.py b/src/middlewared/middlewared/plugins/disk_/disk_info_base.py index 08c6bedce03f..01832aab894a 100644 --- a/src/middlewared/middlewared/plugins/disk_/disk_info_base.py +++ b/src/middlewared/middlewared/plugins/disk_/disk_info_base.py @@ -33,11 +33,11 @@ async def get_swap_part_type(self): async def get_swap_devices(self): raise NotImplementedError() - async def label_to_dev(self, label, *args): + async def label_to_dev(self, label, geom_scan=True, cache=None): raise NotImplementedError() @private - async def label_to_disk(self, label, *args): + async def label_to_disk(self, label, geom_scan=True, cache=None): raise NotImplementedError() @private diff --git a/src/middlewared/middlewared/plugins/disk_/disk_info_freebsd.py b/src/middlewared/middlewared/plugins/disk_/disk_info_freebsd.py index 7e1f4cc9d083..2bc8b6757a2d 100644 --- a/src/middlewared/middlewared/plugins/disk_/disk_info_freebsd.py +++ b/src/middlewared/middlewared/plugins/disk_/disk_info_freebsd.py @@ -74,13 +74,33 @@ async def get_swap_part_type(self): def get_swap_devices(self): return [os.path.join('/dev', i.devname) for i in getswapinfo()] - def label_to_dev(self, label, *args): - geom_scan = args[0] if args else True + def label_to_dev_disk_cache(self): + label_to_dev = {} + for label in geom.class_by_name('LABEL').xml: + if (name := label.find('name')) is not None: + if (provider := label.find('provider/name')) is not None: + label_to_dev[provider.text] = name.text + + dev_to_disk = {} + for label in geom.class_by_name('PART').xml: + if (name := label.find('name')) is not None: + if (provider := label.find('provider/name')) is not None: + dev_to_disk[provider.text] = name.text + + return { + 'label_to_dev': label_to_dev, + 'dev_to_disk': dev_to_disk, + } + + def label_to_dev(self, label, geom_scan=True, cache=None): if label.endswith('.nop'): label = label[:-4] elif label.endswith('.eli'): label = label[:-4] + if cache is not None: + return cache['label_to_dev'].get(label) + if geom_scan: geom.scan() klass = geom.class_by_name('LABEL') @@ -88,11 +108,13 @@ def label_to_dev(self, label, *args): if prov is not None: return prov.text - def label_to_disk(self, label, *args): - geom_scan = args[0] if args else True - if geom_scan: - geom.scan() - dev = self.label_to_dev(label, geom_scan) or label + def label_to_disk(self, label, geom_scan=True, cache=None): + if cache is None: + if geom_scan: + geom.scan() + dev = self.label_to_dev(label, geom_scan, cache) or label + if cache is not None: + return cache['dev_to_disk'].get(dev) part = geom.class_by_name('PART').xml.find(f'.//provider[name="{dev}"]/../name') if part is not None: return part.text diff --git a/src/middlewared/middlewared/plugins/disk_/disk_info_linux.py b/src/middlewared/middlewared/plugins/disk_/disk_info_linux.py deleted file mode 100644 index 4f48be213cc1..000000000000 --- a/src/middlewared/middlewared/plugins/disk_/disk_info_linux.py +++ /dev/null @@ -1,130 +0,0 @@ -import glob -import os -import pyudev - -from middlewared.service import CallError, Service - -from .disk_info_base import DiskInfoBase - - -class DiskService(Service, DiskInfoBase): - - def get_dev_size(self, dev): - try: - block_device = pyudev.Devices.from_name(pyudev.Context(), 'block', dev) - except pyudev.DeviceNotFoundByNameError: - return - - if block_device.get('DEVTYPE') not in ('disk', 'partition'): - return - - logical_sector_size = self.middleware.call_sync( - 'device.logical_sector_size', - dev if block_device['DEVTYPE'] == 'disk' else block_device.find_parent('block').sys_name - ) - if not logical_sector_size: - return - - if block_device['DEVTYPE'] == 'disk': - path = os.path.join('/sys/block', dev, 'device/block', dev, 'size') - if os.path.exists(path): - with open(path, 'r') as f: - return int(f.read().strip()) * logical_sector_size - elif block_device.get('ID_PART_ENTRY_SIZE'): - return logical_sector_size * int(block_device['ID_PART_ENTRY_SIZE']) - - def list_partitions(self, disk): - parts = [] - try: - block_device = pyudev.Devices.from_name(pyudev.Context(), 'block', disk) - except pyudev.DeviceNotFoundByNameError: - return parts - - if not block_device.children: - return parts - - logical_sector_size = self.middleware.call_sync('device.logical_sector_size', disk) - - 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 - ): - if disk.startswith('nvme'): - # This is a hack for nvme disks, however let's please come up with a better way - # to link disks with their partitions - part_name = f'{disk}p{p["ID_PART_ENTRY_NUMBER"]}' - else: - part_name = f'{disk}{p["ID_PART_ENTRY_NUMBER"]}' - 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, - 'size': None, - 'id': part_name, - 'path': os.path.join('/dev', part_name), - 'encrypted_provider': None, - } - if logical_sector_size: - part['size'] = logical_sector_size * int(p['ID_PART_ENTRY_SIZE']) - - encrypted_provider = glob.glob(f'/sys/block/dm-*/slaves/{part["name"]}') - if encrypted_provider: - part['encrypted_provider'] = os.path.join('/dev', encrypted_provider[0].split('/')[3]) - parts.append(part) - return parts - - def gptid_from_part_type(self, disk, part_type): - try: - block_device = pyudev.Devices.from_name(pyudev.Context(), 'block', disk) - except pyudev.DeviceNotFoundByNameError: - raise CallError(f'{disk} not found') - - if not block_device.children: - raise CallError(f'{disk} has no partitions') - - part = next( - (p['ID_PART_ENTRY_UUID'] for p in block_device.children if all( - p.get(k) for k in ('ID_PART_ENTRY_UUID', 'ID_PART_ENTRY_TYPE') - ) and p['ID_PART_ENTRY_TYPE'] == part_type), None - ) - if not part: - raise CallError(f'Partition type {part_type} not found on {disk}') - return f'disk/by-partuuid/{part}' - - async def get_zfs_part_type(self): - return '6a898cc3-1dd2-11b2-99a6-080020736631' - - async def get_swap_part_type(self): - return '0657fd6d-a4ab-43c4-84e5-0933c84b4f4f' - - def get_swap_devices(self): - with open('/proc/swaps', 'r') as f: - data = f.read() - devices = [] - for dev_line in filter(lambda l: l.startswith('/dev'), data.splitlines()): - devices.append(dev_line.split()[0]) - return devices - - def label_to_dev(self, label, *args): - dev = os.path.realpath(os.path.join('/dev', label)).split('/')[-1] - return dev if dev != label.split('/')[-1] else None - - def label_to_disk(self, label, *args): - part_disk = self.label_to_dev(label) - return self.get_disk_from_partition(part_disk) if part_disk else None - - def get_disk_from_partition(self, part_name): - if not os.path.exists(os.path.join('/dev', part_name)): - return None - with open(os.path.join('/sys/class/block', part_name, 'partition'), 'r') as f: - part_num = f.read().strip() - if part_name.startswith('nvme'): - # nvme partitions would be like nvmen1p1 where disk is nvmen1 - part_num = f'p{part_num}' - return part_name.rsplit(part_num, 1)[0].strip() diff --git a/src/middlewared/middlewared/plugins/pool.py b/src/middlewared/middlewared/plugins/pool.py index 8ae171954093..39f933775d1f 100644 --- a/src/middlewared/middlewared/plugins/pool.py +++ b/src/middlewared/middlewared/plugins/pool.py @@ -410,11 +410,13 @@ def transform_topology(self, x, options=None): options = options or {} if isinstance(x, dict): if options.get('device_disk', True): + if 'label_to_dev_disk_cache' not in options: + options['label_to_dev_disk_cache'] = self.middleware.call_sync('disk.label_to_dev_disk_cache') path = x.get('path') if path is not None: device = disk = None if path.startswith('/dev/'): - args = [path[5:]] + ([] if osc.IS_LINUX else [options.get('geom_scan', True)]) + args = [path[5:], False, options['label_to_dev_disk_cache']] device = self.middleware.call_sync('disk.label_to_dev', *args) disk = self.middleware.call_sync('disk.label_to_disk', *args) x['device'] = device