Merge pull request #1213 from dbungert/offset-in-gap

filesystem: show offset in gap & partition
This commit is contained in:
Dan Bungert 2022-03-14 14:57:01 -06:00 committed by GitHub
commit 6f89bcb64c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 126 additions and 44 deletions

View File

@ -23,7 +23,6 @@ from subiquity.models.filesystem import (
Disk,
LVM_CHUNK_SIZE,
LVM_VolGroup,
GPT_OVERHEAD,
Raid,
)
@ -31,6 +30,7 @@ from subiquity.models.filesystem import (
@attr.s(auto_attribs=True)
class Gap:
device: object
offset: int
size: int
type: str = 'gap'
@ -51,6 +51,8 @@ def parts_and_gaps_disk(device):
return []
r = []
used = 0
ad = device.alignment_data()
used += ad.min_start_offset
for p in device._partitions:
used = align_up(used + p.size, 1 << 20)
r.append(p)
@ -58,9 +60,9 @@ def parts_and_gaps_disk(device):
return r
if device.ptable == 'vtoc' and len(device._partitions) >= 3:
return r
end = align_down(device.size, 1 << 20) - GPT_OVERHEAD
end = align_down(device.size - ad.min_end_offset, 1 << 20)
if end - used >= (1 << 20):
r.append(Gap(device, end - used))
r.append(Gap(device, used, end - used))
return r
@ -75,7 +77,7 @@ def _parts_and_gaps_vg(device):
return r
remaining = align_down(device.size - used, LVM_CHUNK_SIZE)
if remaining >= LVM_CHUNK_SIZE:
r.append(Gap(device, remaining))
r.append(Gap(device, 0, remaining))
return r

View File

@ -310,10 +310,11 @@ def _for_client_partition(partition, *, min_size=0):
boot=partition.boot,
annotations=annotations(partition) + usage_labels(partition),
os=partition.os,
offset=partition.offset,
mount=partition.mount,
format=partition.format)
@for_client.register(gaps.Gap)
def _for_client_gap(gap, *, min_size=0):
return types.Gap(offset=0, size=gap.size)
return types.Gap(offset=gap.offset, size=gap.size)

View File

@ -134,10 +134,9 @@ class FilesystemManipulator:
self.model.remove_filesystem(fs)
delete_format = delete_filesystem
def create_partition(self, device, spec, flag="", wipe=None,
grub_device=None):
def create_partition(self, device, gap, spec, **kw):
part = self.model.add_partition(
device, spec["size"], flag, wipe, grub_device)
device, spec["size"], offset=gap.offset, **kw)
self.create_filesystem(part, spec)
return part
@ -152,7 +151,8 @@ class FilesystemManipulator:
if part_size > gaps.largest_gap_size(disk):
largest_part = max(disk.partitions(), key=lambda p: p.size)
largest_part.size -= (part_size - gaps.largest_gap_size(disk))
return self.create_partition(disk, spec, **kwargs)
gap = gaps.largest_gap(disk)
return self.create_partition(disk, gap, spec, **kwargs)
def _create_boot_partition(self, disk):
bootloader = self.model.bootloader
@ -290,7 +290,8 @@ class FilesystemManipulator:
spec['size'], part.size, gaps.largest_gap_size(disk))
spec['size'] = gaps.largest_gap_size(disk)
self.create_partition(disk, spec)
gap = gaps.largest_gap(disk)
self.create_partition(disk, gap, spec)
log.debug("Successfully added partition")

View File

