From 2b540a00bb9f16f8e6341e4089582f21c3160ced Mon Sep 17 00:00:00 2001 From: Michael Turek Date: Wed, 16 May 2018 15:52:07 +0000 Subject: [PATCH] Add logic to create PReP partition for ppc64* arch When booting a partition image localy, ppc64* expects an 8 MiB partition, with the prep and boot flags set, containing the bootloader [0]. This patch adds logic to ironic-lib to create the parition with these flags when the node has a ppc64* arch. [0] https://www.ibm.com/developerworks/community/wikis/home?lang=en#!/wiki/W51a7ffcf4dfd_4b40_9d82_446ebc23c550/page/PowerLinux%20Boot%20howto Change-Id: I8f9748dd58146bfb2411c229b02969e0faf18222 Story: #1749057 Task: #22998 --- ironic_lib/disk_partitioner.py | 10 ++- ironic_lib/disk_utils.py | 51 ++++++++++--- ironic_lib/tests/test_disk_partitioner.py | 23 +++++- ironic_lib/tests/test_disk_utils.py | 92 +++++++++++++++++++---- 4 files changed, 147 insertions(+), 29 deletions(-) diff --git a/ironic_lib/disk_partitioner.py b/ironic_lib/disk_partitioner.py index 84a3982a..fdb5cd7e 100644 --- a/ironic_lib/disk_partitioner.py +++ b/ironic_lib/disk_partitioner.py @@ -74,7 +74,7 @@ class DiskPartitioner(object): use_standard_locale=True, run_as_root=True) def add_partition(self, size, part_type='primary', fs_type='', - boot_flag=None): + boot_flag=None, extra_flags=None): """Add a partition. :param size: The size of the partition in MiB. @@ -87,13 +87,16 @@ class DiskPartitioner(object): :param boot_flag: Boot flag that needs to be configured on the partition. Ignored if None. It can take values 'bios_grub', 'boot'. + :param extra_flags: List of flags to set on the partition. Ignored + if None. :returns: The partition number. """ self._partitions.append({'size': size, 'type': part_type, 'fs_type': fs_type, - 'boot_flag': boot_flag}) + 'boot_flag': boot_flag, + 'extra_flags': extra_flags}) return len(self._partitions) def get_partitions(self): @@ -118,6 +121,9 @@ class DiskPartitioner(object): str(start), str(end)]) if part['boot_flag']: cmd_args.extend(['set', str(num), part['boot_flag'], 'on']) + if part['extra_flags']: + for flag in part['extra_flags']: + cmd_args.extend(['set', str(num), flag, 'on']) start = end self._exec(*cmd_args) diff --git a/ironic_lib/disk_utils.py b/ironic_lib/disk_utils.py index 36558913..dc84bb13 100644 --- a/ironic_lib/disk_utils.py +++ b/ironic_lib/disk_utils.py @@ -172,12 +172,13 @@ def is_nvme_device(dev): def make_partitions(dev, root_mb, swap_mb, ephemeral_mb, configdrive_mb, node_uuid, commit=True, boot_option="netboot", boot_mode="bios", - disk_label=None): + disk_label=None, cpu_arch=""): """Partition the disk device. Create partitions for root, swap, ephemeral and configdrive on a disk device. + :param dev: Path for the device to work on. :param root_mb: Size of the root partition in mebibytes (MiB). :param swap_mb: Size of the swap partition in mebibytes (MiB). If 0, no partition will be created. @@ -193,6 +194,11 @@ def make_partitions(dev, root_mb, swap_mb, ephemeral_mb, :param disk_label: The disk label to be used when creating the partition table. Valid values are: "msdos", "gpt" or None; If None Ironic will figure it out according to the boot_mode parameter. + :param cpu_arch: Architecture of the node the disk device belongs to. + When using the default value of None, no architecture specific + steps will be taken. This default should be used for x86_64. When + set to ppc64*, architecture specific steps are taken for booting a + partition image locally. :returns: A dictionary containing the partition type as Key and partition path as Value for the partitions created by this method. @@ -227,11 +233,25 @@ def make_partitions(dev, root_mb, swap_mb, ephemeral_mb, boot_flag='boot') part_dict['efi system partition'] = part_template % part_num - if boot_mode == "bios" and boot_option == "local" and disk_label == "gpt": + if (boot_mode == "bios" and boot_option == "local" and disk_label == "gpt" + and not cpu_arch.startswith('ppc64')): part_num = dp.add_partition(CONF.disk_utils.bios_boot_partition_size, boot_flag='bios_grub') part_dict['BIOS Boot partition'] = part_template % part_num + # NOTE(mjturek): With ppc64* nodes, partition images are expected to have + # a PrEP partition at the start of the disk. This is an 8 MiB partition + # with the boot and prep flags set. The bootloader should be installed + # here. + if (cpu_arch.startswith("ppc64") and boot_mode == "bios" and + boot_option == "local"): + LOG.debug("Add PReP boot partition (8 MB) to device: " + "%(dev)s for node %(node)s", + {'dev': dev, 'node': node_uuid}) + boot_flag = 'boot' if disk_label == 'msdos' else None + part_num = dp.add_partition(8, part_type='primary', + boot_flag=boot_flag, extra_flags=['prep']) + part_dict['PReP Boot partition'] = part_template % part_num if ephemeral_mb: LOG.debug("Add ephemeral partition (%(size)d MB) to device: %(dev)s " "for node %(node)s", @@ -258,10 +278,10 @@ def make_partitions(dev, root_mb, swap_mb, ephemeral_mb, "for node %(node)s", {'dev': dev, 'size': root_mb, 'node': node_uuid}) - boot_val = None - if (boot_mode == "bios" and boot_option == "local" and - disk_label == "msdos"): - boot_val = 'boot' + boot_val = 'boot' if (not cpu_arch.startswith("ppc64") + and boot_mode == "bios" + and boot_option == "local" + and disk_label == "msdos") else None part_num = dp.add_partition(root_mb, boot_flag=boot_val) @@ -458,7 +478,7 @@ def _get_configdrive(configdrive, node_uuid, tempdir=None): def work_on_disk(dev, root_mb, swap_mb, ephemeral_mb, ephemeral_format, image_path, node_uuid, preserve_ephemeral=False, configdrive=None, boot_option="netboot", boot_mode="bios", - tempdir=None, disk_label=None): + tempdir=None, disk_label=None, cpu_arch=""): """Create partitions and copy an image to the root partition. :param dev: Path for the device to work on. @@ -481,6 +501,11 @@ def work_on_disk(dev, root_mb, swap_mb, ephemeral_mb, ephemeral_format, :param disk_label: The disk label to be used when creating the partition table. Valid values are: "msdos", "gpt" or None; If None Ironic will figure it out according to the boot_mode parameter. + :param cpu_arch: Architecture of the node the disk device belongs to. + When using the default value of None, no architecture specific + steps will be taken. This default should be used for x86_64. When + set to ppc64*, architecture specific steps are taken for booting a + partition image locally. :returns: a dictionary containing the following keys: 'root uuid': UUID of root partition 'efi system partition uuid': UUID of the uefi system partition @@ -509,7 +534,8 @@ def work_on_disk(dev, root_mb, swap_mb, ephemeral_mb, ephemeral_format, commit=commit, boot_option=boot_option, boot_mode=boot_mode, - disk_label=disk_label) + disk_label=disk_label, + cpu_arch=cpu_arch) LOG.info("Successfully completed the disk device" " %(dev)s partitioning for node %(node)s", {'dev': dev, "node": node_uuid}) @@ -524,7 +550,7 @@ def work_on_disk(dev, root_mb, swap_mb, ephemeral_mb, ephemeral_format, _("Root device '%s' not found") % root_part) for part in ('swap', 'ephemeral', 'configdrive', - 'efi system partition'): + 'efi system partition', 'PReP Boot partition'): part_device = part_dict.get(part) LOG.debug("Checking for %(part)s device (%(dev)s) on node " "%(node)s.", {'part': part, 'dev': part_device, @@ -572,9 +598,14 @@ def work_on_disk(dev, root_mb, swap_mb, ephemeral_mb, ephemeral_format, uuids_to_return = { 'root uuid': root_part, - 'efi system partition uuid': part_dict.get('efi system partition') + 'efi system partition uuid': part_dict.get('efi system partition'), } + if cpu_arch.startswith('ppc'): + uuids_to_return[ + 'PReP Boot partition uuid' + ] = part_dict.get('PReP Boot partition') + try: for part, part_dev in uuids_to_return.items(): if part_dev: diff --git a/ironic_lib/tests/test_disk_partitioner.py b/ironic_lib/tests/test_disk_partitioner.py index c63b6a84..3cf296a2 100644 --- a/ironic_lib/tests/test_disk_partitioner.py +++ b/ironic_lib/tests/test_disk_partitioner.py @@ -32,18 +32,22 @@ class DiskPartitionerTestCase(base.IronicLibTestCase): dp.add_partition(2048, boot_flag='boot') dp.add_partition(2048, boot_flag='bios_grub') expected = [(1, {'boot_flag': None, + 'extra_flags': None, 'fs_type': '', 'type': 'primary', 'size': 1024}), (2, {'boot_flag': None, + 'extra_flags': None, 'fs_type': 'linux-swap', 'type': 'primary', 'size': 512}), (3, {'boot_flag': 'boot', + 'extra_flags': None, 'fs_type': '', 'type': 'primary', 'size': 2048}), (4, {'boot_flag': 'bios_grub', + 'extra_flags': None, 'fs_type': '', 'type': 'primary', 'size': 2048})] @@ -57,14 +61,22 @@ class DiskPartitionerTestCase(base.IronicLibTestCase): def test_commit(self, mock_utils_exc, mock_disk_partitioner_exec): dp = disk_partitioner.DiskPartitioner('/dev/fake') fake_parts = [(1, {'boot_flag': None, + 'extra_flags': None, 'fs_type': 'fake-fs-type', 'type': 'fake-type', 'size': 1}), (2, {'boot_flag': 'boot', + 'extra_flags': None, 'fs_type': 'fake-fs-type', 'type': 'fake-type', 'size': 1}), (3, {'boot_flag': 'bios_grub', + 'extra_flags': None, + 'fs_type': 'fake-fs-type', + 'type': 'fake-type', + 'size': 1}), + (4, {'boot_flag': 'boot', + 'extra_flags': ['prep', 'fake-flag'], 'fs_type': 'fake-fs-type', 'type': 'fake-type', 'size': 1})] @@ -79,7 +91,10 @@ class DiskPartitionerTestCase(base.IronicLibTestCase): 'mkpart', 'fake-type', 'fake-fs-type', '2', '3', 'set', '2', 'boot', 'on', 'mkpart', 'fake-type', 'fake-fs-type', '3', '4', - 'set', '3', 'bios_grub', 'on') + 'set', '3', 'bios_grub', 'on', + 'mkpart', 'fake-type', 'fake-fs-type', '4', '5', + 'set', '4', 'boot', 'on', 'set', '4', 'prep', 'on', + 'set', '4', 'fake-flag', 'on') mock_utils_exc.assert_called_once_with( 'fuser', '/dev/fake', run_as_root=True, check_exit_code=[0, 1]) @@ -92,10 +107,12 @@ class DiskPartitionerTestCase(base.IronicLibTestCase): mock_disk_partitioner_exec): dp = disk_partitioner.DiskPartitioner('/dev/fake') fake_parts = [(1, {'boot_flag': None, + 'extra_flags': None, 'fs_type': 'fake-fs-type', 'type': 'fake-type', 'size': 1}), (2, {'boot_flag': 'boot', + 'extra_flags': None, 'fs_type': 'fake-fs-type', 'type': 'fake-type', 'size': 1})] @@ -125,10 +142,12 @@ class DiskPartitionerTestCase(base.IronicLibTestCase): mock_disk_partitioner_exec): dp = disk_partitioner.DiskPartitioner('/dev/fake') fake_parts = [(1, {'boot_flag': None, + 'extra_flags': None, 'fs_type': 'fake-fs-type', 'type': 'fake-type', 'size': 1}), (2, {'boot_flag': 'boot', + 'extra_flags': None, 'fs_type': 'fake-fs-type', 'type': 'fake-type', 'size': 1})] @@ -159,10 +178,12 @@ class DiskPartitionerTestCase(base.IronicLibTestCase): mock_disk_partitioner_exec): dp = disk_partitioner.DiskPartitioner('/dev/fake') fake_parts = [(1, {'boot_flag': None, + 'extra_flags': None, 'fs_type': 'fake-fs-type', 'type': 'fake-type', 'size': 1}), (2, {'boot_flag': 'boot', + 'extra_flags': None, 'fs_type': 'fake-fs-type', 'type': 'fake-type', 'size': 1})] diff --git a/ironic_lib/tests/test_disk_utils.py b/ironic_lib/tests/test_disk_utils.py index f9b9c7a1..f889923a 100644 --- a/ironic_lib/tests/test_disk_utils.py +++ b/ironic_lib/tests/test_disk_utils.py @@ -115,7 +115,8 @@ class WorkOnDiskTestCase(base.IronicLibTestCase): self.node_uuid, commit=True, boot_option="netboot", boot_mode="bios", - disk_label=None) + disk_label=None, + cpu_arch="") def test_no_swap_partition(self): self.mock_ibd.side_effect = iter([True, False]) @@ -133,7 +134,8 @@ class WorkOnDiskTestCase(base.IronicLibTestCase): self.node_uuid, commit=True, boot_option="netboot", boot_mode="bios", - disk_label=None) + disk_label=None, + cpu_arch="") def test_no_ephemeral_partition(self): ephemeral_part = '/dev/fake-part1' @@ -160,7 +162,8 @@ class WorkOnDiskTestCase(base.IronicLibTestCase): self.node_uuid, commit=True, boot_option="netboot", boot_mode="bios", - disk_label=None) + disk_label=None, + cpu_arch="") @mock.patch.object(utils, 'unlink_without_raise', autospec=True) @mock.patch.object(disk_utils, '_get_configdrive', autospec=True) @@ -193,7 +196,8 @@ class WorkOnDiskTestCase(base.IronicLibTestCase): commit=True, boot_option="netboot", boot_mode="bios", - disk_label=None) + disk_label=None, + cpu_arch="") mock_unlink.assert_called_once_with('fake-path') @mock.patch.object(utils, 'mkfs', lambda fs, path, label=None: None) @@ -224,7 +228,8 @@ class WorkOnDiskTestCase(base.IronicLibTestCase): self.node_uuid, commit=True, boot_option="netboot", boot_mode="bios", - disk_label='gpt') + disk_label='gpt', + cpu_arch="") @mock.patch.object(disk_utils, 'block_uuid', autospec=True) @mock.patch.object(disk_utils, 'populate_image', autospec=True) @@ -252,7 +257,8 @@ class WorkOnDiskTestCase(base.IronicLibTestCase): self.node_uuid, commit=True, boot_option="local", boot_mode="uefi", - disk_label=None) + disk_label=None, + cpu_arch="") self.assertEqual(self.mock_ibd.call_args_list, mock_ibd_calls) mock_mkfs.assert_called_once_with(fs='vfat', path=efi_part, label='efi-part') @@ -288,7 +294,38 @@ class WorkOnDiskTestCase(base.IronicLibTestCase): self.node_uuid, commit=False, boot_option="netboot", boot_mode="bios", - disk_label=None) + disk_label=None, + cpu_arch="") + self.assertFalse(mock_mkfs.called) + + @mock.patch.object(disk_utils, 'block_uuid', autospec=True) + @mock.patch.object(disk_utils, 'populate_image', autospec=True) + @mock.patch.object(utils, 'mkfs', autospec=True) + def test_ppc64le_prep_part(self, mock_mkfs, mock_populate_image, + mock_block_uuid): + """Test that PReP partition uuid is returned.""" + prep_part = '/dev/fake-part1' + root_part = '/dev/fake-part2' + + self.mock_mp.return_value = {'PReP Boot partition': prep_part, + 'root': root_part} + self.mock_ibd.return_value = True + calls = [mock.call(root_part), + mock.call(prep_part)] + disk_utils.work_on_disk(self.dev, self.root_mb, + self.swap_mb, self.ephemeral_mb, + self.ephemeral_format, self.image_path, + self.node_uuid, boot_option="local", + cpu_arch='ppc64le') + self.assertEqual(self.mock_ibd.call_args_list, calls) + self.mock_mp.assert_called_once_with(self.dev, self.root_mb, + self.swap_mb, self.ephemeral_mb, + self.configdrive_mb, + self.node_uuid, commit=True, + boot_option="local", + boot_mode="bios", + disk_label=None, + cpu_arch="ppc64le") self.assertFalse(mock_mkfs.called) @@ -314,12 +351,13 @@ class MakePartitionsTestCase(base.IronicLibTestCase): '--', 'unit', 'MiB', 'mklabel', label] def _test_make_partitions(self, mock_exc, boot_option, boot_mode='bios', - disk_label=None): + disk_label=None, cpu_arch=""): mock_exc.return_value = ('', '') disk_utils.make_partitions(self.dev, self.root_mb, self.swap_mb, self.ephemeral_mb, self.configdrive_mb, self.node_uuid, boot_option=boot_option, - boot_mode=boot_mode, disk_label=disk_label) + boot_mode=boot_mode, disk_label=disk_label, + cpu_arch=cpu_arch) if boot_option == "local" and boot_mode == "uefi": add_efi_sz = lambda x: str(x + self.efi_size) @@ -333,14 +371,28 @@ class MakePartitionsTestCase(base.IronicLibTestCase): else: if boot_option == "local": if disk_label == "gpt": - add_bios_sz = lambda x: str(x + self.bios_size) - expected_mkpart = ['mkpart', 'primary', '', '1', - add_bios_sz(1), - 'set', '1', 'bios_grub', 'on', + if cpu_arch.startswith('ppc64'): + expected_mkpart = ['mkpart', 'primary', '', '1', '9', + 'set', '1', 'prep', 'on', + 'mkpart', 'primary', 'linux-swap', + '9', '521', 'mkpart', 'primary', + '', '521', '1545'] + else: + add_bios_sz = lambda x: str(x + self.bios_size) + expected_mkpart = ['mkpart', 'primary', '', '1', + add_bios_sz(1), + 'set', '1', 'bios_grub', 'on', + 'mkpart', 'primary', 'linux-swap', + add_bios_sz(1), add_bios_sz(513), + 'mkpart', 'primary', '', + add_bios_sz(513), add_bios_sz(1537)] + elif cpu_arch.startswith('ppc64'): + expected_mkpart = ['mkpart', 'primary', '', '1', '9', + 'set', '1', 'boot', 'on', + 'set', '1', 'prep', 'on', 'mkpart', 'primary', 'linux-swap', - add_bios_sz(1), add_bios_sz(513), - 'mkpart', 'primary', '', - add_bios_sz(513), add_bios_sz(1537)] + '9', '521', 'mkpart', 'primary', + '', '521', '1545'] else: expected_mkpart = ['mkpart', 'primary', 'linux-swap', '1', '513', 'mkpart', 'primary', '', '513', @@ -377,6 +429,14 @@ class MakePartitionsTestCase(base.IronicLibTestCase): self._test_make_partitions(mock_exc, boot_option="netboot", disk_label="gpt") + def test_make_partitions_mbr_with_prep(self, mock_exc): + self._test_make_partitions(mock_exc, boot_option="local", + disk_label="msdos", cpu_arch="ppc64le") + + def test_make_partitions_gpt_with_prep(self, mock_exc): + self._test_make_partitions(mock_exc, boot_option="local", + disk_label="gpt", cpu_arch="ppc64le") + def test_make_partitions_with_ephemeral(self, mock_exc): self.ephemeral_mb = 2048 expected_mkpart = ['mkpart', 'primary', '', '1', '2049',