From 7a39a53d59791a6f4b030317f84ba6789367fca0 Mon Sep 17 00:00:00 2001 From: Michael Hudson-Doyle Date: Wed, 19 Jul 2023 13:00:54 +1200 Subject: [PATCH] offer reformats for the install media --- .../machines/lp-1986676-missing-osprober.json | 2 +- subiquity/common/filesystem/boot.py | 12 +- subiquity/server/controllers/filesystem.py | 16 ++- .../controllers/tests/test_filesystem.py | 122 +++++++++++++++--- 4 files changed, 127 insertions(+), 25 deletions(-) diff --git a/examples/machines/lp-1986676-missing-osprober.json b/examples/machines/lp-1986676-missing-osprober.json index f7bc0f835..3ab42fd56 100644 --- a/examples/machines/lp-1986676-missing-osprober.json +++ b/examples/machines/lp-1986676-missing-osprober.json @@ -623,7 +623,7 @@ "range": "16", "removable": "1", "ro": "0", - "size": "15728640000", + "size": "157286400000", "stat": " 71504 922 8712293 373538 2643 1512 442264 197181 0 367044 570719 0 0 0 0 0 0", "subsystem": "block", "uevent": "MAJOR=8\nMINOR=48\nDEVNAME=sdd\nDEVTYPE=disk\nDISKSEQ=15" diff --git a/subiquity/common/filesystem/boot.py b/subiquity/common/filesystem/boot.py index 80f8829b5..e16f777d1 100644 --- a/subiquity/common/filesystem/boot.py +++ b/subiquity/common/filesystem/boot.py @@ -15,6 +15,7 @@ import abc import functools +import logging from typing import Any, Optional import attr @@ -29,6 +30,9 @@ ) +log = logging.getLogger('subiquity.common.filesystem.boot') + + @functools.singledispatch def is_boot_device(device): """Is `device` a boot device?""" @@ -331,7 +335,9 @@ def can_be_boot_device(device, *, def _can_be_boot_device_disk(disk, *, resize_partition=None, with_reformatting=False): if with_reformatting: - return True + new_disk = attr.evolve(disk) + new_disk._partitions = [p for p in disk.partitions() if p._is_in_use] + disk = new_disk plan = get_boot_device_plan(disk, resize_partition=resize_partition) return plan is not None @@ -358,7 +364,9 @@ def is_esp(device): @is_esp.register(Partition) def _is_esp_partition(partition): - if not can_be_boot_device(partition.device, with_reformatting=True): + new_disk = attr.evolve(partition.device) + new_disk._partitions = [] + if not can_be_boot_device(new_disk, with_reformatting=True): return False if partition.device.ptable == "gpt": return partition.flag == "boot" diff --git a/subiquity/server/controllers/filesystem.py b/subiquity/server/controllers/filesystem.py index f707be8f9..7e76f1f87 100644 --- a/subiquity/server/controllers/filesystem.py +++ b/subiquity/server/controllers/filesystem.py @@ -496,7 +496,13 @@ def start_guided(self, target: GuidedStorageTarget, def start_guided_reformat(self, target: GuidedStorageTargetReformat, disk: ModelDisk) -> gaps.Gap: """Perform the reformat, and return the resulting gap.""" - self.reformat(disk, wipe='superblock-recursive') + in_use_parts = [p for p in disk.partitions() if p._is_in_use] + if in_use_parts: + for p in disk.partitions(): + if not p._is_in_use: + self.delete_partition(p) + else: + self.reformat(disk, wipe='superblock-recursive') return gaps.largest_gap(disk) @start_guided.register @@ -680,7 +686,7 @@ def potential_boot_disks(self, check_boot=True, with_reformatting=False): if can_be_boot: continue disks.append(disk) - return [d for d in disks if not d._has_in_use_partition] + return [d for d in disks] def _offsets_and_sizes_for_volume(self, volume): offset = self.model._partition_alignment_data['gpt'].min_start_offset @@ -943,7 +949,11 @@ async def v2_guided_GET(self, wait: bool = False) \ scenarios.append((disk.size, reformat)) for disk in self.potential_boot_disks(with_reformatting=False): - parts = [p for p in disk.partitions() if p.flag != 'bios_grub'] + parts = [ + p + for p in disk.partitions() + if p.flag != 'bios_grub' and not p._is_in_use + ] if len(parts) < 1: # On an (essentially) empty disk, don't bother to offer it # with UseGap, as it's basically the same as the Reformat diff --git a/subiquity/server/controllers/tests/test_filesystem.py b/subiquity/server/controllers/tests/test_filesystem.py index 4241f756b..c9b8c4f3c 100644 --- a/subiquity/server/controllers/tests/test_filesystem.py +++ b/subiquity/server/controllers/tests/test_filesystem.py @@ -26,7 +26,7 @@ from subiquitycore.utils import matching_dicts from subiquitycore.tests.util import random_string -from subiquity.common.filesystem import gaps +from subiquity.common.filesystem import gaps, labels from subiquity.common.filesystem.actions import DeviceAction from subiquity.common.types import ( AddPartitionV2, @@ -34,6 +34,7 @@ Gap, GapUsable, GuidedCapability, + GuidedDisallowedCapability, GuidedDisallowedCapabilityReason, GuidedChoiceV2, GuidedStorageTargetManual, @@ -76,6 +77,13 @@ ] +default_capabilities_disallowed_too_small = [ + GuidedDisallowedCapability( + capability=cap, + reason=GuidedDisallowedCapabilityReason.TOO_SMALL) + for cap in default_capabilities] + + class TestSubiquityControllerFilesystem(IsolatedAsyncioTestCase): MOCK_PREFIX = 'subiquity.server.controllers.filesystem.' @@ -646,24 +654,13 @@ async def test_manual(self): async def test_small_blank_disk(self, bootloader, ptable): await self._setup(bootloader, ptable, size=1 << 30) resp = await self.fsc.v2_guided_GET() - self.assertEqual(2, len(resp.targets)) - self.assertEqual(GuidedStorageTargetManual(), resp.targets[1]) - self.assertEqual(0, len(resp.targets[0].allowed)) - self.assertEqual( - { - disabled_cap.capability - for disabled_cap in resp.targets[0].disallowed - }, - set(default_capabilities) - ) - self.assertEqual( - { - disabled_cap.reason - for disabled_cap in resp.targets[0].disallowed - }, - { - GuidedDisallowedCapabilityReason.TOO_SMALL, - }) + expected = [ + GuidedStorageTargetReformat( + disk_id=self.disk.id, allowed=[], + disallowed=default_capabilities_disallowed_too_small), + GuidedStorageTargetManual(), + ] + self.assertEqual(expected, resp.targets) @parameterized.expand(bootloaders_and_ptables) async def test_used_half_disk(self, bootloader, ptable): @@ -904,6 +901,93 @@ async def test_unscaled_disk(self, bootloader, ptable): # but we should using most of the disk, minus boot partition(s) self.assertLess(45 << 30, size) + @parameterized.expand(bootloaders_and_ptables) + async def test_in_use(self, bootloader, ptable): + # Disks with "in use" partitions allow a reformat if there is + # enough space on the rest of the disk. + await self._setup(bootloader, ptable, fix_bios=True) + make_partition( + self.model, self.disk, preserve=True, + size=4 << 30, is_in_use=True) + expected = [ + GuidedStorageTargetReformat( + disk_id=self.disk.id, allowed=default_capabilities), + GuidedStorageTargetManual(), + ] + resp = await self.fsc.v2_guided_GET() + self.assertEqual(expected, resp.targets) + self.assertEqual(ProbeStatus.DONE, resp.status) + + @parameterized.expand(bootloaders_and_ptables) + async def test_in_use_full(self, bootloader, ptable): + # Disks with "in use" partitions allow a reformat (but not + # usegap) if there is enough space on the rest of the disk, + # even if the disk is full of other partitions. + await self._setup(bootloader, ptable, fix_bios=True) + make_partition( + self.model, self.disk, preserve=True, + size=4 << 30, is_in_use=True) + make_partition( + self.model, self.disk, preserve=True, + size=gaps.largest_gap_size(self.disk)) + expected = [ + GuidedStorageTargetReformat( + disk_id=self.disk.id, allowed=default_capabilities), + GuidedStorageTargetManual(), + ] + resp = await self.fsc.v2_guided_GET() + self.assertEqual(expected, resp.targets) + self.assertEqual(ProbeStatus.DONE, resp.status) + + @parameterized.expand(bootloaders_and_ptables) + async def test_in_use_too_small(self, bootloader, ptable): + # Disks with "in use" partitions do not allow a reformat if + # there is not enough space on the rest of the disk. + await self._setup(bootloader, ptable, fix_bios=True, size=5 << 30) + make_partition( + self.model, self.disk, preserve=True, + size=4 << 30, is_in_use=True) + make_partition( + self.model, self.disk, preserve=True, + size=gaps.largest_gap_size(self.disk)) + expected = [ + GuidedStorageTargetReformat( + disk_id=self.disk.id, allowed=default_capabilities), + GuidedStorageTargetManual(), + ] + resp = await self.fsc.v2_guided_GET() + expected = [ + GuidedStorageTargetReformat( + disk_id=self.disk.id, allowed=[], + disallowed=default_capabilities_disallowed_too_small), + GuidedStorageTargetManual(), + ] + self.assertEqual(expected, resp.targets) + + @parameterized.expand(bootloaders_and_ptables) + async def test_in_use_reformat_and_gap(self, bootloader, ptable): + # Disks with "in use" partitions allow both a reformat and a + # usegap if there is an in use partition, another partition + # and a big enough gap. + await self._setup(bootloader, ptable, fix_bios=True) + make_partition( + self.model, self.disk, preserve=True, + size=4 << 30, is_in_use=True) + make_partition( + self.model, self.disk, preserve=True, + size=gaps.largest_gap_size(self.disk)//2) + expected = [ + GuidedStorageTargetReformat( + disk_id=self.disk.id, allowed=default_capabilities), + GuidedStorageTargetUseGap( + disk_id=self.disk.id, allowed=default_capabilities, + gap=labels.for_client(gaps.largest_gap(self.disk))), + GuidedStorageTargetManual(), + ] + resp = await self.fsc.v2_guided_GET() + self.assertEqual(expected, resp.targets) + self.assertEqual(ProbeStatus.DONE, resp.status) + class TestManualBoot(IsolatedAsyncioTestCase): def _setup(self, bootloader, ptable, **kw):