Skip to content

Commit

Permalink
Setup default minimum volume size per filesystem
Browse files Browse the repository at this point in the history
The former method provided a static value but there are huge
differences for the minimum size requirement of a filesystem.
For example extX is fine with 30MB whereas XFS requires 300MB.
This commit adds a more dynamic default value based on the
used filesystem.
  • Loading branch information
schaefi committed Aug 8, 2024
1 parent 47ebc8c commit ff7adbd
Show file tree
Hide file tree
Showing 10 changed files with 37 additions and 23 deletions.
10 changes: 8 additions & 2 deletions kiwi/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -1148,15 +1148,21 @@ def get_min_partition_mbytes():
return 10

@staticmethod
def get_min_volume_mbytes():
def get_min_volume_mbytes(filesystem: str):
"""
Provides default minimum LVM volume size in mbytes
per filesystem
:return: mbsize value
:rtype: int
"""
return 120
if filesystem == 'btrfs':
return 120
elif filesystem == 'xfs':
return 300
else:
return 30

@staticmethod
def get_lvm_overhead_mbytes():
Expand Down
8 changes: 5 additions & 3 deletions kiwi/storage/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,9 @@ def _accumulate_volume_size(self, root_mbytes):
# on first boot of the disk image in oemboot/repart
if self.disk_resize_requested:
for volume in self.volumes:
disk_volume_mbytes += Defaults.get_min_volume_mbytes()
disk_volume_mbytes += Defaults.get_min_volume_mbytes(
self.filesystem
)
return disk_volume_mbytes

