From 12d69183887a068510eb25ec148599fc9d427e43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20Sch=C3=A4fer?= Date: Wed, 12 Jul 2023 17:58:41 +0200 Subject: [PATCH 01/12] Evaluate the @root volume name also for btrfs In a volume setup the special volume declaration was only evaluated for the LVM volume manager. In case of btrfs a hardcoded root volume name '@' was used. This commit allows to specify a custom name for the root volume for btrfs as well and also allows to specify that there should be no such root volume. Example: Name the root volume '@'. If not specified this stays as the default to stay compatible Indicate no root volume is wanted. All subvolumes resides below root (/) Name the root volume 'foo' This is related to Issue #2316 and a first patch to address the requested changes --- kiwi/volume_manager/btrfs.py | 100 ++++++++++++++++++------- kiwi/xml_state.py | 5 +- test/unit/volume_manager/btrfs_test.py | 62 +++++++++++++-- 3 files changed, 133 insertions(+), 34 deletions(-) diff --git a/kiwi/volume_manager/btrfs.py b/kiwi/volume_manager/btrfs.py index 34c63a30172..abbfa214f32 100644 --- a/kiwi/volume_manager/btrfs.py +++ b/kiwi/volume_manager/btrfs.py @@ -74,6 +74,18 @@ def post_init(self, custom_args): if 'quota_groups' not in self.custom_args: self.custom_args['quota_groups'] = False + self.root_volume_name = '@' + canonical_volume_list = self.get_canonical_volume_list() + for volume in canonical_volume_list.volumes: + if volume.is_root_volume and volume.name: + self.root_volume_name = volume.name + + if self.custom_args['root_is_snapshot'] and \ + self.root_volume_name == '/': + log.warning('root_is_snapshot requires a toplevel sub-volume') + log.warning('root_is_snapshot has been disabled') + self.custom_args['root_is_snapshot'] = False + self.subvol_mount_list = [] self.toplevel_mount = None self.toplevel_volume = None @@ -82,9 +94,9 @@ def setup(self, name=None): """ Setup btrfs volume management - In case of btrfs a toplevel(@) subvolume is created and marked + In case of btrfs an optional toplevel subvolume is created and marked as default volume. If snapshots are activated via the custom_args - the setup method also created the @/.snapshots/1/snapshot + the setup method also creates the .snapshots/1/snapshot subvolumes. There is no concept of a volume manager name, thus the name argument is not used for btrfs @@ -112,31 +124,37 @@ def setup(self, name=None): Command.run( ['btrfs', 'quota', 'enable', self.mountpoint] ) - root_volume = self.mountpoint + '/@' - Command.run( - ['btrfs', 'subvolume', 'create', root_volume] - ) + if self.root_volume_name != '/': + root_volume = self.mountpoint + f'/{self.root_volume_name}' + Command.run( + ['btrfs', 'subvolume', 'create', root_volume] + ) if self.custom_args['root_is_snapshot']: - snapshot_volume = self.mountpoint + '/@/.snapshots' + snapshot_volume = self.mountpoint + \ + f'/{self.root_volume_name}/.snapshots' Command.run( ['btrfs', 'subvolume', 'create', snapshot_volume] ) os.chmod(snapshot_volume, 0o700) Path.create(snapshot_volume + '/1') - snapshot = self.mountpoint + '/@/.snapshots/1/snapshot' + snapshot = self.mountpoint + \ + f'/{self.root_volume_name}/.snapshots/1/snapshot' Command.run( ['btrfs', 'subvolume', 'create', snapshot] ) - self._set_default_volume('@/.snapshots/1/snapshot') - snapshot = self.mountpoint + '/@/.snapshots/1/snapshot' - # Mount /@/.snapshots as /.snapshots inside the root + self._set_default_volume( + f'{self.root_volume_name}/.snapshots/1/snapshot' + ) + snapshot = self.mountpoint + \ + f'/{self.root_volume_name}/.snapshots/1/snapshot' + # Mount /{some-name}/.snapshots as /.snapshots inside the root snapshots_mount = MountManager( device=self.device, mountpoint=snapshot + '/.snapshots' ) self.subvol_mount_list.append(snapshots_mount) - else: - self._set_default_volume('@') + elif self.root_volume_name != '/': + self._set_default_volume(self.root_volume_name) def create_volumes(self, filesystem_name): """ @@ -163,12 +181,12 @@ def create_volumes(self, filesystem_name): for volume in canonical_volume_list.volumes: if volume.is_root_volume: - # the btrfs root volume named '@' has been created as + # the btrfs root volume has been created as # part of the setup procedure pass else: log.info('--> sub volume %s', volume.realpath) - toplevel = self.mountpoint + '/@/' + toplevel = self.mountpoint + f'/{self.root_volume_name}/' volume_parent_path = os.path.normpath( toplevel + os.path.dirname(volume.realpath) ) @@ -183,15 +201,21 @@ def create_volumes(self, filesystem_name): self.apply_attributes_on_volume( toplevel, volume ) + volume_mountpoint = self.mountpoint + \ + self.root_volume_name + '/' if self.custom_args['root_is_snapshot']: - snapshot = self.mountpoint + '/@/.snapshots/1/snapshot/' - volume_mount = MountManager( - device=self.device, - mountpoint=os.path.normpath(snapshot + volume.realpath) - ) - self.subvol_mount_list.append( - volume_mount + volume_mountpoint = self.mountpoint + \ + f'/{self.root_volume_name}/.snapshots/1/snapshot/' + + volume_mount = MountManager( + device=self.device, + mountpoint=os.path.normpath( + volume_mountpoint + volume.realpath ) + ) + self.subvol_mount_list.append( + volume_mount + ) def get_fstab(self, persistency_type='by-label', filesystem_name=None): """ @@ -219,7 +243,10 @@ def get_fstab(self, persistency_type='by-label', filesystem_name=None): ) fstab_entry = ' '.join( [ - blkid_type + '=' + device_id, subvol_name.replace('@', ''), + blkid_type + '=' + device_id, + subvol_name.replace( + self.root_volume_name, '' + ) if self.root_volume_name != '/' else subvol_name, 'btrfs', ','.join(mount_entry_options), '0 {fs_passno}'.format( fs_passno='2' if fs_check else '0' @@ -245,7 +272,10 @@ def get_volumes(self): 'subvol=' + subvol_name ] + self.custom_filesystem_args['mount_options'] ) - volumes[subvol_name.replace('@', '')] = { + subvol_path = subvol_name.replace( + self.root_volume_name, '' + ) if self.root_volume_name != '/' else subvol_name + volumes[subvol_path] = { 'volume_options': subvol_options, 'volume_device': volume_mount.device } @@ -309,7 +339,9 @@ def get_mountpoint(self) -> str: :rtype: string """ - sync_target: List[str] = [self.mountpoint, '@'] + sync_target: List[str] = [self.mountpoint] + if self.root_volume_name != '/': + sync_target.append(self.root_volume_name) if self.custom_args.get('root_is_snapshot'): sync_target.extend(['.snapshots', '1', 'snapshot']) return os.path.join(*sync_target) @@ -327,7 +359,12 @@ def sync_data(self, exclude=None): sync_target = self.get_mountpoint() if self.custom_args['root_is_snapshot']: self._create_snapshot_info( - ''.join([self.mountpoint, '/@/.snapshots/1/info.xml']) + ''.join( + [ + self.mountpoint, + f'/{self.root_volume_name}/.snapshots/1/info.xml' + ] + ) ) data = DataSync(self.root_dir, sync_target) data.sync_data( @@ -389,7 +426,12 @@ def _xml_pretty(self, toplevel_element): return xml_data_domtree.toprettyxml(indent=" ") def _create_snapper_quota_configuration(self): - root_path = os.sep.join([self.mountpoint, '@/.snapshots/1/snapshot']) + root_path = os.sep.join( + [ + self.mountpoint, + f'{self.root_volume_name}/.snapshots/1/snapshot' + ] + ) snapper_default_conf = Defaults.get_snapper_config_template_file( root_path ) @@ -453,7 +495,9 @@ def _get_subvol_name_from_mountpoint(self, volume_mount): ) if self.toplevel_volume and self.toplevel_volume in subvol_name: subvol_name = subvol_name.replace(self.toplevel_volume, '') - return os.path.normpath(os.sep.join(['@', subvol_name])) + return os.path.normpath( + os.sep.join([self.root_volume_name, subvol_name]).replace('//', '/') + ) def __del__(self): if self.toplevel_mount: diff --git a/kiwi/xml_state.py b/kiwi/xml_state.py index d8bf4cfd939..5cce0004e0b 100644 --- a/kiwi/xml_state.py +++ b/kiwi/xml_state.py @@ -1680,6 +1680,9 @@ def get_volumes(self) -> List[volume_type]: if not have_root_volume_setup: # There must always be a root volume setup. It will be the # full size volume if no other volume has this setup + volume_management = self.get_volume_management() + root_volume_name = \ + defaults.ROOT_VOLUME_NAME if volume_management == 'lvm' else '' if have_full_size_volume: size = 'freespace:' + format( Defaults.get_min_volume_mbytes() @@ -1690,7 +1693,7 @@ def get_volumes(self) -> List[volume_type]: fullsize = True volume_type_list.append( volume_type( - name=defaults.ROOT_VOLUME_NAME, + name=root_volume_name, size=size, fullsize=fullsize, mountpoint=None, diff --git a/test/unit/volume_manager/btrfs_test.py b/test/unit/volume_manager/btrfs_test.py index d9c2fdba80b..8ccbdf40b53 100644 --- a/test/unit/volume_manager/btrfs_test.py +++ b/test/unit/volume_manager/btrfs_test.py @@ -27,12 +27,12 @@ def inject_fixtures(self, caplog): def setup(self, mock_path): self.volumes = [ volume_type( - name='LVRoot', size='freespace:100', realpath='/', + name='@', size='freespace:100', realpath='/', mountpoint=None, fullsize=False, label=None, attributes=[], is_root_volume=True ), volume_type( - name='LVetc', size='freespace:200', realpath='/etc', + name='etc', size='freespace:200', realpath='/etc', mountpoint='/etc', fullsize=False, label=None, attributes=[], is_root_volume=False ), @@ -42,7 +42,7 @@ def setup(self, mock_path): attributes=[], is_root_volume=False ), volume_type( - name='LVhome', size=None, realpath='/home', + name='home', size=None, realpath='/home', mountpoint='/home', fullsize=True, label=None, attributes=[], is_root_volume=False ) @@ -69,6 +69,17 @@ def test_post_init(self): self.volume_manager.post_init({'some-arg': 'some-val'}) assert self.volume_manager.custom_args['some-arg'] == 'some-val' + def test_post_init_root_is_snapshot_without_toplevel_volume(self): + self.volume_manager.volumes = [ + volume_type( + name='/', size='freespace:100', realpath='/', + mountpoint=None, fullsize=False, label=None, + attributes=[], is_root_volume=True + ) + ] + self.volume_manager.post_init({'root_is_snapshot': True}) + assert self.volume_manager.custom_args['root_is_snapshot'] is False + @patch('os.path.exists') @patch('kiwi.volume_manager.btrfs.Command.run') @patch('kiwi.volume_manager.btrfs.FileSystem.new') @@ -159,6 +170,47 @@ def test_setup_volume_id_not_detected( with raises(KiwiVolumeRootIDError): self.volume_manager.setup() + @patch('os.path.exists') + @patch('kiwi.volume_manager.btrfs.Command.run') + @patch('kiwi.volume_manager.btrfs.MountManager') + @patch('kiwi.volume_manager.btrfs.Path.create') + @patch('kiwi.volume_manager.base.VolumeManagerBase.apply_attributes_on_volume') + def test_create_volumes_no_toplevel_volume( + self, mock_attrs, mock_path, mock_mount, mock_command, mock_os_exists + ): + volume_mount = Mock() + mock_mount.return_value = volume_mount + self.volume_manager.mountpoint = 'tmpdir' + self.volume_manager.custom_args['root_is_snapshot'] = False + mock_os_exists.return_value = False + + self.volume_manager.root_volume_name = '/' + self.volume_manager.volumes = [ + volume_type( + name='/', size='freespace:100', realpath='/', + mountpoint=None, fullsize=False, label=None, + attributes=[], is_root_volume=True + ), + volume_type( + name='home', size=None, realpath='/home', + mountpoint='/home', fullsize=True, label=None, + attributes=[], is_root_volume=False + ) + ] + + self.volume_manager.create_volumes('btrfs') + + assert mock_path.call_args_list == [ + call('root_dir/home'), + call('tmpdir') + ] + mock_command.assert_called_once_with( + ['btrfs', 'subvolume', 'create', 'tmpdir/home'] + ) + mock_mount.assert_called_once_with( + device='/dev/storage', mountpoint='tmpdir/home' + ) + @patch('os.path.exists') @patch('kiwi.volume_manager.btrfs.Command.run') @patch('kiwi.volume_manager.btrfs.MountManager') @@ -186,7 +238,7 @@ def test_create_volumes( ), call( 'tmpdir/@/', volume_type( - name='LVetc', size='freespace:200', realpath='/etc', + name='etc', size='freespace:200', realpath='/etc', mountpoint='/etc', fullsize=False, label=None, attributes=[], is_root_volume=False @@ -194,7 +246,7 @@ def test_create_volumes( ), call( 'tmpdir/@/', volume_type( - name='LVhome', size=None, realpath='/home', + name='home', size=None, realpath='/home', mountpoint='/home', fullsize=True, label=None, attributes=[], is_root_volume=False From 1844e80fbfaff9406b761d1bbc6929da24ce3753 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20Sch=C3=A4fer?= Date: Sun, 16 Jul 2023 20:32:12 +0200 Subject: [PATCH 02/12] Add btrfs_create_toplevel_subvolume attribute Allow to explicitly select if a toplevel subvolume should be created or not. To avoid a behavior change, kiwi will create a toplevel based btrfs structure if this attribute is not specified. However, a deprecation message to inform about future behavior change will be printed. This is related to Issue #2316 --- doc/source/image_description/elements.rst | 14 ++++++++++ kiwi/builder/disk.py | 3 +++ kiwi/schema/kiwi.rnc | 12 +++++++++ kiwi/schema/kiwi.rng | 17 +++++++++++++ kiwi/volume_manager/btrfs.py | 31 +++++++++++++++++++---- kiwi/xml_parse.py | 17 ++++++++++++- test/unit/xml_state_test.py | 4 +++ 7 files changed, 92 insertions(+), 6 deletions(-) diff --git a/doc/source/image_description/elements.rst b/doc/source/image_description/elements.rst index 3079fcdf218..0ecc3914b3e 100644 --- a/doc/source/image_description/elements.rst +++ b/doc/source/image_description/elements.rst @@ -412,6 +412,20 @@ btrfs_quota_groups="true|false": Boolean parameter to activate filesystem quotas if the filesystem is `btrfs`. By default quotas are inactive. +btrfs_create_toplevel_subvolume="true|false": + Tell kiwi to nest all subvolumes below a toplevel volume. + The name of this toplevel volume is by default set to: `@` + The name of the toplevel volume can be changed via + a volume entry of the form + + .. code:: xml + + + + + + By default the creation of a toplevel volume is set to: `true` + btrfs_root_is_snapshot="true|false": Boolean parameter that tells {kiwi} to install the system into a btrfs snapshot. The snapshot layout is compatible with diff --git a/kiwi/builder/disk.py b/kiwi/builder/disk.py index b17be379d68..e0e4f24200a 100644 --- a/kiwi/builder/disk.py +++ b/kiwi/builder/disk.py @@ -417,6 +417,9 @@ def create_disk(self) -> Result: 'root_is_readonly_snapshot': self.xml_state.build_type. get_btrfs_root_is_readonly_snapshot(), + 'create_toplevel_subvolume': + self.xml_state.build_type. + get_btrfs_create_toplevel_subvolume(), 'quota_groups': self.xml_state.build_type.get_btrfs_quota_groups(), 'resize_on_boot': diff --git a/kiwi/schema/kiwi.rnc b/kiwi/schema/kiwi.rnc index d8432fd3d09..b9e380fa1c4 100644 --- a/kiwi/schema/kiwi.rnc +++ b/kiwi/schema/kiwi.rnc @@ -1433,6 +1433,17 @@ div { sch:param [ name = "attr" value = "btrfs_quota_groups" ] sch:param [ name = "types" value = "oem" ] ] + k.type.btrfs_create_toplevel_subvolume = + ## Tell kiwi to nest all subvolumes below a toplevel volume. + ## The name of this toplevel volume is by default set to: '@' + ## The name of the toplevel volume can be changed via + ## a volume entry of the form '' + ## By default the creation of a toplevel volume is set to: true + attribute btrfs_create_toplevel_subvolume { xsd:boolean } + >> sch:pattern [ id = "btrfs_create_toplevel_subvolume" is-a = "image_type" + sch:param [ name = "attr" value = "btrfs_create_toplevel_subvolume" ] + sch:param [ name = "types" value = "oem" ] + ] k.type.btrfs_root_is_snapshot.attribute = ## Tell kiwi to install the system into a btrfs snapshot ## The snapshot layout is compatible with the snapper management @@ -2218,6 +2229,7 @@ div { k.type.bootprofile.attribute? & k.type.btrfs_quota_groups.attribute? & k.type.btrfs_root_is_snapshot.attribute? & + k.type.btrfs_create_toplevel_subvolume? & k.type.btrfs_root_is_readonly_snapshot.attribute? & k.type.compressed.attribute? & k.type.devicepersistency.attribute? & diff --git a/kiwi/schema/kiwi.rng b/kiwi/schema/kiwi.rng index f29a55bb6db..fdd34d41fce 100644 --- a/kiwi/schema/kiwi.rng +++ b/kiwi/schema/kiwi.rng @@ -2093,6 +2093,20 @@ By default the quota system is inactive + + + Tell kiwi to nest all subvolumes below a toplevel volume. +The name of this toplevel volume is by default set to: '@' +The name of the toplevel volume can be changed via +a volume entry of the form '<volume name="@root=TOPLEVEL_NAME"/>' +By default the creation of a toplevel volume is set to: true + + + + + + + Tell kiwi to install the system into a btrfs snapshot @@ -3177,6 +3191,9 @@ kiwi-ng result bundle ... + + + diff --git a/kiwi/volume_manager/btrfs.py b/kiwi/volume_manager/btrfs.py index abbfa214f32..441e72e6b4e 100644 --- a/kiwi/volume_manager/btrfs.py +++ b/kiwi/volume_manager/btrfs.py @@ -71,14 +71,18 @@ def post_init(self, custom_args): self.custom_args['root_is_snapshot'] = False if 'root_is_readonly_snapshot' not in self.custom_args: self.custom_args['root_is_readonly_snapshot'] = False + if 'create_toplevel_subvolume' not in self.custom_args: + self.custom_args['create_toplevel_subvolume'] = None if 'quota_groups' not in self.custom_args: self.custom_args['quota_groups'] = False - self.root_volume_name = '@' - canonical_volume_list = self.get_canonical_volume_list() - for volume in canonical_volume_list.volumes: - if volume.is_root_volume and volume.name: - self.root_volume_name = volume.name + self.root_volume_name = '/' + if self._has_root_volume(): + self.root_volume_name = '@' + canonical_volume_list = self.get_canonical_volume_list() + for volume in canonical_volume_list.volumes: + if volume.is_root_volume and volume.name: + self.root_volume_name = volume.name if self.custom_args['root_is_snapshot'] and \ self.root_volume_name == '/': @@ -388,6 +392,23 @@ def set_property_readonly_root(self): ['btrfs', 'property', 'set', sync_target, 'ro', 'true'] ) + def _has_root_volume(self) -> bool: + has_root_volume = bool(self.custom_args['create_toplevel_subvolume']) + if self.custom_args['create_toplevel_subvolume'] is None: + # toplevel volume not explicitly configured, will + # be enabled by default but this is going to change + # in the future. Print a deprecation message to inform + # the user about a potential behavior change + log.warning("Implicitly creating a toplevel volume") + log.warning( + "--> Future versions of kiwi will not do this anymore" + ) + log.warning( + "--> Please specify btrfs_create_toplevel_subvolume true|false" + ) + has_root_volume = True + return has_root_volume + def _is_volume_enabled_for_fs_check(self, mountpoint): for volume in self.volumes: if volume.realpath in mountpoint: diff --git a/kiwi/xml_parse.py b/kiwi/xml_parse.py index 8b7b3af8092..ffeb57ce8e1 100644 --- a/kiwi/xml_parse.py +++ b/kiwi/xml_parse.py @@ -3048,7 +3048,7 @@ class type_(GeneratedsSuper): """The Image Type of the Logical Extend""" subclass = None superclass = None - def __init__(self, boot=None, bootfilesystem=None, firmware=None, bootkernel=None, bootpartition=None, bootpartsize=None, efipartsize=None, efifatimagesize=None, efiparttable=None, dosparttable_extended_layout=None, bootprofile=None, btrfs_quota_groups=None, btrfs_root_is_snapshot=None, btrfs_root_is_readonly_snapshot=None, compressed=None, devicepersistency=None, editbootconfig=None, editbootinstall=None, filesystem=None, flags=None, format=None, formatoptions=None, fsmountoptions=None, fscreateoptions=None, squashfscompression=None, gcelicense=None, hybridpersistent=None, hybridpersistent_filesystem=None, gpt_hybrid_mbr=None, force_mbr=None, initrd_system=None, image=None, metadata_path=None, installboot=None, install_continue_on_timeout=None, installprovidefailsafe=None, installiso=None, installstick=None, installpxe=None, mediacheck=None, kernelcmdline=None, luks=None, luks_version=None, luksOS=None, luks_randomize=None, luks_pbkdf=None, mdraid=None, overlayroot=None, overlayroot_write_partition=None, overlayroot_readonly_partsize=None, verity_blocks=None, embed_verity_metadata=None, standalone_integrity=None, embed_integrity_metadata=None, integrity_legacy_hmac=None, integrity_metadata_key_description=None, integrity_keyfile=None, primary=None, ramonly=None, rootfs_label=None, spare_part=None, spare_part_mountpoint=None, spare_part_fs=None, spare_part_fs_attributes=None, spare_part_is_last=None, target_blocksize=None, target_removable=None, selinux_policy=None, vga=None, vhdfixedtag=None, volid=None, wwid_wait_timeout=None, derived_from=None, delta_root=None, ensure_empty_tmpdirs=None, xen_server=None, publisher=None, disk_start_sector=None, root_clone=None, boot_clone=None, bundle_format=None, bootloader=None, containerconfig=None, machine=None, oemconfig=None, size=None, systemdisk=None, partitions=None, vagrantconfig=None, installmedia=None, luksformat=None): + def __init__(self, boot=None, bootfilesystem=None, firmware=None, bootkernel=None, bootpartition=None, bootpartsize=None, efipartsize=None, efifatimagesize=None, efiparttable=None, dosparttable_extended_layout=None, bootprofile=None, btrfs_quota_groups=None, btrfs_root_is_snapshot=None, btrfs_create_toplevel_subvolume=None, btrfs_root_is_readonly_snapshot=None, compressed=None, devicepersistency=None, editbootconfig=None, editbootinstall=None, filesystem=None, flags=None, format=None, formatoptions=None, fsmountoptions=None, fscreateoptions=None, squashfscompression=None, gcelicense=None, hybridpersistent=None, hybridpersistent_filesystem=None, gpt_hybrid_mbr=None, force_mbr=None, initrd_system=None, image=None, metadata_path=None, installboot=None, install_continue_on_timeout=None, installprovidefailsafe=None, installiso=None, installstick=None, installpxe=None, mediacheck=None, kernelcmdline=None, luks=None, luks_version=None, luksOS=None, luks_randomize=None, luks_pbkdf=None, mdraid=None, overlayroot=None, overlayroot_write_partition=None, overlayroot_readonly_partsize=None, verity_blocks=None, embed_verity_metadata=None, standalone_integrity=None, embed_integrity_metadata=None, integrity_legacy_hmac=None, integrity_metadata_key_description=None, integrity_keyfile=None, primary=None, ramonly=None, rootfs_label=None, spare_part=None, spare_part_mountpoint=None, spare_part_fs=None, spare_part_fs_attributes=None, spare_part_is_last=None, target_blocksize=None, target_removable=None, selinux_policy=None, vga=None, vhdfixedtag=None, volid=None, wwid_wait_timeout=None, derived_from=None, delta_root=None, ensure_empty_tmpdirs=None, xen_server=None, publisher=None, disk_start_sector=None, root_clone=None, boot_clone=None, bundle_format=None, bootloader=None, containerconfig=None, machine=None, oemconfig=None, size=None, systemdisk=None, partitions=None, vagrantconfig=None, installmedia=None, luksformat=None): self.original_tagname_ = None self.boot = _cast(None, boot) self.bootfilesystem = _cast(None, bootfilesystem) @@ -3063,6 +3063,7 @@ def __init__(self, boot=None, bootfilesystem=None, firmware=None, bootkernel=Non self.bootprofile = _cast(None, bootprofile) self.btrfs_quota_groups = _cast(bool, btrfs_quota_groups) self.btrfs_root_is_snapshot = _cast(bool, btrfs_root_is_snapshot) + self.btrfs_create_toplevel_subvolume = _cast(bool, btrfs_create_toplevel_subvolume) self.btrfs_root_is_readonly_snapshot = _cast(bool, btrfs_root_is_readonly_snapshot) self.compressed = _cast(bool, compressed) self.devicepersistency = _cast(None, devicepersistency) @@ -3258,6 +3259,8 @@ def get_btrfs_quota_groups(self): return self.btrfs_quota_groups def set_btrfs_quota_groups(self, btrfs_quota_groups): self.btrfs_quota_groups = btrfs_quota_groups def get_btrfs_root_is_snapshot(self): return self.btrfs_root_is_snapshot def set_btrfs_root_is_snapshot(self, btrfs_root_is_snapshot): self.btrfs_root_is_snapshot = btrfs_root_is_snapshot + def get_btrfs_create_toplevel_subvolume(self): return self.btrfs_create_toplevel_subvolume + def set_btrfs_create_toplevel_subvolume(self, btrfs_create_toplevel_subvolume): self.btrfs_create_toplevel_subvolume = btrfs_create_toplevel_subvolume def get_btrfs_root_is_readonly_snapshot(self): return self.btrfs_root_is_readonly_snapshot def set_btrfs_root_is_readonly_snapshot(self, btrfs_root_is_readonly_snapshot): self.btrfs_root_is_readonly_snapshot = btrfs_root_is_readonly_snapshot def get_compressed(self): return self.compressed @@ -3513,6 +3516,9 @@ def exportAttributes(self, outfile, level, already_processed, namespaceprefix_=' if self.btrfs_root_is_snapshot is not None and 'btrfs_root_is_snapshot' not in already_processed: already_processed.add('btrfs_root_is_snapshot') outfile.write(' btrfs_root_is_snapshot="%s"' % self.gds_format_boolean(self.btrfs_root_is_snapshot, input_name='btrfs_root_is_snapshot')) + if self.btrfs_create_toplevel_subvolume is not None and 'btrfs_create_toplevel_subvolume' not in already_processed: + already_processed.add('btrfs_create_toplevel_subvolume') + outfile.write(' btrfs_create_toplevel_subvolume="%s"' % self.gds_format_boolean(self.btrfs_create_toplevel_subvolume, input_name='btrfs_create_toplevel_subvolume')) if self.btrfs_root_is_readonly_snapshot is not None and 'btrfs_root_is_readonly_snapshot' not in already_processed: already_processed.add('btrfs_root_is_readonly_snapshot') outfile.write(' btrfs_root_is_readonly_snapshot="%s"' % self.gds_format_boolean(self.btrfs_root_is_readonly_snapshot, input_name='btrfs_root_is_readonly_snapshot')) @@ -3840,6 +3846,15 @@ def buildAttributes(self, node, attrs, already_processed): self.btrfs_root_is_snapshot = False else: raise_parse_error(node, 'Bad boolean attribute') + value = find_attr_value_('btrfs_create_toplevel_subvolume', node) + if value is not None and 'btrfs_create_toplevel_subvolume' not in already_processed: + already_processed.add('btrfs_create_toplevel_subvolume') + if value in ('true', '1'): + self.btrfs_create_toplevel_subvolume = True + elif value in ('false', '0'): + self.btrfs_create_toplevel_subvolume = False + else: + raise_parse_error(node, 'Bad boolean attribute') value = find_attr_value_('btrfs_root_is_readonly_snapshot', node) if value is not None and 'btrfs_root_is_readonly_snapshot' not in already_processed: already_processed.add('btrfs_root_is_readonly_snapshot') diff --git a/test/unit/xml_state_test.py b/test/unit/xml_state_test.py index bff18b00ce3..6b0e42bcbe3 100644 --- a/test/unit/xml_state_test.py +++ b/test/unit/xml_state_test.py @@ -1132,3 +1132,7 @@ def test_get_bootloader_options(self): assert state.get_bootloader_config_options() == [ '--joe', '-x' ] + + def test_get_btrfs_create_toplevel_subvolume(self): + assert self.state.build_type.get_btrfs_create_toplevel_subvolume() is \ + None From cc2ba15a8834bb3d374d59f979d4c575e5e67387 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20Sch=C3=A4fer?= Date: Tue, 18 Jul 2023 16:59:36 +0200 Subject: [PATCH 03/12] Add parent attribute to volume setup For the btrfs volume management, allow to put a volume into a specific parent volume. If not specified the volume is below the default volume This Fixes #2316 --- kiwi/mount_manager.py | 17 +++- kiwi/schema/kiwi.rnc | 8 ++ kiwi/schema/kiwi.rng | 12 +++ kiwi/volume_manager/base.py | 13 +++ kiwi/volume_manager/btrfs.py | 111 +++++++++++++++++-------- kiwi/xml_parse.py | 12 ++- kiwi/xml_state.py | 6 ++ test/unit/mount_manager_test.py | 3 + test/unit/volume_manager/base_test.py | 3 + test/unit/volume_manager/btrfs_test.py | 87 ++++++++++++++----- test/unit/volume_manager/lvm_test.py | 19 +++-- test/unit/xml_state_test.py | 26 +++--- 12 files changed, 235 insertions(+), 82 deletions(-) diff --git a/kiwi/mount_manager.py b/kiwi/mount_manager.py index 8923ae21467..91397fd6f12 100644 --- a/kiwi/mount_manager.py +++ b/kiwi/mount_manager.py @@ -19,7 +19,9 @@ import time import logging from textwrap import dedent -from typing import List +from typing import ( + List, Dict +) # project from kiwi.path import Path @@ -41,9 +43,14 @@ class MountManager: * :param string device: device node name * :param string mountpoint: mountpoint directory name + * :param dict attributes: optional attributes to store """ - def __init__(self, device: str, mountpoint: str = ''): + def __init__( + self, device: str, mountpoint: str = '', + attributes: Dict[str, str] = {} + ): self.device = device + self.attributes = attributes if not mountpoint: self.mountpoint_tempdir = Temporary( prefix='kiwi_mount_manager.' @@ -53,6 +60,12 @@ def __init__(self, device: str, mountpoint: str = ''): Path.create(mountpoint) self.mountpoint = mountpoint + def get_attributes(self) -> Dict[str, str]: + """ + Return attributes dict for this mount manager + """ + return self.attributes + def bind_mount(self) -> None: """ Bind mount the device to the mountpoint diff --git a/kiwi/schema/kiwi.rnc b/kiwi/schema/kiwi.rnc index b9e380fa1c4..fcae7c00ba3 100644 --- a/kiwi/schema/kiwi.rnc +++ b/kiwi/schema/kiwi.rnc @@ -2564,6 +2564,13 @@ div { ## not specified the name specifies a path which has to ## exist inside the root directory. attribute name { text } + k.volume.parent.attribute = + ## The name/path of the parent volume. + ## Evaluated only for the btrfs volume manager to allow + ## specifying the parent subvolume to nest this volume in. + ## If not specified the parent is always the volume set + ## as the default volume + attribute parent { text } k.volume.mountpoint.attribute = ## volume path. The mountpoint specifies a path which has to ## exist inside the root directory. @@ -2596,6 +2603,7 @@ div { k.volume.mountpoint.attribute? & k.volume.label.attribute? & k.volume.name.attribute & + k.volume.parent.attribute? & k.volume.size.attribute? k.volume = ## Specify which parts of the filesystem should be diff --git a/kiwi/schema/kiwi.rng b/kiwi/schema/kiwi.rng index fdd34d41fce..9d1d9b9e6dd 100644 --- a/kiwi/schema/kiwi.rng +++ b/kiwi/schema/kiwi.rng @@ -3851,6 +3851,15 @@ not specified the name specifies a path which has to exist inside the root directory. + + + The name/path of the parent volume. +Evaluated only for the btrfs volume manager to allow +specifying the parent subvolume to nest this volume in. +If not specified the parent is always the volume set +as the default volume + + volume path. The mountpoint specifies a path which has to @@ -3907,6 +3916,9 @@ The latter is the default. + + + diff --git a/kiwi/volume_manager/base.py b/kiwi/volume_manager/base.py index a3783385dbe..71f226e9c59 100644 --- a/kiwi/volume_manager/base.py +++ b/kiwi/volume_manager/base.py @@ -331,6 +331,19 @@ def get_mountpoint(self): """ return self.mountpoint + def get_root_volume_name(self) -> str: + """ + Provides name of the root volume + + This is by default set to '/'. Volume Managers that supports + the concept of sub-volumes overrides this method + + :return: directory path name + + :rtype: string + """ + return '/' + def sync_data(self, exclude=None): """ Implements sync of root directory to mounted volumes diff --git a/kiwi/volume_manager/btrfs.py b/kiwi/volume_manager/btrfs.py index 441e72e6b4e..04489f6cd3e 100644 --- a/kiwi/volume_manager/btrfs.py +++ b/kiwi/volume_manager/btrfs.py @@ -25,8 +25,6 @@ from typing import List # project -import kiwi.defaults as defaults - from kiwi.command import Command from kiwi.volume_manager.base import VolumeManagerBase from kiwi.mount_manager import MountManager @@ -92,7 +90,6 @@ def post_init(self, custom_args): self.subvol_mount_list = [] self.toplevel_mount = None - self.toplevel_volume = None def setup(self, name=None): """ @@ -154,12 +151,26 @@ def setup(self, name=None): # Mount /{some-name}/.snapshots as /.snapshots inside the root snapshots_mount = MountManager( device=self.device, + attributes={ + 'subvol_path': f'{self.root_volume_name}/.snapshots', + 'subvol_name': f'{self.root_volume_name}/.snapshots' + }, mountpoint=snapshot + '/.snapshots' ) self.subvol_mount_list.append(snapshots_mount) elif self.root_volume_name != '/': self._set_default_volume(self.root_volume_name) + def get_root_volume_name(self) -> str: + """ + Provides name of the root volume + + :return: directory path name + + :rtype: string + """ + return self.root_volume_name + def create_volumes(self, filesystem_name): """ Create configured btrfs subvolumes @@ -190,31 +201,57 @@ def create_volumes(self, filesystem_name): pass else: log.info('--> sub volume %s', volume.realpath) - toplevel = self.mountpoint + f'/{self.root_volume_name}/' - volume_parent_path = os.path.normpath( - toplevel + os.path.dirname(volume.realpath) + toplevel = os.path.normpath( + self.mountpoint + os.sep + self.root_volume_name + ) + if volume.parent: + toplevel = os.path.normpath( + self.mountpoint + os.sep + volume.parent + ) + + Path.create( + os.path.dirname( + os.path.normpath(toplevel + os.sep + volume.realpath) + ) ) - if not os.path.exists(volume_parent_path): - Path.create(volume_parent_path) Command.run( [ 'btrfs', 'subvolume', 'create', - os.path.normpath(toplevel + volume.realpath) + os.path.normpath(toplevel + os.sep + volume.realpath) ] ) self.apply_attributes_on_volume( toplevel, volume ) - volume_mountpoint = self.mountpoint + \ - self.root_volume_name + '/' + + volume_mountpoint = toplevel + + attributes = { + 'parent': volume.parent or '', + 'subvol_path': os.path.normpath( + toplevel.replace( + self.mountpoint, '' + ) + os.sep + volume.realpath + ).lstrip(os.sep), + 'subvol_name': volume.name + } if self.custom_args['root_is_snapshot']: volume_mountpoint = self.mountpoint + \ f'/{self.root_volume_name}/.snapshots/1/snapshot/' + attributes = { + 'subvol_path': os.path.normpath( + self.root_volume_name + os.sep + volume.realpath + ), + 'subvol_name': os.path.normpath( + self.root_volume_name + os.sep + volume.realpath + ) + } volume_mount = MountManager( device=self.device, + attributes=attributes, mountpoint=os.path.normpath( - volume_mountpoint + volume.realpath + volume_mountpoint + os.sep + volume.realpath ) ) self.subvol_mount_list.append( @@ -240,17 +277,28 @@ def get_fstab(self, persistency_type='by-label', filesystem_name=None): blkid_type = 'LABEL' if persistency_type == 'by-label' else 'UUID' device_id = block_operation.get_blkid(blkid_type) for volume_mount in self.subvol_mount_list: - subvol_name = self._get_subvol_name_from_mountpoint(volume_mount) - mount_entry_options = mount_options + ['subvol=' + subvol_name] + mount_point = volume_mount.get_attributes().get('subvol_path') + + # Delete root_volume_name from mountpoint path if present + if self.root_volume_name != '/' and \ + mount_point.startswith(self.root_volume_name): + mount_point = mount_point.replace(self.root_volume_name, '') + + mount_entry_options = mount_options + [ + 'subvol=' + volume_mount.get_attributes().get( + 'subvol_path' + ).lstrip(os.sep) + ] + fs_check = self._is_volume_enabled_for_fs_check( volume_mount.mountpoint ) fstab_entry = ' '.join( [ blkid_type + '=' + device_id, - subvol_name.replace( - self.root_volume_name, '' - ) if self.root_volume_name != '/' else subvol_name, + mount_point if mount_point.startswith( + os.sep + ) else f'{os.sep}{mount_point}', 'btrfs', ','.join(mount_entry_options), '0 {fs_passno}'.format( fs_passno='2' if fs_check else '0' @@ -270,15 +318,15 @@ def get_volumes(self): """ volumes = {} for volume_mount in self.subvol_mount_list: - subvol_name = self._get_subvol_name_from_mountpoint(volume_mount) + subvol_path = volume_mount.get_attributes().get('subvol_path') subvol_options = ','.join( [ - 'subvol=' + subvol_name + 'subvol=' + subvol_path ] + self.custom_filesystem_args['mount_options'] ) - subvol_path = subvol_name.replace( + subvol_path = subvol_path.replace( self.root_volume_name, '' - ) if self.root_volume_name != '/' else subvol_name + ) if self.root_volume_name != '/' else subvol_path volumes[subvol_path] = { 'volume_options': subvol_options, 'volume_device': volume_mount.device @@ -295,15 +343,18 @@ def mount_volumes(self): for volume_mount in self.subvol_mount_list: if self.volumes_mounted_initially: + toplevel = self.root_volume_name + if self.custom_args['root_is_snapshot']: + toplevel = f'{self.root_volume_name}/.snapshots/1/snapshot' volume_mount.mountpoint = os.path.normpath( - volume_mount.mountpoint.replace(self.toplevel_volume, '', 1) + volume_mount.mountpoint.replace(toplevel, '', 1) ) if not os.path.exists(volume_mount.mountpoint): Path.create(volume_mount.mountpoint) - subvol_name = self._get_subvol_name_from_mountpoint(volume_mount) + subvol_path = volume_mount.get_attributes().get('subvol_path') subvol_options = ','.join( [ - 'subvol=' + subvol_name + 'subvol=' + subvol_path ] + self.custom_filesystem_args['mount_options'] ) volume_mount.mount( @@ -432,7 +483,6 @@ def _set_default_volume(self, default_volume): volume_id, self.mountpoint ] ) - self.toplevel_volume = default_volume return raise KiwiVolumeRootIDError( @@ -509,17 +559,6 @@ def _create_snapshot_info(self, filename): with open(filename, 'w') as snapshot_info_file: snapshot_info_file.write(self._xml_pretty(snapshot)) - def _get_subvol_name_from_mountpoint(self, volume_mount): - path_start_index = len(defaults.TEMP_DIR.split(os.sep)) + 1 - subvol_name = os.sep.join( - volume_mount.mountpoint.split(os.sep)[path_start_index:] - ) - if self.toplevel_volume and self.toplevel_volume in subvol_name: - subvol_name = subvol_name.replace(self.toplevel_volume, '') - return os.path.normpath( - os.sep.join([self.root_volume_name, subvol_name]).replace('//', '/') - ) - def __del__(self): if self.toplevel_mount: log.info('Cleaning up %s instance', type(self).__name__) diff --git a/kiwi/xml_parse.py b/kiwi/xml_parse.py index ffeb57ce8e1..693f1ec452e 100644 --- a/kiwi/xml_parse.py +++ b/kiwi/xml_parse.py @@ -4984,7 +4984,7 @@ class volume(GeneratedsSuper): """Specify which parts of the filesystem should be on an extra volume.""" subclass = None superclass = None - def __init__(self, copy_on_write=None, filesystem_check=None, freespace=None, mountpoint=None, label=None, name=None, size=None): + def __init__(self, copy_on_write=None, filesystem_check=None, freespace=None, mountpoint=None, label=None, name=None, parent=None, size=None): self.original_tagname_ = None self.copy_on_write = _cast(bool, copy_on_write) self.filesystem_check = _cast(bool, filesystem_check) @@ -4992,6 +4992,7 @@ def __init__(self, copy_on_write=None, filesystem_check=None, freespace=None, mo self.mountpoint = _cast(None, mountpoint) self.label = _cast(None, label) self.name = _cast(None, name) + self.parent = _cast(None, parent) self.size = _cast(None, size) def factory(*args_, **kwargs_): if CurrentSubclassModule_ is not None: @@ -5016,6 +5017,8 @@ def get_label(self): return self.label def set_label(self, label): self.label = label def get_name(self): return self.name def set_name(self, name): self.name = name + def get_parent(self): return self.parent + def set_parent(self, parent): self.parent = parent def get_size(self): return self.size def set_size(self, size): self.size = size def validate_volume_size_type(self, value): @@ -5071,6 +5074,9 @@ def exportAttributes(self, outfile, level, already_processed, namespaceprefix_=' if self.name is not None and 'name' not in already_processed: already_processed.add('name') outfile.write(' name=%s' % (self.gds_encode(self.gds_format_string(quote_attrib(self.name), input_name='name')), )) + if self.parent is not None and 'parent' not in already_processed: + already_processed.add('parent') + outfile.write(' parent=%s' % (self.gds_encode(self.gds_format_string(quote_attrib(self.parent), input_name='parent')), )) if self.size is not None and 'size' not in already_processed: already_processed.add('size') outfile.write(' size=%s' % (quote_attrib(self.size), )) @@ -5120,6 +5126,10 @@ def buildAttributes(self, node, attrs, already_processed): if value is not None and 'name' not in already_processed: already_processed.add('name') self.name = value + value = find_attr_value_('parent', node) + if value is not None and 'parent' not in already_processed: + already_processed.add('parent') + self.parent = value value = find_attr_value_('size', node) if value is not None and 'size' not in already_processed: already_processed.add('size') diff --git a/kiwi/xml_state.py b/kiwi/xml_state.py index 5cce0004e0b..1f640a24ea6 100644 --- a/kiwi/xml_state.py +++ b/kiwi/xml_state.py @@ -67,6 +67,7 @@ volume_type = NamedTuple( 'volume_type', [ ('name', str), + ('parent', str), ('size', str), ('realpath', str), ('mountpoint', Optional[str]), @@ -1574,6 +1575,7 @@ def get_volumes(self) -> List[volume_type]: [ volume_type( name=volume_name, + parent=volume_parent, size=volume_size, realpath=path, mountpoint=path, @@ -1600,6 +1602,7 @@ def get_volumes(self) -> List[volume_type]: # volume setup for a full qualified volume with name and # mountpoint information. See below for exceptions name = volume.get_name() + parent = volume.get_parent() or '' mountpoint = volume.get_mountpoint() realpath = mountpoint size = volume.get_size() @@ -1667,6 +1670,7 @@ def get_volumes(self) -> List[volume_type]: volume_type_list.append( volume_type( name=name, + parent=parent, size=size, fullsize=fullsize, mountpoint=mountpoint, @@ -1694,6 +1698,7 @@ def get_volumes(self) -> List[volume_type]: volume_type_list.append( volume_type( name=root_volume_name, + parent='', size=size, fullsize=fullsize, mountpoint=None, @@ -1708,6 +1713,7 @@ def get_volumes(self) -> List[volume_type]: volume_type_list.append( volume_type( name=swap_name, + parent='', size='size:{0}'.format(swap_mbytes), fullsize=False, mountpoint=None, diff --git a/test/unit/mount_manager_test.py b/test/unit/mount_manager_test.py index c192c14bd30..f976f27d97a 100644 --- a/test/unit/mount_manager_test.py +++ b/test/unit/mount_manager_test.py @@ -25,6 +25,9 @@ def setup(self, mock_path_create): def setup_method(self, cls, mock_path_create): self.setup() + def test_get_attributes(self): + assert self.mount_manager.get_attributes() == {} + @patch('kiwi.mount_manager.Temporary') def test_setup_empty_mountpoint(self, mock_Temporary): mock_Temporary.return_value.new_dir.return_value.name = 'tmpdir' diff --git a/test/unit/volume_manager/base_test.py b/test/unit/volume_manager/base_test.py index ab7d6fbf0f7..aaba787a34f 100644 --- a/test/unit/volume_manager/base_test.py +++ b/test/unit/volume_manager/base_test.py @@ -53,6 +53,9 @@ def setup(self, mock_path): def setup_method(self, cls, mock_path): self.setup() + def test_get_root_volume_name(self): + assert self.volume_manager.get_root_volume_name() == '/' + @patch('os.path.exists') def test_init_custom_args(self, mock_exists): mock_exists.return_value = True diff --git a/test/unit/volume_manager/btrfs_test.py b/test/unit/volume_manager/btrfs_test.py index 8ccbdf40b53..f0f48d70089 100644 --- a/test/unit/volume_manager/btrfs_test.py +++ b/test/unit/volume_manager/btrfs_test.py @@ -27,22 +27,22 @@ def inject_fixtures(self, caplog): def setup(self, mock_path): self.volumes = [ volume_type( - name='@', size='freespace:100', realpath='/', + name='@', parent='', size='freespace:100', realpath='/', mountpoint=None, fullsize=False, label=None, attributes=[], is_root_volume=True ), volume_type( - name='etc', size='freespace:200', realpath='/etc', + name='etc', parent='', size='freespace:200', realpath='/etc', mountpoint='/etc', fullsize=False, label=None, attributes=[], is_root_volume=False ), volume_type( - name='myvol', size='size:500', realpath='/data', + name='myvol', parent='', size='size:500', realpath='/data', mountpoint='LVdata', fullsize=False, label=None, attributes=[], is_root_volume=False ), volume_type( - name='home', size=None, realpath='/home', + name='home', parent='', size=None, realpath='/home', mountpoint='/home', fullsize=True, label=None, attributes=[], is_root_volume=False ) @@ -60,19 +60,23 @@ def setup(self, mock_path): self.volume_manager = VolumeManagerBtrfs( self.device_map, 'root_dir', self.volumes ) + self.volume_manager.mountpoint = '/var/tmp/kiwi_volumes.XXX' @patch('os.path.exists') def setup_method(self, cls, mock_path): self.setup() + def test_get_root_volume_name(self): + assert self.volume_manager.get_root_volume_name() == '@' + def test_post_init(self): self.volume_manager.post_init({'some-arg': 'some-val'}) assert self.volume_manager.custom_args['some-arg'] == 'some-val' - def test_post_init_root_is_snapshot_without_toplevel_volume(self): + def test_post_init_root_is_snapshot_without_root_volume(self): self.volume_manager.volumes = [ volume_type( - name='/', size='freespace:100', realpath='/', + name='/', parent='', size='freespace:100', realpath='/', mountpoint=None, fullsize=False, label=None, attributes=[], is_root_volume=True ) @@ -138,7 +142,14 @@ def test_setup_with_snapshot( assert mock_mount.call_args_list == [ call(device='/dev/storage', mountpoint='tmpdir'), - call(device='/dev/storage', mountpoint='tmpdir/@/.snapshots/1/snapshot/.snapshots') + call( + device='/dev/storage', + attributes={ + 'subvol_path': '@/.snapshots', + 'subvol_name': '@/.snapshots' + }, + mountpoint='tmpdir/@/.snapshots/1/snapshot/.snapshots' + ) ] toplevel_mount.mount.assert_called_once_with([]) assert mock_command.call_args_list == [ @@ -175,7 +186,7 @@ def test_setup_volume_id_not_detected( @patch('kiwi.volume_manager.btrfs.MountManager') @patch('kiwi.volume_manager.btrfs.Path.create') @patch('kiwi.volume_manager.base.VolumeManagerBase.apply_attributes_on_volume') - def test_create_volumes_no_toplevel_volume( + def test_create_volumes_no_root_volume( self, mock_attrs, mock_path, mock_mount, mock_command, mock_os_exists ): volume_mount = Mock() @@ -187,12 +198,12 @@ def test_create_volumes_no_toplevel_volume( self.volume_manager.root_volume_name = '/' self.volume_manager.volumes = [ volume_type( - name='/', size='freespace:100', realpath='/', + name='/', parent='', size='freespace:100', realpath='/', mountpoint=None, fullsize=False, label=None, attributes=[], is_root_volume=True ), volume_type( - name='home', size=None, realpath='/home', + name='home', parent='/', size=None, realpath='/home', mountpoint='/home', fullsize=True, label=None, attributes=[], is_root_volume=False ) @@ -208,7 +219,13 @@ def test_create_volumes_no_toplevel_volume( ['btrfs', 'subvolume', 'create', 'tmpdir/home'] ) mock_mount.assert_called_once_with( - device='/dev/storage', mountpoint='tmpdir/home' + device='/dev/storage', + attributes={ + 'parent': '/', + 'subvol_path': 'home', + 'subvol_name': 'home' + }, + mountpoint='tmpdir/home' ) @patch('os.path.exists') @@ -229,24 +246,24 @@ def test_create_volumes( assert mock_attrs.call_args_list == [ call( - 'tmpdir/@/', volume_type( - name='myvol', size='size:500', realpath='/data', + 'tmpdir/@', volume_type( + name='myvol', parent='', size='size:500', realpath='/data', mountpoint='LVdata', fullsize=False, label=None, attributes=[], is_root_volume=False ) ), call( - 'tmpdir/@/', volume_type( - name='etc', size='freespace:200', realpath='/etc', + 'tmpdir/@', volume_type( + name='etc', parent='', size='freespace:200', realpath='/etc', mountpoint='/etc', fullsize=False, label=None, attributes=[], is_root_volume=False ) ), call( - 'tmpdir/@/', volume_type( - name='home', size=None, realpath='/home', + 'tmpdir/@', volume_type( + name='home', parent='', size=None, realpath='/home', mountpoint='/home', fullsize=True, label=None, attributes=[], is_root_volume=False @@ -269,15 +286,27 @@ def test_create_volumes( assert mock_mount.call_args_list == [ call( device='/dev/storage', - mountpoint='tmpdir/@/.snapshots/1/snapshot/data' + mountpoint='tmpdir/@/.snapshots/1/snapshot/data', + attributes={ + 'subvol_path': '@/data', + 'subvol_name': '@/data' + } ), call( device='/dev/storage', - mountpoint='tmpdir/@/.snapshots/1/snapshot/etc' + mountpoint='tmpdir/@/.snapshots/1/snapshot/etc', + attributes={ + 'subvol_path': '@/etc', + 'subvol_name': '@/etc' + } ), call( device='/dev/storage', - mountpoint='tmpdir/@/.snapshots/1/snapshot/home' + mountpoint='tmpdir/@/.snapshots/1/snapshot/home', + attributes={ + 'subvol_path': '@/home', + 'subvol_name': '@/home' + } ) ] @@ -286,7 +315,9 @@ def test_get_volumes(self): volume_mount.mountpoint = \ '/var/tmp/kiwi_volumes.xx/@/.snapshots/1/snapshot/boot/grub2' volume_mount.device = 'device' - self.volume_manager.toplevel_volume = '@/.snapshots/1/snapshot' + volume_mount.get_attributes.return_value = { + 'subvol_path': '@/boot/grub2' + } self.volume_manager.subvol_mount_list = [volume_mount] self.volume_manager.custom_args['root_is_snapshot'] = True assert self.volume_manager.get_volumes() == { @@ -305,14 +336,19 @@ def test_get_fstab(self, mock_command): volume_mount.mountpoint = \ '/var/tmp/kiwi_volumes.XXX/@/.snapshots/1/snapshot/var/tmp' volume_mount.device = 'device' - self.volume_manager.toplevel_volume = '@/.snapshots/1/snapshot' + volume_mount.get_attributes.return_value = { + 'subvol_path': '@/var/tmp', + 'parent': 'subvol_takes_precedence' + } self.volume_manager.subvol_mount_list = [volume_mount] + self.volume_manager.custom_args['root_is_snapshot'] = True assert self.volume_manager.get_fstab() == [ 'LABEL=id /var/tmp btrfs defaults,subvol=@/var/tmp 0 0' ] self.volumes.append( volume_type( name='device', + parent='', size='freespace:100', realpath='/var/tmp', mountpoint=volume_mount.mountpoint, @@ -337,7 +373,9 @@ def test_mount_volumes(self, mock_path, mock_os_exists): volume_mount = Mock() volume_mount.mountpoint = \ '/var/tmp/kiwi_volumes.xx/@/.snapshots/1/snapshot/var/tmp' - self.volume_manager.toplevel_volume = '@/.snapshots/1/snapshot' + volume_mount.get_attributes.return_value = { + 'subvol_path': '@/var/tmp' + } self.volume_manager.custom_args['root_is_snapshot'] = True self.volume_manager.subvol_mount_list = [volume_mount] @@ -383,6 +421,9 @@ def test_remount_volumes( volume_mount = Mock() volume_mount.mountpoint = \ '/var/tmp/kiwi_volumes.xx/@/.snapshots/1/snapshot/var/tmp' + volume_mount.get_attributes.return_value = { + 'subvol_path': '@/var/tmp' + } self.volume_manager.subvol_mount_list = [volume_mount] self.volume_manager.mount_volumes() diff --git a/test/unit/volume_manager/lvm_test.py b/test/unit/volume_manager/lvm_test.py index 56168b77189..588339a82ad 100644 --- a/test/unit/volume_manager/lvm_test.py +++ b/test/unit/volume_manager/lvm_test.py @@ -22,27 +22,27 @@ def inject_fixtures(self, caplog): def setup(self, mock_path): self.volumes = [ volume_type( - name='LVRoot', size='freespace:100', realpath='/', + name='LVRoot', parent='', size='freespace:100', realpath='/', mountpoint=None, fullsize=False, label=None, attributes=[], is_root_volume=True ), volume_type( - name='LVSwap', size='size:100', realpath='swap', + name='LVSwap', parent='', size='size:100', realpath='swap', mountpoint=None, fullsize=False, label='SWAP', attributes=[], is_root_volume=False ), volume_type( - name='LVetc', size='freespace:200', realpath='/etc', + name='LVetc', parent='', size='freespace:200', realpath='/etc', mountpoint='/etc', fullsize=False, label='etc', attributes=[], is_root_volume=False ), volume_type( - name='myvol', size='size:500', realpath='/data', + name='myvol', parent='', size='size:500', realpath='/data', mountpoint='LVdata', fullsize=False, label=None, attributes=[], is_root_volume=False ), volume_type( - name='LVhome', size=None, realpath='/home', + name='LVhome', parent='', size=None, realpath='/home', mountpoint='/home', fullsize=True, label=None, attributes=[], is_root_volume=False ), @@ -178,28 +178,28 @@ def mock_os_exists_return(path): assert mock_attrs.call_args_list == [ call( 'root_dir', volume_type( - name='LVSwap', size='size:100', realpath='swap', + name='LVSwap', parent='', size='size:100', realpath='swap', mountpoint=None, fullsize=False, label='SWAP', attributes=[], is_root_volume=False ) ), call( 'root_dir', volume_type( - name='LVRoot', size='freespace:100', realpath='/', + name='LVRoot', parent='', size='freespace:100', realpath='/', mountpoint=None, fullsize=False, label=None, attributes=[], is_root_volume=True ) ), call( 'root_dir', volume_type( - name='myvol', size='size:500', realpath='/data', + name='myvol', parent='', size='size:500', realpath='/data', mountpoint='LVdata', fullsize=False, label=None, attributes=[], is_root_volume=False ) ), call( 'root_dir', volume_type( - name='LVetc', size='freespace:200', realpath='/etc', + name='LVetc', parent='', size='freespace:200', realpath='/etc', mountpoint='/etc', fullsize=False, label='etc', attributes=[], is_root_volume=False ) @@ -342,6 +342,7 @@ def test_get_fstab(self): self.volumes.append( volume_type( name='device', + parent='', size='freespace:100', realpath='/var/tmp', mountpoint=volume_mount.mountpoint, diff --git a/test/unit/xml_state_test.py b/test/unit/xml_state_test.py index 6b0e42bcbe3..dbc7fa0b574 100644 --- a/test/unit/xml_state_test.py +++ b/test/unit/xml_state_test.py @@ -386,6 +386,7 @@ def test_get_volumes_custom_root_volume_name(self): volume_type = namedtuple( 'volume_type', [ 'name', + 'parent', 'size', 'realpath', 'mountpoint', @@ -397,7 +398,7 @@ def test_get_volumes_custom_root_volume_name(self): ) assert state.get_volumes() == [ volume_type( - name='myroot', size='freespace:500', + name='myroot', parent='', size='freespace:500', realpath='/', mountpoint=None, fullsize=False, label=None, @@ -413,6 +414,7 @@ def test_get_volumes(self): volume_type = namedtuple( 'volume_type', [ 'name', + 'parent', 'size', 'realpath', 'mountpoint', @@ -424,7 +426,7 @@ def test_get_volumes(self): ) assert state.get_volumes() == [ volume_type( - name='usr_lib', size='size:1024', + name='usr_lib', parent='', size='size:1024', realpath='usr/lib', mountpoint='usr/lib', fullsize=False, @@ -433,7 +435,7 @@ def test_get_volumes(self): is_root_volume=False ), volume_type( - name='LVRoot', size='freespace:500', + name='LVRoot', parent='', size='freespace:500', realpath='/', mountpoint=None, fullsize=False, label=None, @@ -441,7 +443,7 @@ def test_get_volumes(self): is_root_volume=True ), volume_type( - name='etc_volume', size='freespace:30', + name='etc_volume', parent='', size='freespace:30', realpath='etc', mountpoint='etc', fullsize=False, label=None, @@ -449,7 +451,7 @@ def test_get_volumes(self): is_root_volume=False ), volume_type( - name='bin_volume', size=None, + name='bin_volume', parent='', size=None, realpath='/usr/bin', mountpoint='/usr/bin', fullsize=True, label=None, @@ -457,7 +459,7 @@ def test_get_volumes(self): is_root_volume=False ), volume_type( - name='LVSwap', size='size:128', + name='LVSwap', parent='', size='size:128', realpath='swap', mountpoint=None, fullsize=False, label='SWAP', @@ -473,6 +475,7 @@ def test_get_volumes_no_explicit_root_setup(self): volume_type = namedtuple( 'volume_type', [ 'name', + 'parent', 'size', 'realpath', 'mountpoint', @@ -484,14 +487,14 @@ def test_get_volumes_no_explicit_root_setup(self): ) assert state.get_volumes() == [ volume_type( - name='LVRoot', size=None, realpath='/', + name='LVRoot', parent='', size=None, realpath='/', mountpoint=None, fullsize=True, label=None, attributes=[], is_root_volume=True ), volume_type( - name='LVSwap', size='size:128', + name='LVSwap', parent='', size='size:128', realpath='swap', mountpoint=None, fullsize=False, label='SWAP', @@ -509,6 +512,7 @@ def test_get_volumes_no_explicit_root_setup_other_fullsize_volume(self): volume_type = namedtuple( 'volume_type', [ 'name', + 'parent', 'size', 'realpath', 'mountpoint', @@ -520,21 +524,21 @@ def test_get_volumes_no_explicit_root_setup_other_fullsize_volume(self): ) assert state.get_volumes() == [ volume_type( - name='usr', size=None, realpath='usr', + name='usr', parent='', size=None, realpath='usr', mountpoint='usr', fullsize=True, label=None, attributes=[], is_root_volume=False ), volume_type( - name='LVRoot', size='freespace:30', realpath='/', + name='LVRoot', parent='', size='freespace:30', realpath='/', mountpoint=None, fullsize=False, label=None, attributes=[], is_root_volume=True ), volume_type( - name='LVSwap', size='size:128', + name='LVSwap', parent='', size='size:128', realpath='swap', mountpoint=None, fullsize=False, label='SWAP', From 1c498159493186c788ff15fa4269158d232ac979 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20Sch=C3=A4fer?= Date: Sun, 16 Jul 2023 21:24:30 +0200 Subject: [PATCH 04/12] Use btrfs for fedora/test-image-live-disk test Change the Virtual profile to build a btrfs based image for testing respective btrfs layouts --- build-tests/x86/fedora/test-image-live-disk/appliance.kiwi | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/build-tests/x86/fedora/test-image-live-disk/appliance.kiwi b/build-tests/x86/fedora/test-image-live-disk/appliance.kiwi index 5bdc5592528..965fafb252c 100644 --- a/build-tests/x86/fedora/test-image-live-disk/appliance.kiwi +++ b/build-tests/x86/fedora/test-image-live-disk/appliance.kiwi @@ -30,7 +30,11 @@ - + + + + + false @@ -67,6 +71,7 @@ + From 8b39909a2399067a29aea6d38d45fd21d8eacb33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20Sch=C3=A4fer?= Date: Wed, 19 Jul 2023 15:24:04 +0200 Subject: [PATCH 05/12] Fixed apply_attributes_on_volume Make the function call more robust in terms of path separation --- kiwi/volume_manager/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kiwi/volume_manager/base.py b/kiwi/volume_manager/base.py index 71f226e9c59..67cbdf319a4 100644 --- a/kiwi/volume_manager/base.py +++ b/kiwi/volume_manager/base.py @@ -150,7 +150,7 @@ def apply_attributes_on_volume(self, toplevel, volume): Command.run( [ 'chattr', '+C', - os.path.normpath(toplevel + volume.realpath) + os.path.normpath(toplevel + os.sep + volume.realpath) ] ) From 6d40c6f26c3077cb79213194dd309aaa1f79da23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20Sch=C3=A4fer?= Date: Wed, 19 Jul 2023 16:06:09 +0200 Subject: [PATCH 06/12] Use bootpartition for Fedora integration test When using btrfs with the proposed layout for testing the delivered grub bios module for the Fedora system used to build the integration test (FC37) is not capable to find the grub config file. A manual call for configfile in the grub shell fixes this with the existing kiwi created grub early-boot script. However, it is expected that the delivered grub image works and kiwi only creates its own one if no distro delivered grub image was found. To make the integration test functional for both BIOS and EFI the simple solution is to use an extra not btrfs based boot partition. This still allows to test the desired btrfs layout in terms of volumes and sub-volumes and does not break on any of the boot methods. --- build-tests/x86/fedora/test-image-live-disk/appliance.kiwi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build-tests/x86/fedora/test-image-live-disk/appliance.kiwi b/build-tests/x86/fedora/test-image-live-disk/appliance.kiwi index 965fafb252c..79a4554d790 100644 --- a/build-tests/x86/fedora/test-image-live-disk/appliance.kiwi +++ b/build-tests/x86/fedora/test-image-live-disk/appliance.kiwi @@ -30,7 +30,7 @@ - + From f97b47e8fbbcab78b69c48ee08d13a4e803bcc0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20Sch=C3=A4fer?= Date: Tue, 25 Jul 2023 12:43:31 +0200 Subject: [PATCH 07/12] Fix fallback secure boot setup Don't copy the same file. This case happens when rebuilding an image using --allow-existing-root when the fallback setup has done its job already in the first run --- kiwi/bootloader/config/grub2.py | 56 ++++++++++++++--------- test/unit/bootloader/config/grub2_test.py | 12 +++-- 2 files changed, 43 insertions(+), 25 deletions(-) diff --git a/kiwi/bootloader/config/grub2.py b/kiwi/bootloader/config/grub2.py index 06cd347a1c0..3e8bbde7dec 100644 --- a/kiwi/bootloader/config/grub2.py +++ b/kiwi/bootloader/config/grub2.py @@ -809,35 +809,47 @@ def _setup_secure_boot_efi_image( # a grub image that got signed by the shim. The shim image # is the one that gets loaded by the firmware which itself # loads the second stage grub image - log.info( - f'--> Using shim image: {shim_image.filename}' - ) - log.info( - f'--> Using grub image: {grub_image.filename}' - ) - Command.run( - ['cp', shim_image.filename, self._get_efi_image_name()] - ) - Command.run( - [ - 'cp', grub_image.filename, - os.sep.join([self.efi_boot_path, grub_image.binaryname]) - ] + target_efi_image_name = self._get_efi_image_name() + target_grub_image_name = os.sep.join( + [self.efi_boot_path, grub_image.binaryname] ) + if not os.path.isfile(target_efi_image_name): + log.info( + f'--> Using shim image: {shim_image.filename}' + ) + Command.run( + ['cp', shim_image.filename, target_efi_image_name] + ) + if not os.path.isfile(target_grub_image_name): + log.info( + f'--> Using grub image: {grub_image.filename}' + ) + Command.run( + ['cp', grub_image.filename, target_grub_image_name] + ) mok_manager = Defaults.get_mok_manager(lookup_path) if mok_manager: - Command.run( - ['cp', mok_manager, self.efi_boot_path] + target_mok_manager = os.sep.join( + [self.efi_boot_path, os.path.basename(mok_manager)] ) + if not os.path.isfile(target_mok_manager): + log.info( + f'--> Using mok image: {mok_manager}' + ) + Command.run( + ['cp', mok_manager, self.efi_boot_path] + ) else: # Without shim a self signed grub image is used that # gets loaded by the firmware - log.info( - f'--> No shim image, using grub image: {grub_image.filename}' - ) - Command.run( - ['cp', grub_image.filename, self._get_efi_image_name()] - ) + target_efi_image_name = self._get_efi_image_name() + if not os.path.isfile(target_efi_image_name): + log.info( + f'--> No shim image, using grub image: {grub_image.filename}' + ) + Command.run( + ['cp', grub_image.filename, target_efi_image_name] + ) self._create_efi_config_search(uuid, mbrid) def _setup_efi_image( diff --git a/test/unit/bootloader/config/grub2_test.py b/test/unit/bootloader/config/grub2_test.py index 03245cb7497..619e75fadb2 100644 --- a/test/unit/bootloader/config/grub2_test.py +++ b/test/unit/bootloader/config/grub2_test.py @@ -1494,8 +1494,9 @@ def side_effect(arg): @patch('glob.iglob') @patch('os.chmod') @patch('os.stat') + @patch('os.path.isfile') def test_setup_disk_boot_images_bios_plus_efi_secure_boot_no_shim_install( - self, mock_stat, mock_chmod, mock_glob, + self, mock_isfile, mock_stat, mock_chmod, mock_glob, mock_exists, mock_command, mock_which, mock_get_boot_path ): # we expect the copy of shim.efi and grub.efi from the fallback @@ -1503,6 +1504,7 @@ def test_setup_disk_boot_images_bios_plus_efi_secure_boot_no_shim_install( Defaults.set_platform_name('x86_64') mock_get_boot_path.return_value = '/boot' mock_which.return_value = None + mock_isfile.return_value = False self.firmware.efi_mode = Mock( return_value='uefi' ) @@ -1587,8 +1589,9 @@ def side_effect_glob(arg): @patch('glob.iglob') @patch('os.chmod') @patch('os.stat') + @patch('os.path.isfile') def test_setup_disk_boot_images_bios_plus_efi_secure_boot_no_shim_at_all( - self, mock_stat, mock_chmod, mock_glob, + self, mock_isfile, mock_stat, mock_chmod, mock_glob, mock_exists, mock_command, mock_which, mock_get_boot_path, mock_get_shim_loader ): @@ -1599,6 +1602,7 @@ def test_setup_disk_boot_images_bios_plus_efi_secure_boot_no_shim_at_all( Defaults.set_platform_name('x86_64') mock_get_boot_path.return_value = '/boot' mock_which.return_value = None + mock_isfile.return_value = False self.firmware.efi_mode = Mock( return_value='uefi' ) @@ -1878,12 +1882,14 @@ def side_effect(arg): @patch('glob.iglob') @patch('os.chmod') @patch('os.stat') + @patch('os.path.isfile') def test_setup_install_boot_images_efi_secure_boot( - self, mock_stat, mock_chmod, mock_glob, + self, mock_isfile, mock_stat, mock_chmod, mock_glob, mock_exists, mock_command, mock_supports_bios_modules ): Defaults.set_platform_name('x86_64') mock_supports_bios_modules.return_value = False + mock_isfile.return_value = False self.os_exists['root_dir'] = True self.firmware.efi_mode = Mock( return_value='uefi' From f8f1c0056e711f9d7ceef7b6057768a29d63af91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20Sch=C3=A4fer?= Date: Tue, 25 Jul 2023 13:05:35 +0200 Subject: [PATCH 08/12] Take subvol mount option for root into account If the rootfs is btrfs based make sure the fstab entry for it takes the name of the root subvolume into account --- kiwi/builder/disk.py | 7 +++++++ test/unit/builder/disk_test.py | 35 ++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/kiwi/builder/disk.py b/kiwi/builder/disk.py index e0e4f24200a..c574d99ca61 100644 --- a/kiwi/builder/disk.py +++ b/kiwi/builder/disk.py @@ -1119,6 +1119,13 @@ def _write_generic_fstab( custom_root_mount_args += ['ro'] fs_check_interval = '0 0' + if self.volume_manager_name and self.volume_manager_name == 'btrfs': + root_volume_name = system.get_root_volume_name() + if root_volume_name != '/': + custom_root_mount_args += [ + f'defaults,subvol={root_volume_name}' + ] + self._add_fstab_entry( device_map['root'].get_device(), '/', custom_root_mount_args, fs_check_interval diff --git a/test/unit/builder/disk_test.py b/test/unit/builder/disk_test.py index b943b402db8..c38e035d90a 100644 --- a/test/unit/builder/disk_test.py +++ b/test/unit/builder/disk_test.py @@ -1197,6 +1197,41 @@ def get_filesystem(): call('LABEL=blkid_result /var ext3 defaults 0 0') ] in self.disk_builder.fstab.add_entry.call_args_list + @patch('kiwi.builder.disk.FileSystem.new') + @patch('kiwi.builder.disk.VolumeManager.new') + @patch('kiwi.builder.disk.Command.run') + @patch('kiwi.builder.disk.Defaults.get_grub_boot_directory_name') + @patch('kiwi.builder.disk.ImageSystem') + @patch('os.path.exists') + def test_create_disk_btrfs_managed_root( + self, mock_exists, mock_ImageSystem, mock_grub_dir, mock_command, + mock_volume_manager, mock_fs + ): + mock_exists.return_value = True + volume_manager = Mock() + volume_manager.get_device = Mock( + return_value={ + 'root': MappedDevice('root', Mock()) + } + ) + volume_manager.get_fstab = Mock( + return_value=['fstab_volume_entries'] + ) + volume_manager.get_root_volume_name = Mock( + return_value='@' + ) + mock_volume_manager.return_value = volume_manager + filesystem = Mock() + mock_fs.return_value = filesystem + self.disk_builder.volume_manager_name = 'btrfs' + + with patch('builtins.open'): + self.disk_builder.create_disk() + + assert [ + call('UUID=blkid_result / blkid_result_fs ro,defaults,subvol=@ 0 0') + ] in self.disk_builder.fstab.add_entry.call_args_list + @patch('kiwi.builder.disk.FileSystem.new') @patch('kiwi.builder.disk.VolumeManager.new') @patch('kiwi.builder.disk.Command.run') From a31ef6a1449daaa6a2a376db2f82f85e22b3081c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20Sch=C3=A4fer?= Date: Thu, 27 Jul 2023 09:18:33 +0200 Subject: [PATCH 09/12] add btrfs_set_default_volume attribute By default kiwi runs btrfs set-default on the volume that is considered the default volume according to the btrfs settings and defaults. btrfs_set_default_volume="false" allows to deactivate this action. Along with the change also the misleading name of the btrfs_create_toplevel_subvolume has been changed to root_is_subvolume --- doc/source/image_description/elements.rst | 18 +++++++--- kiwi/builder/disk.py | 7 ++-- kiwi/schema/kiwi.rnc | 33 +++++++++++------ kiwi/schema/kiwi.rng | 38 ++++++++++++++------ kiwi/volume_manager/btrfs.py | 34 ++++++++++-------- kiwi/xml_parse.py | 43 +++++++++++++++-------- test/unit/xml_state_test.py | 4 +-- 7 files changed, 120 insertions(+), 57 deletions(-) diff --git a/doc/source/image_description/elements.rst b/doc/source/image_description/elements.rst index 0ecc3914b3e..905597d70ff 100644 --- a/doc/source/image_description/elements.rst +++ b/doc/source/image_description/elements.rst @@ -412,11 +412,19 @@ btrfs_quota_groups="true|false": Boolean parameter to activate filesystem quotas if the filesystem is `btrfs`. By default quotas are inactive. -btrfs_create_toplevel_subvolume="true|false": - Tell kiwi to nest all subvolumes below a toplevel volume. - The name of this toplevel volume is by default set to: `@` - The name of the toplevel volume can be changed via - a volume entry of the form +btrfs_set_default_volume="true|false": + Tell kiwi to explicitly make a volume the default volume + This can be either `/` or the root subvolume or the root + snapshot depending on the specified btrfs configuration + attributes. By default btrfs_set_default_volume is set to: true + If no default volume should be set, this attribute can be + used to turn it off + +btrfs_root_is_subvolume="true|false": + Tell kiwi to create a root volume to host (/) inside. + The name of this subvolume is by default set to: `@`. + The name of the subvolume can be changed via a volume entry + of the form: .. code:: xml diff --git a/kiwi/builder/disk.py b/kiwi/builder/disk.py index c574d99ca61..9141d75ef08 100644 --- a/kiwi/builder/disk.py +++ b/kiwi/builder/disk.py @@ -417,9 +417,12 @@ def create_disk(self) -> Result: 'root_is_readonly_snapshot': self.xml_state.build_type. get_btrfs_root_is_readonly_snapshot(), - 'create_toplevel_subvolume': + 'root_is_subvolume': self.xml_state.build_type. - get_btrfs_create_toplevel_subvolume(), + get_btrfs_root_is_subvolume(), + 'set_default_volume': + self.xml_state.build_type. + get_btrfs_set_default_volume(), 'quota_groups': self.xml_state.build_type.get_btrfs_quota_groups(), 'resize_on_boot': diff --git a/kiwi/schema/kiwi.rnc b/kiwi/schema/kiwi.rnc index fcae7c00ba3..a8fc8b7b673 100644 --- a/kiwi/schema/kiwi.rnc +++ b/kiwi/schema/kiwi.rnc @@ -1433,15 +1433,27 @@ div { sch:param [ name = "attr" value = "btrfs_quota_groups" ] sch:param [ name = "types" value = "oem" ] ] - k.type.btrfs_create_toplevel_subvolume = - ## Tell kiwi to nest all subvolumes below a toplevel volume. - ## The name of this toplevel volume is by default set to: '@' - ## The name of the toplevel volume can be changed via - ## a volume entry of the form '' - ## By default the creation of a toplevel volume is set to: true - attribute btrfs_create_toplevel_subvolume { xsd:boolean } - >> sch:pattern [ id = "btrfs_create_toplevel_subvolume" is-a = "image_type" - sch:param [ name = "attr" value = "btrfs_create_toplevel_subvolume" ] + k.type.btrfs_set_default_volume.attribute = + ## Tell kiwi to explicitly make a volume the default volume + ## This can be either (/) or the root subvolume or the root + ## snapshot depending on the specified btrfs configuration + ## attributes. By default btrfs_set_default_volume is set to: true + ## If no default volume should be set, this attribute can be + ## used to turn it off + attribute btrfs_set_default_volume { xsd:boolean } + >> sch:pattern [ id = "btrfs_set_default_volume" is-a = "image_type" + sch:param [ name = "attr" value = "btrfs_set_default_volume" ] + sch:param [ name = "types" value = "oem" ] + ] + k.type.btrfs_root_is_subvolume.attribute = + ## Tell kiwi to create a root volume to host (/) inside. + ## The name of this subvolume is by default set to: '@' + ## The name of the subvolume can be changed via + ## a volume entry of the form '' + ## By default the creation of a root subvolume is set to: true + attribute btrfs_root_is_subvolume { xsd:boolean } + >> sch:pattern [ id = "btrfs_root_is_subvolume" is-a = "image_type" + sch:param [ name = "attr" value = "btrfs_root_is_subvolume" ] sch:param [ name = "types" value = "oem" ] ] k.type.btrfs_root_is_snapshot.attribute = @@ -2229,7 +2241,8 @@ div { k.type.bootprofile.attribute? & k.type.btrfs_quota_groups.attribute? & k.type.btrfs_root_is_snapshot.attribute? & - k.type.btrfs_create_toplevel_subvolume? & + k.type.btrfs_root_is_subvolume.attribute? & + k.type.btrfs_set_default_volume.attribute? & k.type.btrfs_root_is_readonly_snapshot.attribute? & k.type.compressed.attribute? & k.type.devicepersistency.attribute? & diff --git a/kiwi/schema/kiwi.rng b/kiwi/schema/kiwi.rng index 9d1d9b9e6dd..2b32feaf77c 100644 --- a/kiwi/schema/kiwi.rng +++ b/kiwi/schema/kiwi.rng @@ -2093,17 +2093,32 @@ By default the quota system is inactive - - - Tell kiwi to nest all subvolumes below a toplevel volume. -The name of this toplevel volume is by default set to: '@' -The name of the toplevel volume can be changed via -a volume entry of the form '<volume name="@root=TOPLEVEL_NAME"/>' -By default the creation of a toplevel volume is set to: true + + + Tell kiwi to explicitly make a volume the default volume +This can be either (/) or the root subvolume or the root +snapshot depending on the specified btrfs configuration +attributes. By default btrfs_set_default_volume is set to: true +If no default volume should be set, this attribute can be +used to turn it off - - + + + + + + + + Tell kiwi to create a root volume to host (/) inside. +The name of this subvolume is by default set to: '@' +The name of the subvolume can be changed via +a volume entry of the form '<volume name="@root=NAME"/>' +By default the creation of a root subvolume is set to: true + + + + @@ -3192,7 +3207,10 @@ kiwi-ng result bundle ... - + + + + diff --git a/kiwi/volume_manager/btrfs.py b/kiwi/volume_manager/btrfs.py index 04489f6cd3e..1064c086b97 100644 --- a/kiwi/volume_manager/btrfs.py +++ b/kiwi/volume_manager/btrfs.py @@ -67,20 +67,24 @@ def post_init(self, custom_args): self.custom_args['root_label'] = 'ROOT' if 'root_is_snapshot' not in self.custom_args: self.custom_args['root_is_snapshot'] = False + if 'set_default_volume' not in self.custom_args: + self.custom_args['set_default_volume'] = True if 'root_is_readonly_snapshot' not in self.custom_args: self.custom_args['root_is_readonly_snapshot'] = False - if 'create_toplevel_subvolume' not in self.custom_args: - self.custom_args['create_toplevel_subvolume'] = None + if 'root_is_subvolume' not in self.custom_args: + self.custom_args['root_is_subvolume'] = None if 'quota_groups' not in self.custom_args: self.custom_args['quota_groups'] = False self.root_volume_name = '/' + self.default_volume_name = self.root_volume_name if self._has_root_volume(): self.root_volume_name = '@' canonical_volume_list = self.get_canonical_volume_list() for volume in canonical_volume_list.volumes: if volume.is_root_volume and volume.name: self.root_volume_name = volume.name + self.default_volume_name = self.root_volume_name if self.custom_args['root_is_snapshot'] and \ self.root_volume_name == '/': @@ -169,7 +173,7 @@ def get_root_volume_name(self) -> str: :rtype: string """ - return self.root_volume_name + return self.default_volume_name def create_volumes(self, filesystem_name): """ @@ -444,18 +448,18 @@ def set_property_readonly_root(self): ) def _has_root_volume(self) -> bool: - has_root_volume = bool(self.custom_args['create_toplevel_subvolume']) - if self.custom_args['create_toplevel_subvolume'] is None: - # toplevel volume not explicitly configured, will + has_root_volume = bool(self.custom_args['root_is_subvolume']) + if self.custom_args['root_is_subvolume'] is None: + # root volume not explicitly configured, will # be enabled by default but this is going to change # in the future. Print a deprecation message to inform # the user about a potential behavior change - log.warning("Implicitly creating a toplevel volume") + log.warning("Implicitly creating root volume") log.warning( "--> Future versions of kiwi will not do this anymore" ) log.warning( - "--> Please specify btrfs_create_toplevel_subvolume true|false" + "--> Please specify btrfs_root_is_subvolume true|false" ) has_root_volume = True return has_root_volume @@ -477,12 +481,14 @@ def _set_default_volume(self, default_volume): volume_id = id_search.group(1) volume_path = id_search.group(2) if volume_path == default_volume: - Command.run( - [ - 'btrfs', 'subvolume', 'set-default', - volume_id, self.mountpoint - ] - ) + if self.custom_args['set_default_volume'] is not False: + Command.run( + [ + 'btrfs', 'subvolume', 'set-default', + volume_id, self.mountpoint + ] + ) + self.default_volume_name = default_volume return raise KiwiVolumeRootIDError( diff --git a/kiwi/xml_parse.py b/kiwi/xml_parse.py index 693f1ec452e..bee51707977 100644 --- a/kiwi/xml_parse.py +++ b/kiwi/xml_parse.py @@ -3,7 +3,7 @@ # # Generated by generateDS.py version 2.29.24. -# Python 3.6.15 (default, Sep 23 2021, 15:41:43) [GCC] +# Python 3.11.3 (main, Jun 03 2023, 22:12:18) [GCC] # # Command line options: # ('-f', '') @@ -16,7 +16,7 @@ # kiwi/schema/kiwi_for_generateDS.xsd # # Command line: -# /home/ms/Project/kiwi/.tox/3.6/bin/generateDS.py -f --external-encoding="utf-8" --no-dates --no-warnings -o "kiwi/xml_parse.py" kiwi/schema/kiwi_for_generateDS.xsd +# /home/ms/Project/kiwi/.tox/unit_py3_11/bin/generateDS.py -f --external-encoding="utf-8" --no-dates --no-warnings -o "kiwi/xml_parse.py" kiwi/schema/kiwi_for_generateDS.xsd # # Current working directory (os.getcwd()): # kiwi @@ -3048,7 +3048,7 @@ class type_(GeneratedsSuper): """The Image Type of the Logical Extend""" subclass = None superclass = None - def __init__(self, boot=None, bootfilesystem=None, firmware=None, bootkernel=None, bootpartition=None, bootpartsize=None, efipartsize=None, efifatimagesize=None, efiparttable=None, dosparttable_extended_layout=None, bootprofile=None, btrfs_quota_groups=None, btrfs_root_is_snapshot=None, btrfs_create_toplevel_subvolume=None, btrfs_root_is_readonly_snapshot=None, compressed=None, devicepersistency=None, editbootconfig=None, editbootinstall=None, filesystem=None, flags=None, format=None, formatoptions=None, fsmountoptions=None, fscreateoptions=None, squashfscompression=None, gcelicense=None, hybridpersistent=None, hybridpersistent_filesystem=None, gpt_hybrid_mbr=None, force_mbr=None, initrd_system=None, image=None, metadata_path=None, installboot=None, install_continue_on_timeout=None, installprovidefailsafe=None, installiso=None, installstick=None, installpxe=None, mediacheck=None, kernelcmdline=None, luks=None, luks_version=None, luksOS=None, luks_randomize=None, luks_pbkdf=None, mdraid=None, overlayroot=None, overlayroot_write_partition=None, overlayroot_readonly_partsize=None, verity_blocks=None, embed_verity_metadata=None, standalone_integrity=None, embed_integrity_metadata=None, integrity_legacy_hmac=None, integrity_metadata_key_description=None, integrity_keyfile=None, primary=None, ramonly=None, rootfs_label=None, spare_part=None, spare_part_mountpoint=None, spare_part_fs=None, spare_part_fs_attributes=None, spare_part_is_last=None, target_blocksize=None, target_removable=None, selinux_policy=None, vga=None, vhdfixedtag=None, volid=None, wwid_wait_timeout=None, derived_from=None, delta_root=None, ensure_empty_tmpdirs=None, xen_server=None, publisher=None, disk_start_sector=None, root_clone=None, boot_clone=None, bundle_format=None, bootloader=None, containerconfig=None, machine=None, oemconfig=None, size=None, systemdisk=None, partitions=None, vagrantconfig=None, installmedia=None, luksformat=None): + def __init__(self, boot=None, bootfilesystem=None, firmware=None, bootkernel=None, bootpartition=None, bootpartsize=None, efipartsize=None, efifatimagesize=None, efiparttable=None, dosparttable_extended_layout=None, bootprofile=None, btrfs_quota_groups=None, btrfs_root_is_snapshot=None, btrfs_root_is_subvolume=None, btrfs_set_default_volume=None, btrfs_root_is_readonly_snapshot=None, compressed=None, devicepersistency=None, editbootconfig=None, editbootinstall=None, filesystem=None, flags=None, format=None, formatoptions=None, fsmountoptions=None, fscreateoptions=None, squashfscompression=None, gcelicense=None, hybridpersistent=None, hybridpersistent_filesystem=None, gpt_hybrid_mbr=None, force_mbr=None, initrd_system=None, image=None, metadata_path=None, installboot=None, install_continue_on_timeout=None, installprovidefailsafe=None, installiso=None, installstick=None, installpxe=None, mediacheck=None, kernelcmdline=None, luks=None, luks_version=None, luksOS=None, luks_randomize=None, luks_pbkdf=None, mdraid=None, overlayroot=None, overlayroot_write_partition=None, overlayroot_readonly_partsize=None, verity_blocks=None, embed_verity_metadata=None, standalone_integrity=None, embed_integrity_metadata=None, integrity_legacy_hmac=None, integrity_metadata_key_description=None, integrity_keyfile=None, primary=None, ramonly=None, rootfs_label=None, spare_part=None, spare_part_mountpoint=None, spare_part_fs=None, spare_part_fs_attributes=None, spare_part_is_last=None, target_blocksize=None, target_removable=None, selinux_policy=None, vga=None, vhdfixedtag=None, volid=None, wwid_wait_timeout=None, derived_from=None, delta_root=None, ensure_empty_tmpdirs=None, xen_server=None, publisher=None, disk_start_sector=None, root_clone=None, boot_clone=None, bundle_format=None, bootloader=None, containerconfig=None, machine=None, oemconfig=None, size=None, systemdisk=None, partitions=None, vagrantconfig=None, installmedia=None, luksformat=None): self.original_tagname_ = None self.boot = _cast(None, boot) self.bootfilesystem = _cast(None, bootfilesystem) @@ -3063,7 +3063,8 @@ def __init__(self, boot=None, bootfilesystem=None, firmware=None, bootkernel=Non self.bootprofile = _cast(None, bootprofile) self.btrfs_quota_groups = _cast(bool, btrfs_quota_groups) self.btrfs_root_is_snapshot = _cast(bool, btrfs_root_is_snapshot) - self.btrfs_create_toplevel_subvolume = _cast(bool, btrfs_create_toplevel_subvolume) + self.btrfs_root_is_subvolume = _cast(bool, btrfs_root_is_subvolume) + self.btrfs_set_default_volume = _cast(bool, btrfs_set_default_volume) self.btrfs_root_is_readonly_snapshot = _cast(bool, btrfs_root_is_readonly_snapshot) self.compressed = _cast(bool, compressed) self.devicepersistency = _cast(None, devicepersistency) @@ -3259,8 +3260,10 @@ def get_btrfs_quota_groups(self): return self.btrfs_quota_groups def set_btrfs_quota_groups(self, btrfs_quota_groups): self.btrfs_quota_groups = btrfs_quota_groups def get_btrfs_root_is_snapshot(self): return self.btrfs_root_is_snapshot def set_btrfs_root_is_snapshot(self, btrfs_root_is_snapshot): self.btrfs_root_is_snapshot = btrfs_root_is_snapshot - def get_btrfs_create_toplevel_subvolume(self): return self.btrfs_create_toplevel_subvolume - def set_btrfs_create_toplevel_subvolume(self, btrfs_create_toplevel_subvolume): self.btrfs_create_toplevel_subvolume = btrfs_create_toplevel_subvolume + def get_btrfs_root_is_subvolume(self): return self.btrfs_root_is_subvolume + def set_btrfs_root_is_subvolume(self, btrfs_root_is_subvolume): self.btrfs_root_is_subvolume = btrfs_root_is_subvolume + def get_btrfs_set_default_volume(self): return self.btrfs_set_default_volume + def set_btrfs_set_default_volume(self, btrfs_set_default_volume): self.btrfs_set_default_volume = btrfs_set_default_volume def get_btrfs_root_is_readonly_snapshot(self): return self.btrfs_root_is_readonly_snapshot def set_btrfs_root_is_readonly_snapshot(self, btrfs_root_is_readonly_snapshot): self.btrfs_root_is_readonly_snapshot = btrfs_root_is_readonly_snapshot def get_compressed(self): return self.compressed @@ -3516,9 +3519,12 @@ def exportAttributes(self, outfile, level, already_processed, namespaceprefix_=' if self.btrfs_root_is_snapshot is not None and 'btrfs_root_is_snapshot' not in already_processed: already_processed.add('btrfs_root_is_snapshot') outfile.write(' btrfs_root_is_snapshot="%s"' % self.gds_format_boolean(self.btrfs_root_is_snapshot, input_name='btrfs_root_is_snapshot')) - if self.btrfs_create_toplevel_subvolume is not None and 'btrfs_create_toplevel_subvolume' not in already_processed: - already_processed.add('btrfs_create_toplevel_subvolume') - outfile.write(' btrfs_create_toplevel_subvolume="%s"' % self.gds_format_boolean(self.btrfs_create_toplevel_subvolume, input_name='btrfs_create_toplevel_subvolume')) + if self.btrfs_root_is_subvolume is not None and 'btrfs_root_is_subvolume' not in already_processed: + already_processed.add('btrfs_root_is_subvolume') + outfile.write(' btrfs_root_is_subvolume="%s"' % self.gds_format_boolean(self.btrfs_root_is_subvolume, input_name='btrfs_root_is_subvolume')) + if self.btrfs_set_default_volume is not None and 'btrfs_set_default_volume' not in already_processed: + already_processed.add('btrfs_set_default_volume') + outfile.write(' btrfs_set_default_volume="%s"' % self.gds_format_boolean(self.btrfs_set_default_volume, input_name='btrfs_set_default_volume')) if self.btrfs_root_is_readonly_snapshot is not None and 'btrfs_root_is_readonly_snapshot' not in already_processed: already_processed.add('btrfs_root_is_readonly_snapshot') outfile.write(' btrfs_root_is_readonly_snapshot="%s"' % self.gds_format_boolean(self.btrfs_root_is_readonly_snapshot, input_name='btrfs_root_is_readonly_snapshot')) @@ -3846,13 +3852,22 @@ def buildAttributes(self, node, attrs, already_processed): self.btrfs_root_is_snapshot = False else: raise_parse_error(node, 'Bad boolean attribute') - value = find_attr_value_('btrfs_create_toplevel_subvolume', node) - if value is not None and 'btrfs_create_toplevel_subvolume' not in already_processed: - already_processed.add('btrfs_create_toplevel_subvolume') + value = find_attr_value_('btrfs_root_is_subvolume', node) + if value is not None and 'btrfs_root_is_subvolume' not in already_processed: + already_processed.add('btrfs_root_is_subvolume') if value in ('true', '1'): - self.btrfs_create_toplevel_subvolume = True + self.btrfs_root_is_subvolume = True elif value in ('false', '0'): - self.btrfs_create_toplevel_subvolume = False + self.btrfs_root_is_subvolume = False + else: + raise_parse_error(node, 'Bad boolean attribute') + value = find_attr_value_('btrfs_set_default_volume', node) + if value is not None and 'btrfs_set_default_volume' not in already_processed: + already_processed.add('btrfs_set_default_volume') + if value in ('true', '1'): + self.btrfs_set_default_volume = True + elif value in ('false', '0'): + self.btrfs_set_default_volume = False else: raise_parse_error(node, 'Bad boolean attribute') value = find_attr_value_('btrfs_root_is_readonly_snapshot', node) diff --git a/test/unit/xml_state_test.py b/test/unit/xml_state_test.py index dbc7fa0b574..d39cf42fed2 100644 --- a/test/unit/xml_state_test.py +++ b/test/unit/xml_state_test.py @@ -1137,6 +1137,6 @@ def test_get_bootloader_options(self): '--joe', '-x' ] - def test_get_btrfs_create_toplevel_subvolume(self): - assert self.state.build_type.get_btrfs_create_toplevel_subvolume() is \ + def test_get_btrfs_root_is_subvolume(self): + assert self.state.build_type.get_btrfs_root_is_subvolume() is \ None From 48149cea512ea164137cb0f491a2923daf77df25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20Sch=C3=A4fer?= Date: Thu, 27 Jul 2023 09:18:54 +0200 Subject: [PATCH 10/12] update Fedora integration test The setting of a default volume is unwanted here --- build-tests/x86/fedora/test-image-live-disk/appliance.kiwi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build-tests/x86/fedora/test-image-live-disk/appliance.kiwi b/build-tests/x86/fedora/test-image-live-disk/appliance.kiwi index 79a4554d790..02f0f3fb697 100644 --- a/build-tests/x86/fedora/test-image-live-disk/appliance.kiwi +++ b/build-tests/x86/fedora/test-image-live-disk/appliance.kiwi @@ -30,7 +30,7 @@ - + From dd110f63a4d8bbf2db44a967d6c906b0dfb5b4cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20Sch=C3=A4fer?= Date: Thu, 27 Jul 2023 12:08:35 +0200 Subject: [PATCH 11/12] Make sure btrfs root volume is used when needed With the possibility to switch off setting the default volume an issue at other parts in the kiwi code which mounted the btrfs based system were uncovered. Without any default volume set it's required to transport the root volume if different from / and pass the respective subvol= option to the mount. This commit fixes it at the places where kiwi trusted btrfs to have a correct default volume set --- kiwi/bootloader/config/base.py | 8 ++++-- kiwi/bootloader/config/grub2.py | 8 ++++-- kiwi/bootloader/install/grub2.py | 9 ++++++- kiwi/builder/disk.py | 7 +++++- test/unit/bootloader/config/base_test.py | 6 +++-- test/unit/bootloader/config/grub2_test.py | 2 +- test/unit/bootloader/install/grub2_test.py | 29 ++++++++++++++++------ 7 files changed, 53 insertions(+), 16 deletions(-) diff --git a/kiwi/bootloader/config/base.py b/kiwi/bootloader/config/base.py index d3e2cae9b40..b76828eeef0 100644 --- a/kiwi/bootloader/config/base.py +++ b/kiwi/bootloader/config/base.py @@ -501,7 +501,8 @@ def get_gfxmode(self, target): return gfxmode def _mount_system( - self, root_device, boot_device, efi_device=None, volumes=None + self, root_device, boot_device, efi_device=None, + volumes=None, root_volume_name=None ): self.root_mount = MountManager( device=root_device @@ -522,7 +523,10 @@ def _mount_system( mountpoint=self.root_mount.mountpoint + '/boot/efi' ) - self.root_mount.mount() + custom_root_mount_args = [] + if root_volume_name and root_volume_name != '/': + custom_root_mount_args += [f'subvol={root_volume_name}'] + self.root_mount.mount(options=custom_root_mount_args) if not self.root_mount.device == self.boot_mount.device: self.boot_mount.mount() diff --git a/kiwi/bootloader/config/grub2.py b/kiwi/bootloader/config/grub2.py index 3e8bbde7dec..fcfac01bb76 100644 --- a/kiwi/bootloader/config/grub2.py +++ b/kiwi/bootloader/config/grub2.py @@ -233,14 +233,18 @@ def setup_disk_image_config( 'root_device': string, 'boot_device': string, 'efi_device': string, - 'system_volumes': volume_manager_instance.get_volumes() + 'system_volumes': + volume_manager_instance.get_volumes(), + 'system_root_volume': + volume_manager_instance.get_root_volume_name() } """ self._mount_system( boot_options.get('root_device'), boot_options.get('boot_device'), boot_options.get('efi_device'), - boot_options.get('system_volumes') + boot_options.get('system_volumes'), + boot_options.get('system_root_volume') ) config_file = os.sep.join( [ diff --git a/kiwi/bootloader/install/grub2.py b/kiwi/bootloader/install/grub2.py index 565ae38f9ca..637aa3d7fe0 100644 --- a/kiwi/bootloader/install/grub2.py +++ b/kiwi/bootloader/install/grub2.py @@ -52,6 +52,7 @@ def post_init(self, custom_args): { 'target_removable': bool, 'system_volumes': list_of_volumes, + 'system_root_volume': root volume name if required 'firmware': FirmWare_instance, 'efi_device': string, 'boot_device': string, @@ -71,12 +72,15 @@ def post_init(self, custom_args): self.proc_mount = None self.sysfs_mount = None self.volumes = None + self.root_volume_name = None self.volumes_mount = [] self.target_removable = None if custom_args and 'target_removable' in custom_args: self.target_removable = custom_args['target_removable'] if custom_args and 'system_volumes' in custom_args: self.volumes = custom_args['system_volumes'] + if custom_args and 'system_root_volume' in custom_args: + self.root_volume_name = custom_args['system_root_volume'] if custom_args and 'firmware' in custom_args: self.firmware = custom_args['firmware'] if custom_args and 'install_options' in custom_args: @@ -302,7 +306,10 @@ def _mount_device_and_volumes(self): self.root_mount = MountManager( device=self.custom_args['root_device'] ) - self.root_mount.mount() + custom_root_mount_args = [] + if self.root_volume_name and self.root_volume_name != '/': + custom_root_mount_args += [f'subvol={self.root_volume_name}'] + self.root_mount.mount(options=custom_root_mount_args) if self.boot_mount is None: if 's390' in self.arch: diff --git a/kiwi/builder/disk.py b/kiwi/builder/disk.py index 9141d75ef08..b3bb4d0ad13 100644 --- a/kiwi/builder/disk.py +++ b/kiwi/builder/disk.py @@ -1537,7 +1537,12 @@ def _install_bootloader( if self.volume_manager_name: system.umount_volumes() custom_install_arguments.update( - {'system_volumes': system.get_volumes()} + { + 'system_volumes': system.get_volumes(), + 'system_root_volume': + system.get_root_volume_name() + if self.volume_manager_name == 'btrfs' else None + } ) if self.bootloader != 'custom': diff --git a/test/unit/bootloader/config/base_test.py b/test/unit/bootloader/config/base_test.py index 4bf16c51902..f81847b5b5e 100644 --- a/test/unit/bootloader/config/base_test.py +++ b/test/unit/bootloader/config/base_test.py @@ -397,7 +397,7 @@ def mount_managers_effect(**args): 'volume_options': 'subvol=@/boot/grub2', 'volume_device': 'device' } - } + }, root_volume_name='root' ) assert mock_MountManager.call_args_list == [ call(device='rootdev'), @@ -409,7 +409,9 @@ def mount_managers_effect(**args): call(device='/proc', mountpoint='root_mount_point/proc'), call(device='/sys', mountpoint='root_mount_point/sys') ] - root_mount.mount.assert_called_once_with() + root_mount.mount.assert_called_once_with( + options=['subvol=root'] + ) boot_mount.mount.assert_called_once_with() efi_mount.mount.assert_called_once_with() volume_mount.mount.assert_called_once_with( diff --git a/test/unit/bootloader/config/grub2_test.py b/test/unit/bootloader/config/grub2_test.py index 619e75fadb2..5ced6f250ca 100644 --- a/test/unit/bootloader/config/grub2_test.py +++ b/test/unit/bootloader/config/grub2_test.py @@ -955,7 +955,7 @@ def open_file(filename, mode=None): } ) mock_mount_system.assert_called_once_with( - 'rootdev', 'bootdev', None, None + 'rootdev', 'bootdev', None, None, None ) assert mock_Command_run.call_args_list == [ call( diff --git a/test/unit/bootloader/install/grub2_test.py b/test/unit/bootloader/install/grub2_test.py index 8dbaea70018..6b526bb001e 100644 --- a/test/unit/bootloader/install/grub2_test.py +++ b/test/unit/bootloader/install/grub2_test.py @@ -29,6 +29,7 @@ def setup(self): 'root_device': '/dev/mapper/loop0p1', 'efi_device': '/dev/mapper/loop0p3', 'prep_device': '/dev/mapper/loop0p2', + 'system_root_volume': 'root', 'system_volumes': {'boot/grub2': { 'volume_options': 'subvol=@/boot/grub2', 'volume_device': 'device' @@ -167,7 +168,9 @@ def side_effect(device, mountpoint=None): mock_mount_manager.side_effect = side_effect self.bootloader.install() - self.bootloader.root_mount.mount.assert_called_once_with() + self.bootloader.root_mount.mount.assert_called_once_with( + options=['subvol=root'] + ) self.bootloader.boot_mount.mount.assert_called_once_with() mock_glob.assert_called_once_with( 'tmp_root/boot/*/grubenv' @@ -213,7 +216,9 @@ def side_effect(device, mountpoint=None): mock_mount_manager.side_effect = side_effect self.bootloader.install() - self.bootloader.root_mount.mount.assert_called_once_with() + self.bootloader.root_mount.mount.assert_called_once_with( + options=['subvol=root'] + ) self.bootloader.boot_mount.mount.assert_called_once_with() assert mock_command.call_args_list == [ call( @@ -258,7 +263,9 @@ def side_effect(device, mountpoint=None): mock_mount_manager.side_effect = side_effect self.bootloader.install() - self.bootloader.root_mount.mount.assert_called_once_with() + self.bootloader.root_mount.mount.assert_called_once_with( + options=['subvol=root'] + ) self.bootloader.boot_mount.mount.assert_called_once_with() mock_wipe.assert_called_once_with( 'tmp_root/boot/grub2/grubenv' @@ -297,7 +304,9 @@ def side_effect(device, mountpoint=None): mock_mount_manager.side_effect = side_effect self.bootloader.install() - self.bootloader.root_mount.mount.assert_called_once_with() + self.bootloader.root_mount.mount.assert_called_once_with( + options=['subvol=root'] + ) self.bootloader.boot_mount.mount.assert_called_once_with() mock_wipe.assert_called_once_with( 'tmp_root/boot/grub2/grubenv' @@ -338,7 +347,9 @@ def side_effect(device, mountpoint=None): self.bootloader.target_removable = True self.bootloader.install() - self.root_mount.mount.assert_called_once_with() + self.root_mount.mount.assert_called_once_with( + options=['subvol=root'] + ) self.volume_mount.mount.assert_called_once_with( options=['subvol=@/boot/grub2'] ) @@ -398,7 +409,9 @@ def side_effect(device, mountpoint=None): '/usr/sbin/grub2-install' ]) ] - self.root_mount.mount.assert_called_once_with() + self.root_mount.mount.assert_called_once_with( + options=['subvol=root'] + ) self.volume_mount.mount.assert_called_once_with( options=['subvol=@/boot/grub2'] ) @@ -421,7 +434,9 @@ def side_effect(device, mountpoint=None): mock_mount_manager.side_effect = side_effect self.bootloader.secure_boot_install() - self.root_mount.mount.assert_called_once_with() + self.root_mount.mount.assert_called_once_with( + options=['subvol=root'] + ) self.volume_mount.mount.assert_called_once_with( options=['subvol=@/boot/grub2'] ) From fcf6b6e64e7f7ec62c1d5c62d763252adcfd619e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20Sch=C3=A4fer?= Date: Mon, 31 Jul 2023 12:53:33 +0200 Subject: [PATCH 12/12] Pass rootflags if no default volume is set In case of btrfs and if btrfs_set_default_volume is explicitly switched off, we create the correct rootflags= kernel cmdline entry to tell the system about the root volume for booting --- .../fedora/test-image-live-disk/appliance.kiwi | 2 +- kiwi/builder/disk.py | 18 ++++++++++++++---- test/unit/builder/disk_test.py | 1 + 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/build-tests/x86/fedora/test-image-live-disk/appliance.kiwi b/build-tests/x86/fedora/test-image-live-disk/appliance.kiwi index 02f0f3fb697..d77e7b146e9 100644 --- a/build-tests/x86/fedora/test-image-live-disk/appliance.kiwi +++ b/build-tests/x86/fedora/test-image-live-disk/appliance.kiwi @@ -30,7 +30,7 @@ - + diff --git a/kiwi/builder/disk.py b/kiwi/builder/disk.py index b3bb4d0ad13..11b4dda7296 100644 --- a/kiwi/builder/disk.py +++ b/kiwi/builder/disk.py @@ -148,6 +148,8 @@ def __init__( self.target_removable = xml_state.build_type.get_target_removable() self.root_filesystem_is_multipath = \ xml_state.get_oemconfig_oem_multipath_scan() + self.btrfs_set_default_volume = \ + xml_state.build_type.get_btrfs_set_default_volume() self.oem_systemsize = xml_state.get_oemconfig_oem_systemsize() self.oem_resize = xml_state.get_oemconfig_oem_resize() self.disk_resize_requested = \ @@ -421,8 +423,7 @@ def create_disk(self) -> Result: self.xml_state.build_type. get_btrfs_root_is_subvolume(), 'set_default_volume': - self.xml_state.build_type. - get_btrfs_set_default_volume(), + self.btrfs_set_default_volume, 'quota_groups': self.xml_state.build_type.get_btrfs_quota_groups(), 'resize_on_boot': @@ -529,7 +530,9 @@ def create_disk(self) -> Result: # create second stage metadata to system image self._copy_first_boot_files_to_system_image() - self._write_bootloader_meta_data_to_system_image(device_map, disk) + self._write_bootloader_meta_data_to_system_image( + device_map, disk, system + ) self.mbrid.write_to_disk( disk.storage_provider @@ -1247,13 +1250,20 @@ def _write_recovery_metadata_to_boot_image(self) -> None: ) def _write_bootloader_meta_data_to_system_image( - self, device_map: Dict, disk: Disk + self, device_map: Dict, disk: Disk, system: Any ) -> None: if self.bootloader != 'custom': log.info('Creating %s bootloader configuration', self.bootloader) boot_options = [] if self.mdraid: boot_options.append('rd.auto') + if self.volume_manager_name \ + and self.volume_manager_name == 'btrfs' \ + and self.btrfs_set_default_volume is False \ + and system.get_root_volume_name() != '/': + boot_options.append( + f'rootflags=subvol={system.get_root_volume_name()}' + ) ro_device = device_map.get('readonly') root_device = device_map['root'] boot_device = root_device diff --git a/test/unit/builder/disk_test.py b/test/unit/builder/disk_test.py index c38e035d90a..5ce9cd1fce8 100644 --- a/test/unit/builder/disk_test.py +++ b/test/unit/builder/disk_test.py @@ -1224,6 +1224,7 @@ def test_create_disk_btrfs_managed_root( filesystem = Mock() mock_fs.return_value = filesystem self.disk_builder.volume_manager_name = 'btrfs' + self.disk_builder.btrfs_set_default_volume = False with patch('builtins.open'): self.disk_builder.create_disk()