From c19984ba3740958af57c0078ceb31fda4bc0ed33 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Tue, 23 Feb 2016 10:43:57 +0000 Subject: [PATCH] Add support for choosing the disk label This patch is adding support for choosing the disk label that will be used when partitoning the disk. Theoretically [0] GPT is compatible with old BIOS (it maintains a "protected MBR") allowing partitions > 2TB for BIOS when used. And EFI can boot from MBR, so no big deal. If the disk label parameter is not specified, Ironic will take the decision on which disk label it should used based on the boot mode (uefi or bios), the approach taken by the patch is quite simple: If the boot mode is UEFI we use GPT, otherwise we use MBR. [0] http://www.rodsbooks.com/gdisk/bios.html Partial-Bug: #1548788 Change-Id: I307315b1b32c9bf0babd7e42d4e9bc2030884980 --- ironic_lib/disk_utils.py | 22 +++++++--- ironic_lib/tests/test_disk_utils.py | 65 +++++++++++++++++++++++------ 2 files changed, 68 insertions(+), 19 deletions(-) diff --git a/ironic_lib/disk_utils.py b/ironic_lib/disk_utils.py index 367dbdb..abb2c32 100644 --- a/ironic_lib/disk_utils.py +++ b/ironic_lib/disk_utils.py @@ -122,7 +122,8 @@ def get_disk_identifier(dev): def make_partitions(dev, root_mb, swap_mb, ephemeral_mb, configdrive_mb, node_uuid, commit=True, - boot_option="netboot", boot_mode="bios"): + boot_option="netboot", boot_mode="bios", + disk_label=None): """Partition the disk device. Create partitions for root, swap, ephemeral and configdrive on a @@ -140,6 +141,9 @@ def make_partitions(dev, root_mb, swap_mb, ephemeral_mb, :param boot_option: Can be "local" or "netboot". "netboot" by default. :param boot_mode: Can be "bios" or "uefi". "bios" by default. :param node_uuid: Node's uuid. Used for logging. + :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. :returns: A dictionary containing the partition type as Key and partition path as Value for the partitions created by this method. @@ -150,16 +154,18 @@ def make_partitions(dev, root_mb, swap_mb, ephemeral_mb, part_template = dev + '-part%d' part_dict = {} + if disk_label is None: + disk_label = 'gpt' if boot_mode == 'uefi' else 'msdos' + + dp = disk_partitioner.DiskPartitioner(dev, disk_label=disk_label) + # For uefi localboot, switch partition table to gpt and create the efi # system partition as the first partition. if boot_mode == "uefi" and boot_option == "local": - dp = disk_partitioner.DiskPartitioner(dev, disk_label="gpt") part_num = dp.add_partition(CONF.disk_utils.efi_system_partition_size, fs_type='fat32', bootable=True) part_dict['efi system partition'] = part_template % part_num - else: - dp = disk_partitioner.DiskPartitioner(dev) if ephemeral_mb: LOG.debug("Add ephemeral partition (%(size)d MB) to device: %(dev)s " @@ -394,7 +400,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): + tempdir=None, disk_label=None): """Create partitions and copy an image to the root partition. :param dev: Path for the device to work on. @@ -414,6 +420,9 @@ def work_on_disk(dev, root_mb, swap_mb, ephemeral_mb, ephemeral_format, :param boot_option: Can be "local" or "netboot". "netboot" by default. :param boot_mode: Can be "bios" or "uefi". "bios" by default. :param tempdir: A temporary directory + :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. :returns: a dictionary containing the following keys: 'root uuid': UUID of root partition 'efi system partition uuid': UUID of the uefi system partition @@ -441,7 +450,8 @@ def work_on_disk(dev, root_mb, swap_mb, ephemeral_mb, ephemeral_format, configdrive_mb, node_uuid, commit=commit, boot_option=boot_option, - boot_mode=boot_mode) + boot_mode=boot_mode, + disk_label=disk_label) LOG.info(_LI("Successfully completed the disk device" " %(dev)s partitioning for node %(node)s"), {'dev': dev, "node": node_uuid}) diff --git a/ironic_lib/tests/test_disk_utils.py b/ironic_lib/tests/test_disk_utils.py index 9e102f4..bfdf3a0 100644 --- a/ironic_lib/tests/test_disk_utils.py +++ b/ironic_lib/tests/test_disk_utils.py @@ -115,7 +115,8 @@ class WorkOnDiskTestCase(test_base.BaseTestCase): self.configdrive_mb, self.node_uuid, commit=True, boot_option="netboot", - boot_mode="bios") + boot_mode="bios", + disk_label=None) def test_no_swap_partition(self): self.mock_ibd.side_effect = iter([True, False]) @@ -132,7 +133,8 @@ class WorkOnDiskTestCase(test_base.BaseTestCase): self.configdrive_mb, self.node_uuid, commit=True, boot_option="netboot", - boot_mode="bios") + boot_mode="bios", + disk_label=None) def test_no_ephemeral_partition(self): ephemeral_part = '/dev/fake-part1' @@ -158,7 +160,8 @@ class WorkOnDiskTestCase(test_base.BaseTestCase): self.configdrive_mb, self.node_uuid, commit=True, boot_option="netboot", - boot_mode="bios") + boot_mode="bios", + disk_label=None) @mock.patch.object(utils, 'unlink_without_raise') @mock.patch.object(disk_utils, '_get_configdrive') @@ -190,9 +193,40 @@ class WorkOnDiskTestCase(test_base.BaseTestCase): configdrive_mb, self.node_uuid, commit=True, boot_option="netboot", - boot_mode="bios") + boot_mode="bios", + disk_label=None) mock_unlink.assert_called_once_with('fake-path') + @mock.patch.object(utils, 'mkfs', lambda *_: None) + @mock.patch.object(disk_utils, 'block_uuid', lambda p: 'uuid') + @mock.patch.object(disk_utils, 'populate_image', lambda *_: None) + def test_gpt_disk_label(self): + ephemeral_part = '/dev/fake-part1' + swap_part = '/dev/fake-part2' + root_part = '/dev/fake-part3' + ephemeral_mb = 256 + ephemeral_format = 'exttest' + + self.mock_mp.return_value = {'ephemeral': ephemeral_part, + 'swap': swap_part, + 'root': root_part} + self.mock_ibd.return_value = True + calls = [mock.call(root_part), + mock.call(swap_part), + mock.call(ephemeral_part)] + disk_utils.work_on_disk(self.dev, self.root_mb, + self.swap_mb, ephemeral_mb, ephemeral_format, + self.image_path, self.node_uuid, + disk_label='gpt') + self.assertEqual(self.mock_ibd.call_args_list, calls) + self.mock_mp.assert_called_once_with(self.dev, self.root_mb, + self.swap_mb, ephemeral_mb, + self.configdrive_mb, + self.node_uuid, commit=True, + boot_option="netboot", + boot_mode="bios", + disk_label='gpt') + @mock.patch.object(utils, 'execute') class MakePartitionsTestCase(test_base.BaseTestCase): @@ -206,26 +240,27 @@ class MakePartitionsTestCase(test_base.BaseTestCase): self.configdrive_mb = 0 self.node_uuid = "12345678-1234-1234-1234-1234567890abcxyz" - def _get_parted_cmd(self, dev): + def _get_parted_cmd(self, dev, label=None): + if label is None: + label = 'msdos' + return ['parted', '-a', 'optimal', '-s', dev, - '--', 'unit', 'MiB', 'mklabel', 'msdos'] + '--', 'unit', 'MiB', 'mklabel', label] - def _get_parted_cmd_uefi(self, dev): - return ['parted', '-a', 'optimal', '-s', - dev, '--', 'unit', 'MiB', 'mklabel', 'gpt'] - - def _test_make_partitions(self, mock_exc, boot_option): + def _test_make_partitions(self, mock_exc, boot_option, disk_label=None): mock_exc.return_value = (None, None) 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) + self.node_uuid, boot_option=boot_option, + disk_label=disk_label) expected_mkpart = ['mkpart', 'primary', 'linux-swap', '1', '513', 'mkpart', 'primary', '', '513', '1537'] if boot_option == "local": expected_mkpart.extend(['set', '2', 'boot', 'on']) self.dev = 'fake-dev' - parted_cmd = self._get_parted_cmd(self.dev) + expected_mkpart + parted_cmd = (self._get_parted_cmd(self.dev, disk_label) + + expected_mkpart) parted_call = mock.call(*parted_cmd, use_standard_locale=True, run_as_root=True, check_exit_code=[0]) fuser_cmd = ['fuser', 'fake-dev'] @@ -239,6 +274,10 @@ class MakePartitionsTestCase(test_base.BaseTestCase): def test_make_partitions_local_boot(self, mock_exc): self._test_make_partitions(mock_exc, boot_option="local") + def test_make_partitions_disk_label_gpt(self, mock_exc): + self._test_make_partitions(mock_exc, boot_option="netboot", + disk_label="gpt") + def test_make_partitions_with_ephemeral(self, mock_exc): self.ephemeral_mb = 2048 expected_mkpart = ['mkpart', 'primary', '', '1', '2049',