# For static disk(no resize requested) we need to add the
Expand All @@ -378,7 +380,7 @@ def _accumulate_volume_size(self, root_mbytes):
data_volume_mbytes.volume[volume.realpath]
if disk_add_mbytes > 0:
disk_volume_mbytes += disk_add_mbytes + \
Defaults.get_min_volume_mbytes()
Defaults.get_min_volume_mbytes(self.filesystem)
else:
message = dedent('''\n
Requested volume size {0}MB for {1!r} is too small
Expand All @@ -402,7 +404,7 @@ def _accumulate_volume_size(self, root_mbytes):

if disk_add_mbytes > 0:
disk_volume_mbytes += disk_add_mbytes + \
Defaults.get_min_volume_mbytes()
Defaults.get_min_volume_mbytes(self.filesystem)
else:
log.warning(
'root volume size of %s MB is too small, skipped',
Expand Down
6 changes: 3 additions & 3 deletions kiwi/volume_manager/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ def get_volume_mbsize(
# Therefore the requested size is set to null and we add
# the required minimum size for just storing the data
size_type = 'freespace'
mbsize = Defaults.get_min_volume_mbytes()
mbsize = Defaults.get_min_volume_mbytes(filesystem_name)

if size_type == 'freespace' and os.path.exists(lookup_abspath):
exclude_paths = []
Expand All @@ -337,8 +337,8 @@ def get_volume_mbsize(
)

volume_size = SystemSize(lookup_abspath)
if mbsize != Defaults.get_min_volume_mbytes():
mbsize += Defaults.get_min_volume_mbytes()
if mbsize != Defaults.get_min_volume_mbytes(filesystem_name):
mbsize += Defaults.get_min_volume_mbytes(filesystem_name)
mbsize += volume_size.customize(
volume_size.accumulate_mbyte_file_sizes(exclude_paths),
filesystem_name
Expand Down
5 changes: 3 additions & 2 deletions kiwi/xml_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -1731,6 +1731,7 @@ def get_volumes(self) -> List[volume_type]:
"""
volume_type_list: List[volume_type] = []
systemdisk_section = self.get_build_type_system_disk_section()
selected_filesystem = self.build_type.get_filesystem()
swap_mbytes = self.get_oemconfig_swap_mbytes()
swap_name = self.get_oemconfig_swap_name()
if not systemdisk_section:
Expand Down Expand Up @@ -1802,7 +1803,7 @@ def get_volumes(self) -> List[volume_type]:
)
else:
size = 'freespace:' + format(
Defaults.get_min_volume_mbytes()
Defaults.get_min_volume_mbytes(selected_filesystem)
)

if ':all' in size:
Expand Down Expand Up @@ -1832,7 +1833,7 @@ def get_volumes(self) -> List[volume_type]:
defaults.ROOT_VOLUME_NAME if volume_management == 'lvm' else ''
if have_full_size_volume:
size = 'freespace:' + format(
Defaults.get_min_volume_mbytes()
Defaults.get_min_volume_mbytes(selected_filesystem)
)
fullsize = False
else:
Expand Down
5 changes: 5 additions & 0 deletions test/unit/defaults_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,3 +217,8 @@ def test_get_snapper_config_template_file(self, mock_os_path_exists):
mock_os_path_exists.return_value = True
assert Defaults.get_snapper_config_template_file('root') == \
'root/etc/snapper/config-templates/default'

def test_get_min_volume_mbytes(self):
assert Defaults.get_min_volume_mbytes('btrfs') == 120
assert Defaults.get_min_volume_mbytes('xfs') == 300
assert Defaults.get_min_volume_mbytes('some') == 30
4 changes: 2 additions & 2 deletions test/unit/storage/setup_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ def test_get_disksize_mbytes_partition_too_small(self, mock_os_path_exists):
@patch('os.path.exists')
def test_get_disksize_mbytes_volumes(self, mock_exists):
mock_exists.side_effect = lambda path: path != 'root_dir/newfolder'
assert self.setup_volumes.get_disksize_mbytes() == 2774
assert self.setup_volumes.get_disksize_mbytes() == 2144

@patch('os.path.exists')
def test_get_disksize_mbytes_partitions(self, mock_exists):
Expand Down Expand Up @@ -244,7 +244,7 @@ def test_get_disksize_mbytes_oem_volumes(self, mock_exists):
Defaults.get_default_efi_boot_mbytes() + \
Defaults.get_default_boot_mbytes() + \
root_size + \
5 * Defaults.get_min_volume_mbytes()
5 * Defaults.get_min_volume_mbytes('ext3')

@patch('os.path.exists')
def test_get_disksize_mbytes_root_volume(self, mock_exists):
Expand Down
4 changes: 2 additions & 2 deletions test/unit/system/profile_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ def test_create_oem(self, mock_which):
os.remove(self.profile_file)
assert self.profile.dot_profile == {
'kiwi_Volume_1': 'usr_lib|size:1024|usr/lib',
'kiwi_Volume_2': 'etc_volume|freespace:120|etc',
'kiwi_Volume_2': 'etc_volume|freespace:30|etc',
'kiwi_Volume_3': 'bin_volume|size:all|/usr/bin',
'kiwi_Volume_4': 'usr_bin|freespace:120|usr/bin',
'kiwi_Volume_4': 'usr_bin|freespace:30|usr/bin',
'kiwi_Volume_5': 'LVSwap|size:128|',
'kiwi_Volume_Root': 'LVRoot|freespace:500|',
'kiwi_bootkernel': None,
Expand Down
8 changes: 4 additions & 4 deletions test/unit/volume_manager/base_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ def test_get_volume_mbsize(
assert self.volume_manager.get_volume_mbsize(
self.volume_manager.volumes[0], self.volume_manager.volumes,
'ext3'
) == 362
) == 272

@patch('kiwi.volume_manager.base.SystemSize')
@patch('os.path.exists')
Expand All @@ -140,7 +140,7 @@ def test_get_volume_mbsize_for_oem_type(
assert self.volume_manager.get_volume_mbsize(
self.volume_manager.volumes[0], self.volume_manager.volumes,
'ext3', True
) == 162
) == 72

@patch('kiwi.volume_manager.base.SystemSize')
@patch('os.path.exists')
Expand Down Expand Up @@ -168,7 +168,7 @@ def test_get_volume_mbsize_nested_volumes(
assert self.volume_manager.get_volume_mbsize(
self.volume_manager.volumes[0], self.volume_manager.volumes,
'ext3'
) == 362
) == 272
size.accumulate_mbyte_file_sizes.assert_called_once_with(
['root_dir/usr/lib']
)
Expand Down Expand Up @@ -204,7 +204,7 @@ def test_get_volume_mbsize_root_volume(
assert self.volume_manager.get_volume_mbsize(
self.volume_manager.volumes[2], self.volume_manager.volumes,
'ext3', True
) == 162
) == 72
size.accumulate_mbyte_file_sizes.assert_called_once_with(
['root_dir/usr', 'root_dir/usr/lib']
)
Expand Down
4 changes: 2 additions & 2 deletions test/unit/volume_manager/lvm_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,8 @@ def mock_os_exists_return(path):
self.volume_manager.volume_group = 'volume_group'
self.volume_manager.create_volumes('ext3')
myvol_size = 500
etc_size = 200 + 42 + Defaults.get_min_volume_mbytes()
root_size = 100 + 42 + Defaults.get_min_volume_mbytes()
etc_size = 200 + 42 + Defaults.get_min_volume_mbytes('ext3')
root_size = 100 + 42 + Defaults.get_min_volume_mbytes('ext3')

assert mock_attrs.call_args_list == [
call(
Expand Down
6 changes: 3 additions & 3 deletions test/unit/xml_state_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ def test_get_volumes_for_arch(self):
volume_type(
name='usr_lib',
parent='',
size='freespace:120',
size='freespace:30',
realpath='usr/lib',
mountpoint='usr/lib',
fullsize=False,
Expand Down Expand Up @@ -493,7 +493,7 @@ def test_get_volumes(self):
is_root_volume=True
),
volume_type(
name='etc_volume', parent='', size='freespace:120',
name='etc_volume', parent='', size='freespace:30',
realpath='etc',
mountpoint='etc', fullsize=False,
label=None,
Expand Down Expand Up @@ -557,7 +557,7 @@ def test_get_volumes_no_explicit_root_setup_other_fullsize_volume(self):
is_root_volume=False
),
volume_type(
name='LVRoot', parent='', size='freespace:120', realpath='/',
name='LVRoot', parent='', size='freespace:30', realpath='/',
mountpoint=None, fullsize=False,
label=None,
attributes=[],
Expand Down

0 comments on commit ff7adbd

Please sign in to comment.