@ -165,17 +165,17 @@ class TestActions(unittest.TestCase):
self.assertActionNotPossible(gpt_disk2, DeviceAction.TOGGLE_BOOT)
# Unless there is already a bios_grub partition we can reuse
gpt_disk3 = make_disk(model, ptable='gpt', preserve=True)
make_partition(
model, gpt_disk3, flag="bios_grub", preserve=True)
make_partition(
model, gpt_disk3, preserve=True)
make_partition(model, gpt_disk3, flag="bios_grub", preserve=True,
offset=1 << 20, size=512 << 20)
make_partition(model, gpt_disk3, preserve=True,
offset=513 << 20, size=8192 << 20)
self.assertActionPossible(gpt_disk3, DeviceAction.TOGGLE_BOOT)
# Edge case city: the bios_grub partition has to be first
gpt_disk4 = make_disk(model, ptable='gpt', preserve=True)
make_partition(
model, gpt_disk4, preserve=True)
make_partition(
model, gpt_disk4, flag="bios_grub", preserve=True)
make_partition(model, gpt_disk4, preserve=True,
offset=1 << 20, size=8192 << 20)
make_partition(model, gpt_disk4, flag="bios_grub", preserve=True,
offset=8193 << 20, size=512 << 20)
self.assertActionNotPossible(gpt_disk4, DeviceAction.TOGGLE_BOOT)
def _test_TOGGLE_BOOT_boot_partition(self, bl, flag):
@ -200,7 +200,8 @@ class TestActions(unittest.TestCase):
make_partition(model, old_disk, preserve=True)
self.assertActionNotPossible(old_disk, DeviceAction.TOGGLE_BOOT)
# If there is an existing ESP/PReP partition though, fine!
make_partition(model, old_disk, flag=flag, preserve=True)
make_partition(model, old_disk, flag=flag, preserve=True,
offset=1 << 20, size=512 << 20)
self.assertActionPossible(old_disk, DeviceAction.TOGGLE_BOOT)
def test_disk_action_TOGGLE_BOOT_UEFI(self):
@ -445,4 +446,4 @@ class TestActions(unittest.TestCase):
self.assertActionNotSupported(lv, DeviceAction.TOGGLE_BOOT)
def test_gap_PARTITION(self):
self.assertActionPossible(gaps.Gap(None, 0), DeviceAction.PARTITION)
self.assertActionPossible(gaps.Gap(None, 0, 0), DeviceAction.PARTITION)

View File

@ -0,0 +1,32 @@
# Copyright 2022 Canonical, Ltd.
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU Affero General Public License as
# published by the Free Software Foundation, either version 3 of the
# License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU Affero General Public License for more details.
#
# You should have received a copy of the GNU Affero General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
import unittest
from subiquity.models.tests.test_filesystem import (
make_model_and_disk,
)
from subiquity.common.filesystem import gaps
class TestGaps(unittest.TestCase):
def test_basic(self):
model, disk1 = make_model_and_disk()
pg = gaps.parts_and_gaps(disk1)
self.assertEqual(1, len(pg))
self.assertTrue(isinstance(pg[0], gaps.Gap))
self.assertEqual(1024 * 1024, pg[0].offset)

View File

@ -225,13 +225,24 @@ class TestFilesystemManipulator(unittest.TestCase):
disk1, size=512 << 20, flag="boot")
disk1p1.preserve = True
disk1p2 = manipulator.model.add_partition(
disk1, size=gaps.largest_gap_size(disk1))
disk1, size=8192 << 20, offset=513 << 20)
disk1p2.preserve = True
manipulator.partition_disk_handler(
disk1, disk1p2, {'fstype': 'ext4', 'mount': '/'})
efi_mnt = manipulator.model._mount_for_path("/boot/efi")
self.assertEqual(efi_mnt.device.volume, disk1p1)
def test_add_boot_has_valid_offset(self):
for bl in Bootloader:
if bl == Bootloader.NONE:
continue
manipulator = make_manipulator(bl)
disk1 = make_disk(manipulator.model, preserve=True)
manipulator.add_boot_disk(disk1)
part = gaps.parts_and_gaps(disk1)[0]
self.assertEqual(1024 * 1024, part.offset)
class TestPartitionSizeScaling(unittest.TestCase):
def test_scale_factors(self):

View File

