From 0de24b41965d0b9911ca218a83bcb2dccd9351f5 Mon Sep 17 00:00:00 2001 From: "Caleb St. John" <30729806+yocalebo@users.noreply.github.com> Date: Tue, 14 May 2024 13:50:33 -0400 Subject: [PATCH] NAS-128891 / 24.10 / Remove the logic for creating swap partitions (#13695) * remove swap * remove swap logic from disk.format * simplify disk.format * fix test_disk_format.py * fix tests and a regression found after API test run * fix test_disk_format.py * some touch-ups * address reviews * remove swap arg in test_disk_wipe.py as well --- debian/debian/ix-swap.service | 18 -- debian/debian/rules | 1 - .../middlewared/plugins/disk_/disk_events.py | 3 - .../middlewared/plugins/disk_/format.py | 210 +++----------- .../plugins/disk_/swap_configure.py | 256 ------------------ .../middlewared/plugins/disk_/swap_mirror.py | 11 - .../middlewared/plugins/pool_/attach_disk.py | 4 +- .../middlewared/plugins/pool_/export.py | 3 - .../middlewared/plugins/pool_/format_disks.py | 13 +- .../middlewared/plugins/pool_/import_pool.py | 4 - .../middlewared/plugins/pool_/pool.py | 16 +- .../plugins/pool_/pool_disk_operations.py | 6 - .../middlewared/plugins/pool_/replace_disk.py | 21 +- .../middlewared/plugins/zfs_/zfs_events.py | 7 - .../plugins/disk/test_swap_configuration.py | 209 -------------- tests/api2/test_disk_format.py | 106 +------- tests/api2/test_disk_wipe.py | 12 +- tests/api2/test_pool_expand.py | 4 +- tests/api2/test_pool_replace_disk.py | 28 +- 19 files changed, 67 insertions(+), 865 deletions(-) delete mode 100644 debian/debian/ix-swap.service delete mode 100644 src/middlewared/middlewared/plugins/disk_/swap_configure.py delete mode 100644 src/middlewared/middlewared/pytest/unit/plugins/disk/test_swap_configuration.py diff --git a/debian/debian/ix-swap.service b/debian/debian/ix-swap.service deleted file mode 100644 index 314266f18439..000000000000 --- a/debian/debian/ix-swap.service +++ /dev/null @@ -1,18 +0,0 @@ -[Unit] -Description=Configure swap filesystem on boot pool -DefaultDependencies=no - -Before=network-pre.target - -After=middlewared.service -Before=local-fs.target - -[Service] -Type=oneshot -RemainAfterExit=yes -TimeoutStartSec=300 -ExecStart=midclt call disk.swaps_configure -StandardOutput=null - -[Install] -WantedBy=multi-user.target diff --git a/debian/debian/rules b/debian/debian/rules index 649f0d8511a8..f0b89287a4f5 100755 --- a/debian/debian/rules +++ b/debian/debian/rules @@ -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 diff --git a/src/middlewared/middlewared/plugins/disk_/disk_events.py b/src/middlewared/middlewared/plugins/disk_/disk_events.py index e35ecd8893f4..ee5f9297a5c9 100644 --- a/src/middlewared/middlewared/plugins/disk_/disk_events.py +++ b/src/middlewared/middlewared/plugins/disk_/disk_events.py @@ -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): diff --git a/src/middlewared/middlewared/plugins/disk_/format.py b/src/middlewared/middlewared/plugins/disk_/format.py index ad5f23477213..604d2621c2b9 100644 --- a/src/middlewared/middlewared/plugins/disk_/format.py +++ b/src/middlewared/middlewared/plugins/disk_/format.py @@ -1,3 +1,5 @@ +import pathlib + import parted from middlewared.service import CallError, private, Service @@ -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] diff --git a/src/middlewared/middlewared/plugins/disk_/swap_configure.py b/src/middlewared/middlewared/plugins/disk_/swap_configure.py deleted file mode 100644 index 0b5124a35d3e..000000000000 --- a/src/middlewared/middlewared/plugins/disk_/swap_configure.py +++ /dev/null @@ -1,256 +0,0 @@ -import os -import subprocess - -from collections import defaultdict - -from middlewared.service import CallError, lock, private, Service -from middlewared.utils import run - - -MIRROR_MAX = 5 - - -class DiskService(Service): - - @private - @lock('swaps_configure') - async def swaps_configure(self): - """ - Configures swap partitions in the system. - We try to mirror all available swap partitions to avoid a system - crash in case one of them dies. - """ - boot_disks = await self.middleware.call('boot.get_disks') - if any(( - await self.middleware.call('system.is_ha_capable'), - await self.swap_boot_partitions_exist(boot_disks) - )): - disks = boot_disks - else: - disks = await self.middleware.call('pool.get_disks') - disks.extend(boot_disks) - - existing_swap_devices = {'mirrors': [], 'partitions': []} - mirrors = await self.middleware.call('disk.get_swap_mirrors') - encrypted_mirrors = {m['encrypted_provider']: m for m in mirrors if m['encrypted_provider']} - all_partitions = { - p['name']: p - for disk in disks - for p in await self.middleware.call('disk.list_partitions', disk) - } - await self.middleware.call('disk.remove_degraded_mirrors') - - for device in await self.middleware.call('disk.get_swap_devices'): - if device in encrypted_mirrors or device.startswith(('/dev/md', '/dev/mirror/')): - # This is going to be complete path for linux and freebsd - existing_swap_devices['mirrors'].append(device) - else: - existing_swap_devices['partitions'].append(device) - - create_swap_devices = {} - used_partitions_in_mirror = set() - for mirror in mirrors: - mirror_name = (mirror['encrypted_provider'] or mirror['real_path']) - - # If the mirror is degraded or disk is not in a pool lets remove it - if mirror_name in existing_swap_devices['mirrors'] and (len(mirror['providers']) == 1 or any( - p['disk'] not in disks for p in mirror['providers'] - )): - await self.middleware.call( - 'disk.swaps_remove_disks_unlocked', - [p['disk'] for p in mirror['providers']], {'configure_swap': False} - ) - existing_swap_devices['mirrors'].remove(mirror_name) - else: - if mirror_name not in existing_swap_devices['mirrors']: - create_swap_devices[mirror_name] = { - 'path': mirror_name, - 'encrypted_provider': mirror['encrypted_provider'], - } - used_partitions_in_mirror.update(p['name'] for p in mirror['providers']) - - # Get all partitions of swap type, indexed by size - swap_partitions_by_size = defaultdict(list) - valid_swap_part_uuids = await self.middleware.call('disk.get_valid_swap_partition_type_uuids') - for swap_part in filter( - lambda d: d['partition_type'] in valid_swap_part_uuids and d['name'] not in used_partitions_in_mirror, - all_partitions.values() - ): - if swap_part['disk'] in disks: - swap_partitions_by_size[swap_part['size']].append(swap_part['name']) - - unused_partitions = [] - swap_redundancy = await self.middleware.call('disk.swap_redundancy') - for size, partitions in swap_partitions_by_size.items(): - # If we don't have enough partitions for requested redundancy, add them to unused_partitions list - if len(partitions) < swap_redundancy: - unused_partitions += partitions - continue - - for i in range(int(len(partitions) / swap_redundancy)): - if ( - len(create_swap_devices) + len(existing_swap_devices['mirrors']) + - len(existing_swap_devices['partitions']) - ) > MIRROR_MAX: - break - part_list = partitions[0:swap_redundancy] - partitions = partitions[swap_redundancy:] - - # We could have a single disk being used as swap, without mirror. - try: - for p in part_list: - remove = False - part_data = all_partitions[p] - if part_data['encrypted_provider'] in existing_swap_devices['partitions']: - part = part_data['encrypted_provider'] - remove = True - elif part_data['path'] in existing_swap_devices['partitions']: - remove = True - part = part_data['path'] - - if remove: - await self.middleware.call( - 'disk.swaps_remove_disks_unlocked', [part_data['disk']], {'configure_swap': False} - ) - existing_swap_devices['partitions'].remove(part) - except Exception: - self.logger.warn('Failed to remove disk from swap', exc_info=True) - # If something failed here there is no point in trying to create the mirror - continue - - swap_path = await self.new_swap_name() - if not swap_path: - # Which means maximum has been reached and we can stop - break - try: - await self.middleware.call( - 'disk.create_swap_mirror', swap_path, { - 'paths': [all_partitions[p]['path'] for p in part_list], - 'extra': {'level': 1}, - } - ) - except CallError as e: - self.logger.warning('Failed to create swap mirror %s: %s', swap_path, e) - continue - - swap_device = os.path.realpath(os.path.join('/dev/md', swap_path)) - create_swap_devices[swap_device] = { - 'path': swap_device, - 'encrypted_provider': None, - } - - # Add remaining partitions to unused list - unused_partitions += partitions - - if unused_partitions and not create_swap_devices and all( - not existing_swap_devices[k] for k in existing_swap_devices - ): - swap_device = all_partitions[unused_partitions[0]] - create_swap_devices[swap_device['path']] = { - 'path': swap_device['path'], - 'encrypted_provider': swap_device['encrypted_provider'], - } - await run('mdadm', '--zero-superblock', '--force', swap_device['path'], encoding='utf8', check=False) - - created_swap_devices = [] - for swap_path, data in create_swap_devices.items(): - if not data['encrypted_provider']: - cp = await run( - 'cryptsetup', '-d', '/dev/urandom', 'open', '--type', 'plain', - swap_path, swap_path.split('/')[-1], check=False, encoding='utf8', - ) - if cp.returncode: - self.logger.warning('Failed to encrypt %s device: %s', swap_path, cp.stderr) - continue - swap_path = os.path.join('/dev/mapper', swap_path.split('/')[-1]) - else: - swap_path = data['encrypted_provider'] - try: - await run('mkswap', swap_path) - except subprocess.CalledProcessError as e: - self.logger.warning('Failed to make swap for %s: %s', swap_path, e.stderr.decode()) - continue - - try: - await run('swapon', swap_path) - except subprocess.CalledProcessError as e: - self.logger.warning('Failed to activate swap partition %s: %s', swap_path, e.stderr.decode()) - continue - else: - created_swap_devices.append(swap_path) - - if existing_swap_devices['partitions'] and (created_swap_devices or existing_swap_devices['mirrors']): - # This will happen in a case where a single partition existed initially - # then other disks were added of different size. Now we don't use a single partition - # for swap unless there is no existing partition/mirror already configured for swap. - # In this case, we did create a mirror now and existing partitions should be removed from swap - # as a mirror has been configured - all_partitions_by_path = { - p['encrypted_provider'] or p['path']: p['disk'] for p in all_partitions.values() - } - try: - await self.middleware.call( - 'disk.swaps_remove_disks_unlocked', [ - all_partitions_by_path[p] for p in existing_swap_devices['partitions'] - if p in all_partitions_by_path - ] - ) - except Exception as e: - self.logger.warning( - 'Failed to remove %s from swap: %s', ','.join(existing_swap_devices['partitions']), str(e) - ) - else: - existing_swap_devices['partitions'] = [] - - return existing_swap_devices['partitions'] + existing_swap_devices['mirrors'] + created_swap_devices - - @private - async def new_swap_name(self): - """ - Get a new name for a swap mirror - - Returns: - str: name of the swap mirror - """ - used_names = [mirror['name'] for mirror in await self.middleware.call('disk.get_swap_mirrors')] - for i in range(MIRROR_MAX): - name = f'swap{i}' - if name not in used_names: - return name - - @private - async def swap_boot_partitions_exist(self, boot_disks=None): - valid_swap_part_uuids = await self.middleware.call('disk.get_valid_swap_partition_type_uuids') - if boot_disks is None: - boot_disks = await self.middleware.call('boot.get_disks') - - for disk in boot_disks: - for part in await self.middleware.call('disk.list_partitions', disk): - if part['partition_type'] in valid_swap_part_uuids: - return True - - return False - - @private - async def create_swap_partition(self): - return not (await self.middleware.call('system.is_ha_capable') or await self.swap_boot_partitions_exist()) - - @private - async def swap_redundancy(self): - group_size = 2 - pools = await self.middleware.call('pool.query', [['status', 'nin', ('OFFLINE', 'FAULTED')]]) - for pool in pools: - vdevs = pool['topology']['data'] - for vdev in vdevs: - new_size = 2 - if vdev['type'] == 'MIRROR': - new_size = len(vdev['children']) - if vdev['type'] == 'RAIDZ2': - new_size = 3 - if vdev['type'] == 'RAIDZ3': - new_size = 4 - # use max group size - if group_size < new_size: - group_size = new_size - - return group_size diff --git a/src/middlewared/middlewared/plugins/disk_/swap_mirror.py b/src/middlewared/middlewared/plugins/disk_/swap_mirror.py index 8bdadc6acd53..df58911b283a 100644 --- a/src/middlewared/middlewared/plugins/disk_/swap_mirror.py +++ b/src/middlewared/middlewared/plugins/disk_/swap_mirror.py @@ -11,17 +11,6 @@ class DiskService(Service): - @private - async def create_swap_mirror(self, name, options): - extra = options['extra'] - await run('mdadm', '--zero-superblock', '--force', *options['paths'], encoding='utf8', check=False) - cp = await run( - 'mdadm', '--create', os.path.join('/dev/md', name), f'--level={extra.get("level", 1)}', - f'--raid-devices={len(options["paths"])}', '--meta=1.2', *options['paths'], encoding='utf8', check=False, - ) - if cp.returncode: - raise CallError(f'Failed to create mirror {name}: {cp.stderr}') - @private async def destroy_swap_mirror(self, name): mirror = await self.middleware.call('disk.get_swap_mirrors', [['name', '=', name]], {'get': True}) diff --git a/src/middlewared/middlewared/plugins/pool_/attach_disk.py b/src/middlewared/middlewared/plugins/pool_/attach_disk.py index 992ace41f46b..10e2bbe20b86 100644 --- a/src/middlewared/middlewared/plugins/pool_/attach_disk.py +++ b/src/middlewared/middlewared/plugins/pool_/attach_disk.py @@ -54,7 +54,7 @@ async def attach(self, job, oid, options): verrors.check() guid = vdev['guid'] if vdev['type'] in ['DISK', 'RAIDZ1', 'RAIDZ2', 'RAIDZ3'] else vdev['children'][0]['guid'] - disks = {options['new_disk']: {'create_swap': topology_type == 'data', 'vdev': []}} + disks = {options['new_disk']: {'create_swap': False}} await self.middleware.call('pool.format_disks', job, disks) devname = disks[options['new_disk']]['vdev'][0] @@ -63,8 +63,6 @@ async def attach(self, job, oid, options): ]) await job.wrap(extend_job) - self.middleware.create_task(self.middleware.call('disk.swaps_configure')) - if vdev['type'] in ('RAIDZ1', 'RAIDZ2', 'RAIDZ3'): while True: expand = await self.middleware.call('zfs.pool.expand_state', pool['name']) diff --git a/src/middlewared/middlewared/plugins/pool_/export.py b/src/middlewared/middlewared/plugins/pool_/export.py index 5e86b1d8f0c9..e5bfe20cb8f4 100644 --- a/src/middlewared/middlewared/plugins/pool_/export.py +++ b/src/middlewared/middlewared/plugins/pool_/export.py @@ -190,8 +190,5 @@ async def unlabel(disk): # scrub needs to be regenerated in crontab await self.middleware.call('service.restart', 'cron') - # Let's reconfigure swap in case dumpdev needs to be configured again - self.middleware.create_task(self.middleware.call('disk.swaps_configure')) - await self.middleware.call_hook('pool.post_export', pool=pool['name'], options=options) self.middleware.send_event('pool.query', 'REMOVED', id=oid) diff --git a/src/middlewared/middlewared/plugins/pool_/format_disks.py b/src/middlewared/middlewared/plugins/pool_/format_disks.py index 5b924cf6aac4..2bc12fb02636 100644 --- a/src/middlewared/middlewared/plugins/pool_/format_disks.py +++ b/src/middlewared/middlewared/plugins/pool_/format_disks.py @@ -9,22 +9,15 @@ async def format_disks(self, job, disks): """ Format all disks, putting all ZFS partitions created into their respective vdevs. """ - # Make sure all SED disks are unlocked await self.middleware.call('disk.sed_unlock_all') - swapgb = (await self.middleware.call('system.advanced.config'))['swapondrive'] - formatted = 0 await self.middleware.call('pool.remove_unsupported_md_devices_from_disks', disks) - create_swap_partition = await self.middleware.call('disk.create_swap_partition') - len_disks = len(disks) + formatted = 0 + len_disks = len(disks) async def format_disk(arg): nonlocal formatted disk, config = arg - swap_size = 0 - if config['create_swap'] and create_swap_partition: - swap_size = swapgb - # Drives are partitioned to maximize the data partition - await self.middleware.call('disk.format', disk, swap_size) + await self.middleware.call('disk.format', disk) formatted += 1 job.set_progress(15, f'Formatting disks ({formatted}/{len_disks})') diff --git a/src/middlewared/middlewared/plugins/pool_/import_pool.py b/src/middlewared/middlewared/plugins/pool_/import_pool.py index 7441f587744f..6ec932496d4c 100644 --- a/src/middlewared/middlewared/plugins/pool_/import_pool.py +++ b/src/middlewared/middlewared/plugins/pool_/import_pool.py @@ -207,7 +207,6 @@ async def import_pool(self, job, data): await self.middleware.call_hook('pool.post_import', pool) await self.middleware.call('pool.dataset.sync_db_keys', pool['name']) - self.middleware.create_task(self.middleware.call('disk.swaps_configure')) self.middleware.send_event('pool.query', 'ADDED', id=pool_id, fields=pool) return True @@ -481,9 +480,6 @@ def import_on_boot(self, job): self.unlock_on_boot_impl(name) - # no reason to wait on this to complete - self.middleware.call_sync('disk.swaps_configure', background=True) - # TODO: we need to fix this. There is 0 reason to do all this stuff # and block the entire boot-up process. self.logger.debug('Calling pool.post_import') diff --git a/src/middlewared/middlewared/plugins/pool_/pool.py b/src/middlewared/middlewared/plugins/pool_/pool.py index b6f9662a08bd..bf463d7b6185 100644 --- a/src/middlewared/middlewared/plugins/pool_/pool.py +++ b/src/middlewared/middlewared/plugins/pool_/pool.py @@ -208,12 +208,9 @@ def pool_extend(self, pool, context): return pool async def __convert_topology_to_vdevs(self, topology): - # We do two things here: - # 1. Gather all disks transversing the topology - # 2. Keep track of the vdev each disk is supposed to be located - # along with a flag whether we should use swap partition in said vdev - # This is required so we can format all disks in one pass, allowing it - # to be performed in parallel if we wish to do so. + # Gather all disks transversing the topology so we can + # format all disks in one pass, allowing it to be performed + # in parallel if we wish to do so. disks = {} vdevs = [] for i in ('data', 'cache', 'log', 'special', 'dedup'): @@ -231,10 +228,8 @@ async def __convert_topology_to_vdevs(self, topology): vdev['draid_data_disks'] = t_vdev['draid_data_disks'] vdev['draid_spare_disks'] = t_vdev['draid_spare_disks'] vdevs.append(vdev) - # cache and log devices should not have a swap - create_swap = True if i == 'data' else False for disk in t_vdev['disks']: - disks[disk] = {'vdev': vdev_devs_list, 'create_swap': create_swap} + disks[disk] = {'vdev': vdev_devs_list, 'create_swap': False} if topology.get('spares'): vdev_devs_list = [] @@ -244,7 +239,7 @@ async def __convert_topology_to_vdevs(self, topology): 'devices': vdev_devs_list, }) for disk in topology['spares']: - disks[disk] = {'vdev': vdev_devs_list, 'create_swap': True} + disks[disk] = {'vdev': vdev_devs_list, 'create_swap': False} return disks, vdevs @@ -665,7 +660,6 @@ async def do_create(self, job, data): # There is really no point in waiting all these services to reload so do them # in background. - self.middleware.create_task(self.middleware.call('disk.swaps_configure')) self.middleware.create_task(self.middleware.call('pool.restart_services')) pool = await self.get_instance(pool_id) diff --git a/src/middlewared/middlewared/plugins/pool_/pool_disk_operations.py b/src/middlewared/middlewared/plugins/pool_/pool_disk_operations.py index 40442137c7b0..83bc943cf1fb 100644 --- a/src/middlewared/middlewared/plugins/pool_/pool_disk_operations.py +++ b/src/middlewared/middlewared/plugins/pool_/pool_disk_operations.py @@ -139,12 +139,6 @@ async def online(self, oid, options): await self.middleware.call('zfs.pool.online', pool['name'], found[1]['guid']) - disk = await self.middleware.call( - 'disk.label_to_disk', found[1]['path'].replace('/dev/', '') - ) - if disk: - self.middleware.create_task(self.middleware.call('disk.swaps_configure')) - return True @item_method diff --git a/src/middlewared/middlewared/plugins/pool_/replace_disk.py b/src/middlewared/middlewared/plugins/pool_/replace_disk.py index c9f440dd937e..4165cbf6a156 100644 --- a/src/middlewared/middlewared/plugins/pool_/replace_disk.py +++ b/src/middlewared/middlewared/plugins/pool_/replace_disk.py @@ -59,19 +59,6 @@ async def replace(self, job, oid, options): verrors.check() - create_swap = found[0] in ('data', 'spare') - - min_size = None - if create_swap: - for vdev in found[2]: - if vdev.get('device'): - size = await self.middleware.call('disk.get_dev_size', vdev['device']) - if size is not None: - if min_size is None: - min_size = size - else: - min_size = min(min_size, size) - swap_disks = [disk['devname']] from_disk = None if found[1] and await self.middleware.run_in_thread(os.path.exists, found[1]['path']): @@ -85,8 +72,8 @@ async def replace(self, job, oid, options): await self.middleware.call('pool.format_disks', job, { disk['devname']: { 'vdev': vdev, - 'size': min_size, - 'create_swap': create_swap, + 'size': None, # pool.format_disks checks size of disk + 'create_swap': False, }, }) @@ -99,10 +86,6 @@ async def replace(self, job, oid, options): else: if from_disk: await self.middleware.call('disk.wipe', from_disk, 'QUICK') - finally: - # Needs to happen even if replace failed to put back disk that had been - # removed from swap prior to replacement - self.middleware.create_task(self.middleware.call('disk.swaps_configure')) if options['preserve_settings']: filters = [['zfs_guid', '=', options['label']]] diff --git a/src/middlewared/middlewared/plugins/zfs_/zfs_events.py b/src/middlewared/middlewared/plugins/zfs_/zfs_events.py index 35e407c1ae45..745478e64fdf 100644 --- a/src/middlewared/middlewared/plugins/zfs_/zfs_events.py +++ b/src/middlewared/middlewared/plugins/zfs_/zfs_events.py @@ -165,13 +165,6 @@ async def zfs_events(middleware, data): ): pool_name = data.get('pool') pool_guid = data.get('guid') - if await middleware.call('system.ready'): - # Swap must be configured only on disks being used by some pool, - # for this reason we must react to certain types of ZFS events to keep - # it in sync every time there is a change. Also, we only want to configure swap - # if system is ready as otherwise when the pools have not been imported, - # middleware will remove swap disks as all pools might not have imported - middleware.create_task(middleware.call('disk.swaps_configure')) if pool_name: await middleware.call('cache.pop', 'VolumeStatusAlerts') diff --git a/src/middlewared/middlewared/pytest/unit/plugins/disk/test_swap_configuration.py b/src/middlewared/middlewared/pytest/unit/plugins/disk/test_swap_configuration.py deleted file mode 100644 index 68a1be37d314..000000000000 --- a/src/middlewared/middlewared/pytest/unit/plugins/disk/test_swap_configuration.py +++ /dev/null @@ -1,209 +0,0 @@ -import pytest - -from unittest.mock import AsyncMock, patch, Mock - -from middlewared.plugins.disk_.swap_configure import DiskService -from middlewared.pytest.unit.middleware import Middleware - - -ALL_PARTITIONS = { - 'sda': [ - { - 'name': 'sda1', - 'partition_type': '21686148-6449-6e6f-744e-656564454649', - 'partition_uuid': 'ee6e4763-1e61-466d-9972-e3c46dfc4a1c', - 'disk': 'sda', - 'size': 1048576, - 'path': '/dev/sda1', - 'encrypted_provider': None - }, - { - 'name': 'sda2', - 'partition_type': 'c12a7328-f81f-11d2-ba4b-00a0c93ec93b', - 'partition_uuid': 'd83226a3-246f-4242-9468-d8deb08ea3d6', - 'disk': 'sda', - 'size': 536870912, - 'path': '/dev/sda2', - 'encrypted_provider': None - }, - { - 'name': 'sda3', - 'partition_type': '6a898cc3-1dd2-11b2-99a6-080020736631', - 'partition_uuid': '89cf93fd-a5a8-4106-8d4e-eec800124bcd', - 'disk': 'sda', - 'size': 46704606720, - 'path': '/dev/sda3', - 'encrypted_provider': None - }, - { - 'name': 'sda4', - 'partition_type': '0657fd6d-a4ab-43c4-84e5-0933c84b4f4f', - 'partition_uuid': 'cca6903c-0ded-475d-a622-98855d32437e', - 'disk': 'sda', - 'size': 17179869184, - 'path': '/dev/sda4', - 'encrypted_provider': None - } - ], - 'sdb': [ - { - 'name': 'sdb1', - 'partition_type': '0657fd6d-a4ab-43c4-84e5-0933c84b4f4f', - 'partition_uuid': '8c1d813e-c1ae-4f80-b15d-dd57375266bc', - 'disk': 'sdb', - 'size': 2147484160, - 'path': '/dev/sdb1', - 'encrypted_provider': None - }, - { - 'name': 'sdb2', - 'partition_type': '6a898cc3-1dd2-11b2-99a6-080020736631', - 'partition_uuid': '41adc2c8-8b87-4e93-9e01-539a544d1c00', - 'disk': 'sdb', - 'size': 19324207104, - 'path': '/dev/sdb2', - 'encrypted_provider': None - } - ], - 'sdc': [ - { - 'name': 'sdc1', - 'partition_type': '0657fd6d-a4ab-43c4-84e5-0933c84b4f4f', - 'partition_uuid': '22227899-09b0-4019-b439-31b1481aad53', - 'disk': 'sdc', - 'size': 2147484160, - 'path': '/dev/sdc1', - 'encrypted_provider': None - }, - { - 'name': 'sdc2', - 'partition_type': '6a898cc3-1dd2-11b2-99a6-080020736631', - 'partition_number': 2, - 'partition_uuid': '40fa5413-e654-4570-b71b-d8f6657fbc96', - 'disk': 'sdc', - 'size': 19324207104, - 'path': '/dev/sdc2', - 'encrypted_provider': None - } - ], - 'sdd': [ - { - 'name': 'sdd1', - 'partition_type': '21686148-6449-6e6f-744e-656564454649', - 'partition_uuid': 'ee6e4763-1e61-466d-9972-e3c46dfc4a1c', - 'disk': 'sda', - 'size': 1048576, - 'path': '/dev/sda1', - 'encrypted_provider': None - }, - { - 'name': 'sdd2', - 'partition_type': 'c12a7328-f81f-11d2-ba4b-00a0c93ec93b', - 'partition_uuid': 'd83226a3-246f-4242-9468-d8deb08ea3d6', - 'disk': 'sda', - 'size': 536870912, - 'path': '/dev/sda2', - 'encrypted_provider': None - }, - { - 'name': 'sdd3', - 'partition_type': '6a898cc3-1dd2-11b2-99a6-080020736631', - 'partition_uuid': '89cf93fd-a5a8-4106-8d4e-eec800124bcd', - 'disk': 'sda', - 'size': 46704606720, - 'path': '/dev/sda3', - 'encrypted_provider': None - }, - { - 'name': 'sdd4', - 'partition_type': '0657fd6d-a4ab-43c4-84e5-0933c84b4f4f', - 'partition_uuid': 'cca6903c-0ded-475d-a622-98855d32437e', - 'disk': 'sda', - 'size': 17179869184, - 'path': '/dev/sda4', - 'encrypted_provider': None - }, - ], - 'sde': [ - { - 'name': 'sde1', - 'partition_type': '0657fd6d-a4ab-43c4-84e5-0933c84b4f4f', - 'partition_uuid': '8c1d813e-c1ae-4f80-b15d-dd57375266bc', - 'disk': 'sde', - 'size': 2147484160, - 'path': '/dev/sde1', - 'encrypted_provider': None - }, - { - 'name': 'sde2', - 'partition_type': '6a898cc3-1dd2-11b2-99a6-080020736632', - 'partition_uuid': '41adc2c8-8b87-4e93-9e01-539a544d1c00', - 'disk': 'sde', - 'size': 19324207104, - 'path': '/dev/sde2', - 'encrypted_provider': None - } - ], - 'sdf': [ - { - 'name': 'sdf1', - 'partition_type': '0657fd6d-a4ab-43c4-84e5-0933c84b4f4f', - 'partition_uuid': '22227899-09b0-4019-b439-31b1481aad53', - 'disk': 'sdf', - 'size': 2147484160, - 'path': '/dev/sdf1', - 'encrypted_provider': None - }, - { - 'name': 'sdf2', - 'partition_type': '6a898cc3-1dd2-11b2-99a6-080020736631', - 'partition_number': 2, - 'partition_uuid': '40fa5413-e654-4570-b71b-d8f6657fbc96', - 'disk': 'sdf', - 'size': 19324207104, - 'path': '/dev/sdf2', - 'encrypted_provider': None - } - ], - 'sdg': [ - { - 'name': 'sdg2', - 'partition_type': '6a898cc3-1dd2-11b2-99a6-080020736631', - 'partition_number': 2, - 'partition_uuid': '40fa5413-e654-4570-b71b-d8f6657fbc96', - 'disk': 'sdg', - 'size': 19324207104, - 'path': '/dev/sdf2', - 'encrypted_provider': None - } - ] -} - - -@pytest.mark.parametrize('pool_disks,boot_disk,expected_output', [ - (['sdb', 'sdc'], ['sda'], ['/dev/mapper/sda4']), # A mirror with a boot swap partition - (['sdb', 'sdc'], ['sdg'], ['/dev/mapper/swap0']), # A mirror without any boot swap partition - (['sdb', 'sdc', 'sde', 'sdf'], ['sdg'], ['/dev/mapper/swap0', '/dev/mapper/swap1']), # Two mirrors - ([], ['sda', 'sdd'], ['/dev/mapper/swap0']), # A boot mirror - (['sdb', 'sdc', 'sde', 'sdf'], ['sda', 'sdd'], ['/dev/mapper/swap0']), # Boot mirror - (['sdf'], ['sdg'], ['/dev/mapper/sdf1']), # Single swap partition - (['sdf', 'sda'], ['sdg'], ['/dev/mapper/sdf1']), # Swap partitions with different sizes -]) -@pytest.mark.asyncio -async def test_swap_partition_configuration(pool_disks, boot_disk, expected_output): - m = Middleware() - m['disk.swap_redundancy'] = AsyncMock(return_value=2) - m['system.is_ha_capable'] = AsyncMock(return_value=False) - m['pool.get_disks'] = AsyncMock(return_value=pool_disks) - m['boot.get_disks'] = AsyncMock(return_value=boot_disk) - m['disk.get_swap_mirrors'] = AsyncMock(return_value=[]) - m['disk.list_partitions'] = lambda disk: ALL_PARTITIONS[disk] - m['disk.remove_degraded_mirrors'] = AsyncMock(return_value=None) - m['disk.get_swap_devices'] = AsyncMock(return_value=[]) - m['disk.get_valid_swap_partition_type_uuids'] = AsyncMock(return_value=['0657fd6d-a4ab-43c4-84e5-0933c84b4f4f']) - m['disk.swaps_remove_disks_unlocked'] = AsyncMock(return_value=None) - m['disk.create_swap_mirror'] = AsyncMock(return_value=None) - with patch('middlewared.plugins.disk_.swap_configure.run') as run: - run.return_value = Mock(returncode=0) - with patch('os.path.realpath', side_effect=expected_output): - assert (await DiskService(m).swaps_configure()) == expected_output diff --git a/tests/api2/test_disk_format.py b/tests/api2/test_disk_format.py index c8116397182b..dea7201ac75c 100644 --- a/tests/api2/test_disk_format.py +++ b/tests/api2/test_disk_format.py @@ -1,6 +1,5 @@ import pytest -from middlewared.service_exception import CallError from middlewared.test.integration.utils import call, ssh """ @@ -24,14 +23,8 @@ # Some 'constants' MBR_SECTOR_GAP = 34 -NO_SWAP = 0 -WITH_2GB_SWAP = 2 ONE_MB = 1048576 -ONE_GB = (ONE_MB * 1024) -SWAP_SIZE = (WITH_2GB_SWAP * ONE_GB) - DATA_TYPE_UUID = "6a898cc3-1dd2-11b2-99a6-080020736631" -SWAP_TYPE_UUID = "0657fd6d-a4ab-43c4-84e5-0933c84b4f4f" # Currently, we use the same 'unused' disk for all tests @@ -46,12 +39,10 @@ def unused_disk(): minimum_io_size = int(ssh(f"cat /sys/block/{disk['name']}/queue/minimum_io_size")) grain_size = 0 - if 0 == optimal_io_size and 0 == alignment_offset: - if 0 == minimum_io_size % 2: - # Alignment value in units of sectors - grain_size = ONE_MB / dd['sectorsize'] - - if 0 != optimal_io_size: + if all((optimal_io_size == 0, alignment_offset == 0, minimum_io_size % 2 == 0)): + # Alignment value in units of sectors + grain_size = ONE_MB / dd['sectorsize'] + elif optimal_io_size != 0: grain_size = optimal_io_size first_sector = alignment_offset if alignment_offset != 0 else grain_size @@ -59,14 +50,12 @@ def unused_disk(): return (disk, dd, grain_size, first_sector) -def test_disk_format_without_swap(unused_disk): - """ - Generate a single data partition, no swap - """ +def test_disk_format(unused_disk): + """Generate a single data partition""" disk, dd, grain_size, first_sector = unused_disk assert grain_size != 0, 'ERROR: Cannot run this test without a non-zero grain_size' - call('disk.format', disk['name'], NO_SWAP) + call('disk.format', disk['name']) partitions = call('disk.list_partitions', disk['name']) assert len(partitions) == 1 @@ -75,7 +64,8 @@ def test_disk_format_without_swap(unused_disk): assert partitions[0]['partition_type'] == DATA_TYPE_UUID # Should be a modulo of grain_size - assert (partitions[0]['end_sector'] - partitions[0]['start_sector']) % grain_size == 0 + assert partitions[0]['size'] % grain_size == 0 + assert partitions[0]['start_sector'] % grain_size == 0 # Uses (almost) all the disk assert partitions[0]['start_sector'] == first_sector @@ -88,76 +78,6 @@ def test_disk_format_without_swap(unused_disk): assert partitions[0]['size'] > disk['size'] * 0.99 -def test_disk_format_with_swap(unused_disk): - """ - Generate two partitions: - 1: swap (2 GiB) - 2: data - """ - disk, dd, grain_size, first_sector = unused_disk - assert grain_size != 0, 'ERROR: Cannot run this test without a non-zero grain_size' - - call('disk.format', disk['name'], WITH_2GB_SWAP) - - partitions = call('disk.list_partitions', disk['name']) - assert len(partitions) == 2 - - # The first partition should be swap - assert partitions[0]['partition_type'] == SWAP_TYPE_UUID - - # Swap should start at the specified offset and be the requested size - assert partitions[0]['start_sector'] == first_sector - num_swap_sectors = partitions[0]['end_sector'] - partitions[0]['start_sector'] - assert num_swap_sectors == SWAP_SIZE / dd['sectorsize'] - - # Should be a modulo of grain_size - assert (partitions[0]['end_sector'] - partitions[0]['start_sector']) % grain_size == 0 - - # Hand wavey swap size test - assert int(partitions[0]['size'] / (1024 ** 3) + 0.5) == 2 - - # The second partition should be data - assert partitions[1]['partition_type'] == DATA_TYPE_UUID - - # The data partition should start after a 'grain_size' gap - assert partitions[1]['start_sector'] == partitions[0]['end_sector'] + grain_size - - # and be a modulo of grain_size - assert (partitions[1]['end_sector'] - partitions[1]['start_sector']) % grain_size == 0 - - # and be maximal sized - assert partitions[1]['end_sector'] >= dd['blocks'] - grain_size - - # And does not clobber the MBR data at the end - assert partitions[0]['end_sector'] < dd['blocks'] - MBR_SECTOR_GAP - - # Hand-wavy data size test - assert partitions[1]['size'] > (disk['size'] - partitions[0]['size']) * 0.99 - - -@pytest.mark.parametrize("swap_val", [-10, 2.5, 1024]) -def test_disk_format_with_invalid_swap(unused_disk, swap_val): - """ - Confirm we can handle erroneous input - """ - assert unused_disk[2] != 0, 'ERROR: Should not run this test without a non-zero grain_size' - disk = unused_disk[0] - - with pytest.raises(CallError) as e: - call('disk.format', disk['name'], swap_val) - - # The error response is input dependent - if swap_val > 100: - assert e.value.errmsg == ( - f'Disk {disk["name"]!r} capacity is too small. ' - 'Please use a larger capacity drive or reduce swap.' - ) - else: - assert e.value.errmsg == ( - 'Requested swap must be a non-negative integer' - ) - - def test_disk_format_removes_existing_partition_table(unused_disk): """ Confirm we can repartion @@ -165,12 +85,10 @@ def test_disk_format_removes_existing_partition_table(unused_disk): assert unused_disk[2] != 0, 'ERROR: Should not run this test without a non-zero grain_size' disk = unused_disk[0] - call('disk.format', disk['name'], 2) - # We should now have two partitions: swap and data partitions = call('disk.list_partitions', disk['name']) - assert len(partitions) == 2 + assert len(partitions) == 1 - call('disk.format', disk['name'], 0) - # We should now have one partition: data + # format removes existing partition labels and creates a new (data) partition + call('disk.format', disk['name']) partitions = call('disk.list_partitions', disk['name']) assert len(partitions) == 1 diff --git a/tests/api2/test_disk_wipe.py b/tests/api2/test_disk_wipe.py index 06927e88ca03..cf265d413294 100644 --- a/tests/api2/test_disk_wipe.py +++ b/tests/api2/test_disk_wipe.py @@ -31,19 +31,18 @@ def test_disk_wipe_partition_clean(): disk = call("disk.get_unused")[0]["name"] - # Create 1 GiB swap and a data partition - call('disk.format', disk, 1) + # Create a data partition + call('disk.format', disk) parts = call('disk.list_partitions', disk) - seek_blk = parts[1]['start_sector'] + seek_blk = parts[0]['start_sector'] blk_size = int(parts[0]['start'] / parts[0]['start_sector']) # Write some private data into the start of the data partition - cmd = ( + ssh( f"echo '{signal_msg}' > junk;" f"dd if=junk bs={blk_size} count=1 oseek={seek_blk} of=/dev/{disk};" "rm -f junk" ) - ssh(cmd) # Confirm presence readback_presence = ssh(f"dd if=/dev/{disk} bs={blk_size} iseek={seek_blk} count=1").splitlines()[0] @@ -64,8 +63,7 @@ def test_disk_wipe_partition_clean(): proc_partitions = str(ssh('cat /proc/partitions')) # If the wipe is truly successful /proc/partitions should have a singular # entry for 'disk' in the table - disk_entries = [line for line in proc_partitions.splitlines() if disk in line] - assert len(disk_entries) == 1 + assert len([line for line in proc_partitions.splitlines() if disk in line]) == 1 @pytest.mark.parametrize('dev_name', ['BOOT', 'UNUSED', 'bogus', '']) diff --git a/tests/api2/test_pool_expand.py b/tests/api2/test_pool_expand.py index ac5c6c73ddc9..8345e1d44919 100644 --- a/tests/api2/test_pool_expand.py +++ b/tests/api2/test_pool_expand.py @@ -11,8 +11,8 @@ def test_expand_pool(): # Transform this pool into a pool on a vdev with a partition that is only 2 GiB ssh(f"zpool export {pool['name']}") - ssh(f"sgdisk -d 2 /dev/{disk}") - ssh(f"sgdisk -n 2:0:+2GiB -t 2:BF01 /dev/{disk}") + ssh(f"sgdisk -d 1 /dev/{disk}") + ssh(f"sgdisk -n 1:0:+2GiB -t 1:BF01 /dev/{disk}") small_partition = call("disk.list_partitions", disk)[-1] assert small_partition["size"] < 2147483648 * 1.01 device = "disk/by-partuuid/" + small_partition["partition_uuid"] diff --git a/tests/api2/test_pool_replace_disk.py b/tests/api2/test_pool_replace_disk.py index a666a3be9580..0f51222231ef 100644 --- a/tests/api2/test_pool_replace_disk.py +++ b/tests/api2/test_pool_replace_disk.py @@ -1,7 +1,7 @@ import pytest from time import sleep -from middlewared.test.integration.assets.pool import mirror_topology, another_pool_topologies, another_pool +from middlewared.test.integration.assets.pool import another_pool_topologies, another_pool from middlewared.test.integration.utils import call from auto_config import ha pytestmark = [ @@ -49,29 +49,3 @@ def test_pool_replace_disk(topology, i): assert call("disk.get_instance", new_disk["identifier"], {"extra": {"pools": True}})["pool"] == pool["name"] assert call("disk.get_instance", to_replace_disk["identifier"], {"extra": {"pools": True}})["pool"] is None - - -@pytest.mark.parametrize("swaps", [[0, 0], [2, 2], [2, 4]]) -def test_pool_replace_disk_swap(swaps): - unused = call("disk.get_unused") - if len(unused) < 3: - raise RuntimeError(f"At least 3 unused disks required to run this test") - - test_disks = unused[:3] - - sizes = {disk["name"]: disk["size"] for disk in test_disks} - assert len(set(sizes.values())) == 1, sizes - - call("system.advanced.update", {"swapondrive": swaps[0]}) - try: - with another_pool(topology=mirror_topology) as pool: - to_replace_vdev = disks(pool["topology"])[0] - - call("system.advanced.update", {"swapondrive": swaps[1]}) - call("pool.replace", pool["id"], { - "label": to_replace_vdev["guid"], - "disk": test_disks[2]["identifier"], - "force": True, - }, job=True) - finally: - call("system.advanced.update", {"swapondrive": 2})