Fixed zipl bootloader setup for s390 images

kiwi/bootloader/config/base.py

@@ -89,7 +89,7 @@ def write_meta_data(self, root_uuid=None, boot_options='', iso_boot=False):
         pass
 
     def setup_disk_image_config(
-        self, boot_uuid, root_uuid, hypervisor, kernel, initrd, boot_options
+        self, boot_uuid, root_uuid, hypervisor, kernel, initrd, boot_options={}
     ):
         """
         Create boot config file to boot from disk.
@@ -99,9 +99,15 @@ def setup_disk_image_config(
         :param string hypervisor: hypervisor name
         :param string kernel: kernel name
         :param string initrd: initrd name
-        :param string boot_options:
-            custom options required to setup the boot. The scope
-            of the options is specified in the implementation
+        :param dict boot_options:
+            custom options dictionary required to setup the bootloader.
+            The scope of the options covers all information needed
+            to setup and configure the bootloader and gets effective
+            in the individual implementation. boot_options should
+            not be mixed up with commandline options used at boot time.
+            This information is provided from the get_*_cmdline
+            methods. The contents of the dictionary can vary between
+            bootloaders or even not be needed
 
         Implementation in specialized bootloader class required
         """

kiwi/bootloader/config/grub2.py

@@ -199,7 +199,7 @@ def setup_disk_image_config(
         :param string hypervisor: unused
         :param string kernel: unused
         :param string initrd: unused
-        :param string boot_options:
+        :param dict boot_options:
             options dictionary that has to contain the root and boot
             device and optional volume configuration. KIWI has to
             mount the system prior to run grub2-mkconfig.

kiwi/bootloader/config/zipl.py

@@ -18,7 +18,6 @@
 import platform
 import logging
 import re
-import os
 
 # project
 from kiwi.bootloader.config.base import BootLoaderConfigBase
@@ -101,29 +100,9 @@ def write(self):
             with open(config_file, 'w') as config:
                 config.write(self.config)
 
-            log.info('Moving initrd/kernel to zipl boot directory')
-            Command.run(
-                [
-                    'mv',
-                    os.sep.join(
-                        [
-                            self.boot_dir, 'boot',
-                            os.readlink(self.boot_dir + '/boot/initrd')
-                        ]
-                    ),
-                    os.sep.join(
-                        [
-                            self.boot_dir, 'boot',
-                            os.readlink(self.boot_dir + '/boot/image')
-                        ]
-                    ),
-                    self._get_zipl_boot_path()
-                ]
-            )
-
     def setup_disk_image_config(
         self, boot_uuid=None, root_uuid=None, hypervisor=None,
-        kernel=None, initrd=None, boot_options=''
+        kernel='image', initrd='initrd', boot_options={}
     ):
         """
         Create the zipl config in memory from a template suitable to
@@ -134,7 +113,7 @@ def setup_disk_image_config(
         :param string hypervisor: unused
         :param string kernel: kernel name
         :param string initrd: initrd name
-        :param string boot_options: kernel options as string
+        :param dict boot_options: unused
         """
         log.info('Creating zipl config file from template')
         parameters = {
@@ -149,10 +128,8 @@ def setup_disk_image_config(
             'title': self.quote_title(self.get_menu_entry_title()),
             'kernel_file': kernel,
             'initrd_file': initrd,
-            'boot_options': ' '.join([self.cmdline, boot_options]),
-            'failsafe_boot_options': ' '.join(
-                [self.cmdline_failsafe, boot_options]
-            )
+            'boot_options': self.cmdline,
+            'failsafe_boot_options': self.cmdline_failsafe
         }
         log.info('--> Using standard disk boot template')
         template = self.zipl.get_template(self.failsafe_boot, self.target_type)

kiwi/bootloader/install/zipl.py

@@ -15,6 +15,7 @@
 # You should have received a copy of the GNU General Public License
 # along with kiwi.  If not, see <http://www.gnu.org/licenses/>
 #
+import os
 import logging
 
 # project
@@ -77,9 +78,8 @@ def install(self):
 
         bash_command = ' '.join(
             [
-                'cd', self.boot_mount.mountpoint, '&&',
-                'zipl', '-V', '-c', self.boot_mount.mountpoint + '/config',
-                '-m', 'menu'
+                'cd', os.sep.join([self.root_dir, 'boot']), '&&',
+                'zipl', '-V', '-c', 'zipl/config', '-m', 'menu'
             ]
         )
         zipl_call = Command.run(

kiwi/builder/disk.py

@@ -992,6 +992,8 @@ def _install_bootloader(self, device_map):
         self.bootloader_config.setup_disk_image_config(
             boot_options=custom_install_arguments
         )
+        if 's390' in self.arch:
+            self.bootloader_config.write()
 
         # cleanup bootloader config resources taken prior to next steps
         del self.bootloader_config

test/unit/bootloader/config/zipl_test.py

@@ -84,16 +84,11 @@ def test_post_init_no_target_base(self, mock_machine):
             BootLoaderConfigZipl(mock.Mock(), 'root_dir')
 
     @patch('os.path.exists')
-    @patch('os.readlink')
     @patch('kiwi.bootloader.config.zipl.Path.create')
     @patch('kiwi.bootloader.config.zipl.Command.run')
     def test_write(
-        self, mock_command, mock_path, mock_readlink, mock_exists
+        self, mock_command, mock_path, mock_exists
     ):
-        mock_readlink.side_effect = [
-            'initrd_readlink_name',
-            'kernel_readlink_name'
-        ]
         mock_exists.return_value = True
 
         self.bootloader.config = 'some-data'
@@ -111,14 +106,6 @@ def test_write(
         m_open.return_value.write.assert_called_once_with(
             'some-data'
         )
-        mock_command.assert_called_once_with(
-            [
-                'mv',
-                'root_dir/boot/initrd_readlink_name',
-                'root_dir/boot/kernel_readlink_name',
-                'root_dir/boot/zipl'
-            ]
-        )
 
     def test_setup_disk_boot_images(self):
         # does nothing on s390
@@ -205,7 +192,7 @@ def side_effect(arg):
         mock_command.side_effect = side_effect
 
         self.bootloader.setup_disk_image_config(
-            kernel='kernel', initrd='initrd', boot_options='foo'
+            kernel='image', initrd='initrd'
         )
 
         self.zipl.get_template.assert_called_once_with(True, 'CDL')
@@ -215,14 +202,14 @@ def side_effect(arg):
                 'initrd_file': 'initrd',
                 'offset': 168,
                 'device': '/dev/loop0',
-                'kernel_file': 'kernel',
+                'kernel_file': 'image',
                 'title': 'image-name_(_OEM_)',
                 'geometry': '10017,15,12',
-                'boot_options': 'cmdline foo',
+                'boot_options': 'cmdline',
                 'target_type': 'CDL',
                 'boot_timeout': '200',
                 'failsafe_boot_options': 'cmdline ide=nodma apm=off '
-                'noresume edd=off nomodeset 3 foo',
+                'noresume edd=off nomodeset 3',
                 'default_boot': '1',
                 'bootpath': '.'
             }
@@ -261,7 +248,7 @@ def side_effect(arg):
         mock_command.side_effect = side_effect
 
         self.bootloader.setup_disk_image_config(
-            kernel='kernel', initrd='initrd'
+            kernel='image', initrd='initrd'
         )
 
         self.zipl.get_template.assert_called_once_with(True, 'CDL')
@@ -271,14 +258,14 @@ def side_effect(arg):
                 'initrd_file': 'initrd',
                 'offset': 129024,
                 'device': '/dev/loop0',
-                'kernel_file': 'kernel',
+                'kernel_file': 'image',
                 'title': 'image-name_(_OEM_)',
                 'geometry': '242251,256,63',
-                'boot_options': 'cmdline ',
+                'boot_options': 'cmdline',
                 'target_type': 'CDL',
                 'boot_timeout': '200',
                 'failsafe_boot_options': 'cmdline ide=nodma apm=off noresume '
-                'edd=off nomodeset 3 ',
+                'edd=off nomodeset 3',
                 'default_boot': '1',
                 'bootpath': '.'
             }

test/unit/bootloader/install/zipl_test.py

@@ -1,5 +1,5 @@
 from mock import (
-    patch, call, Mock
+    patch, Mock
 )
 from pytest import raises
 
@@ -35,17 +35,12 @@ def test_install_required(self):
         assert self.bootloader.install_required() is True
 
     @patch('kiwi.bootloader.install.zipl.Command.run')
-    def test_install(self, mock_command):
+    def test_install(self, mock_Command_run):
         self.bootloader.install()
         self.bootloader.boot_mount.mount.assert_called_once_with()
-        mock_command.call_args_list == [
-            call(
-                ['bash', '-c', 'cd tmpdir && zipl -V -c tmpdir/config -m menu']
-            ),
-            call(
-                ['umount', 'tmpdir']
-            )
-        ]
+        mock_Command_run.assert_called_once_with(
+            ['bash', '-c', 'cd root_dir/boot && zipl -V -c zipl/config -m menu']
+        )
 
     def test_destructor(self):
         self.bootloader.__del__()

test/unit/builder/disk_test.py

@@ -503,7 +503,8 @@ def test_create_disk_standard_root_with_dracut_initrd(
             'kiwi-repart'
         )
         self.boot_image_task.omit_module.assert_called_once_with('multipath')
-        assert self.boot_image_task.write_system_config_file.call_args_list == []
+        assert self.boot_image_task.write_system_config_file.call_args_list == \
+            []
 
     @patch('kiwi.builder.disk.FileSystem')
     @patch('kiwi.builder.disk.FileSystemSquashFs')
@@ -602,6 +603,7 @@ def test_create_disk_standard_root_xen_server_boot(
     def test_create_disk_standard_root_s390_boot(
         self, mock_grub_dir, mock_command, mock_fs
     ):
+        self.disk_builder.arch = 's390x'
         filesystem = mock.Mock()
         mock_fs.return_value = filesystem
         self.disk_builder.volume_manager_name = None
@@ -613,6 +615,8 @@ def test_create_disk_standard_root_s390_boot(
         with patch('builtins.open'):
             self.disk_builder.create_disk()
 
+        self.bootloader_config.write.assert_called_once_with()
+
         assert mock_fs.call_args_list[1] == call(
             'ext2', self.device_map['boot'], 'root_dir/boot/zipl/'
         )
@@ -728,11 +732,14 @@ def test_create_disk_volume_managed_root(
         volume_manager.create_volumes.assert_called_once_with('btrfs')
         volume_manager.mount_volumes.call_args_list[0].assert_called_once_with()
         volume_manager.get_fstab.assert_called_once_with(None, 'btrfs')
-        volume_manager.sync_data.assert_called_once_with([
-            'image', '.profile', '.kconfig', '.buildenv', 'var/cache/kiwi',
-            'boot/*', 'boot/.*', 'boot/efi/*', 'boot/efi/.*'
-        ])
-        volume_manager.umount_volumes.call_args_list[0].assert_called_once_with()
+        volume_manager.sync_data.assert_called_once_with(
+            [
+                'image', '.profile', '.kconfig', '.buildenv', 'var/cache/kiwi',
+                'boot/*', 'boot/.*', 'boot/efi/*', 'boot/efi/.*'
+            ]
+        )
+        volume_manager.umount_volumes.call_args_list[0].assert_called_once_with(
+        )
         self.setup.create_fstab.assert_called_once_with(
             [
                 'fstab_volume_entries',