@ -266,6 +266,7 @@ class Partition:
# does this partition represent the actual boot partition for this device?
boot: Optional[bool] = None
os: Optional[OsProber] = None
offset: Optional[int] = None
@attr.s(auto_attribs=True)

View File

@ -671,6 +671,7 @@ class Partition(_Formattable):
grub_device = attr.ib(default=False)
name = attr.ib(default=None)
multipath = attr.ib(default=None)
offset = attr.ib(default=None)
def available(self):
if self.flag in ['bios_grub', 'prep'] or self.grub_device:
@ -1377,16 +1378,19 @@ class FilesystemModel(object):
_remove_backlinks(obj)
self._actions.remove(obj)
def add_partition(self, device, size, flag="", wipe=None,
def add_partition(self, device, size, *, offset=None, flag="", wipe=None,
grub_device=None):
from subiquity.common.filesystem import boot
real_size = align_up(size)
log.debug("add_partition: rounded size from %s to %s", size, real_size)
if offset is None:
from subiquity.common.filesystem.gaps import largest_gap
offset = largest_gap(device).offset
if device._fs is not None:
raise Exception("%s is already formatted" % (device,))
p = Partition(
m=self, device=device, size=real_size, flag=flag, wipe=wipe,
grub_device=grub_device)
grub_device=grub_device, offset=offset)
if boot.is_bootloader_partition(p):
device._partitions.insert(0, device._partitions.pop())
device.ptable = device.ptable_for_new_partition()

View File

@ -161,13 +161,18 @@ def make_model_and_disk(bootloader=None):
return model, make_disk(model)
def make_partition(model, device=None, *, preserve=False, size=None, **kw):
def make_partition(model, device=None, *, preserve=False, size=None,
offset=None, **kw):
if device is None:
device = make_disk(model)
if size is None:
size = gaps.largest_gap_size(device)//2
partition = Partition(
m=model, device=device, size=size, preserve=preserve, **kw)
if size is None or offset is None:
gap = gaps.largest_gap(device)
if size is None:
size = gap.size//2
if offset is None:
offset = gap.offset
partition = Partition(m=model, device=device, size=size, offset=offset,
preserve=preserve, **kw)
if preserve:
partition.number = len(device._partitions)
model._actions.append(partition)
@ -627,8 +632,10 @@ class TestAutoInstallConfig(unittest.TestCase):
def test_render_includes_all_partitions(self):
model = make_model(Bootloader.NONE)
disk1 = make_disk(model, preserve=True)
disk1p1 = make_partition(model, disk1, preserve=True)
disk1p2 = make_partition(model, disk1, preserve=True)
disk1p1 = make_partition(model, disk1, preserve=True,
offset=1 << 20, size=512 << 20)
disk1p2 = make_partition(model, disk1, preserve=True,
offset=513 << 20, size=8192 << 20)
fs = model.add_filesystem(disk1p2, 'ext4')
model.add_mount(fs, '/')
rendered_ids = {action['id'] for action in model._render_actions()}

View File

@ -142,14 +142,16 @@ class FilesystemController(SubiquityController, FilesystemManipulator):
self.reformat(disk)
if DeviceAction.TOGGLE_BOOT in DeviceAction.supported(disk):
self.add_boot_disk(disk)
gap = gaps.largest_gap(disk)
self.create_partition(
device=disk, spec=dict(
device=disk, gap=gap, spec=dict(
size=get_bootfs_size(disk),
fstype="ext4",
mount='/boot'
))
gap = gaps.largest_gap(disk)
part = self.create_partition(
device=disk, spec=dict(
device=disk, gap=gap, spec=dict(
size=gaps.largest_gap_size(disk),
fstype=None,
))
@ -353,21 +355,21 @@ class FilesystemController(SubiquityController, FilesystemManipulator):
if data.partition.boot is not None:
raise ValueError('add_partition does not support changing boot')
disk = self.model._one(id=data.disk_id)
largest_size = gaps.largest_gap_size(disk)
if largest_size == 0:
gap = gaps.largest_gap(disk)
if gap.size == 0:
raise ValueError('no space on disk')
requested_size = data.partition.size or 0
if requested_size > largest_size:
if requested_size > gap.size:
raise ValueError('new partition too large')
if requested_size < 1:
requested_size = largest_size
requested_size = gap.size
spec = {
'size': requested_size,
'fstype': data.partition.format,
'mount': data.partition.mount,
}
self.create_partition(disk, spec, '', 'superblock', None)
self.create_partition(disk, gap, spec, wipe='superblock')
return await self.v2_GET()
async def v2_delete_partition_POST(self, data: ModifyPartitionV2) \

View File

@ -328,8 +328,27 @@ class TestAdd(TestAPI):
# these manual steps are expected to be equivalent to just adding
# the single partition and getting the automatic boot partition
await inst.post('/storage/v2/add_boot_partition', disk_id=disk_id)
manual_add = await inst.post('/storage/v2/add_partition', data)
self.assertEqual(single_add, manual_add)
# manual_add =
await inst.post('/storage/v2/add_partition', data)
# FIXME single_add places /boot/efi physically second
# self.assertEqual(single_add, manual_add)
# manual behaves as expected
# /boot/efi
# offset: 1 MiB
# size: 538 MiB
# /
# offset: 539 MiB
# size: 9700 MiB
# single flips the order of the physical layout
# /boot/efi
# offset: 9701 MiB
# size: 538 MiB
# /
# offset: 1 MiB
# size: 9700 MiB
@timeout()
async def test_v2_deny_multiple_add_boot_partition(self):

View File

@ -13,6 +13,7 @@ from subiquity.models.filesystem import (
)
from subiquity.models.tests.test_filesystem import (
FakeStorageInfo,
make_model,
)
from subiquity.ui.views.filesystem.filesystem import FilesystemView
@ -23,14 +24,14 @@ class FilesystemViewTests(unittest.TestCase):
controller = mock.create_autospec(spec=FilesystemController)
controller.ui = mock.Mock()
model.bootloader = Bootloader.NONE
model.all_devices.return_value = devices
model.all_devices = mock.Mock(return_value=devices)
return FilesystemView(model, controller)
def test_simple(self):
self.make_view(mock.create_autospec(spec=FilesystemModel))
def test_one_disk(self):
model = mock.create_autospec(spec=FilesystemModel)
model = make_model()
model._probe_data = {}
model._actions = []
model._all_ids = set()

View File

@ -167,7 +167,7 @@ class PartitionViewTests(unittest.TestCase):
'size': "256M",
}
model, disk = make_model_and_disk()
partition = model.add_partition(disk, 512*(2**20), "boot")
partition = model.add_partition(disk, 512*(2**20), flag="boot")
fs = model.add_filesystem(partition, "fat32")
model.add_mount(fs, '/boot/efi')
view, stretchy = make_partition_view(model, disk, partition)
@ -190,7 +190,7 @@ class PartitionViewTests(unittest.TestCase):
def test_edit_existing_unused_boot_partition(self):
model, disk = make_model_and_disk()
partition = model.add_partition(disk, 512*(2**20), "boot")
partition = model.add_partition(disk, 512*(2**20), flag="boot")
fs = model.add_filesystem(partition, "fat32")
model._orig_config = model._render_actions()
disk.preserve = partition.preserve = fs.preserve = True
@ -211,7 +211,7 @@ class PartitionViewTests(unittest.TestCase):
def test_edit_existing_used_boot_partition(self):
model, disk = make_model_and_disk()
partition = model.add_partition(disk, 512*(2**20), "boot")
partition = model.add_partition(disk, 512*(2**20), flag="boot")
fs = model.add_filesystem(partition, "fat32")
model._orig_config = model._render_actions()
partition.grub_device = True