From 9a60d3853fc5b36cd3715e9419568cc9c51f5a8e Mon Sep 17 00:00:00 2001 From: Arne Wiebalck Date: Wed, 20 Jan 2021 19:03:07 +0100 Subject: [PATCH] Handle EBRs and tiny partitions when removing metadata Extended boot records (EBRs) are used in the MSDOS partitioning scheme to manage extended partitions. EBRs are small partitions themselves and Ironic will therefore treat them like any other partition during cleaning. Since EBRs are smaller than a GPT (33 sectors), dd'ing 33 sectors will fail. This patch proposes a fix to handle EBRs (and other tiny paritions) by adjusting the number of blocks writtent to be the partition size at max. Story: #2008539 Task: #41628 Change-Id: Ie9809b3e4fe2bea6b66ee95c442111942e85d618 --- ironic_lib/disk_utils.py | 23 ++++++-- ironic_lib/tests/test_disk_utils.py | 52 +++++++++++++++++++ ...rase-tiny-partitions-c408a3a4afe60d44.yaml | 6 +++ 3 files changed, 76 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/erase-tiny-partitions-c408a3a4afe60d44.yaml diff --git a/ironic_lib/disk_utils.py b/ironic_lib/disk_utils.py index a31390bb..f1cb9ac4 100644 --- a/ironic_lib/disk_utils.py +++ b/ironic_lib/disk_utils.py @@ -80,6 +80,8 @@ _PARTED_PRINT_RE = re.compile(r"^(\d+):([\d\.]+)MiB:" CONFIGDRIVE_LABEL = "config-2" MAX_CONFIG_DRIVE_SIZE_MB = 64 +GPT_SIZE_SECTORS = 33 + # Maximum disk size supported by MBR is 2TB (2 * 1024 * 1024 MB) MAX_DISK_SIZE_MB_SUPPORTED_BY_MBR = 2097152 @@ -497,13 +499,24 @@ def destroy_disk_metadata(dev, node_uuid): # erasing partition data. # This is the same bug as # https://bugs.launchpad.net/ironic-python-agent/+bug/1737556 + + # Overwrite the Primary GPT, catch very small partitions (like EBRs) dd_device = 'of=%s' % dev - gpt_backup = get_dev_block_size(dev) - 33 - dd_seek = 'seek=%i' % gpt_backup - utils.execute('dd', 'bs=512', 'if=/dev/zero', dd_device, 'count=33', + dd_count = 'count=%s' % GPT_SIZE_SECTORS + dev_size = get_dev_block_size(dev) + if dev_size < GPT_SIZE_SECTORS: + dd_count = 'count=%s' % dev_size + utils.execute('dd', 'bs=512', 'if=/dev/zero', dd_device, dd_count, run_as_root=True, use_standard_locale=True) - utils.execute('dd', 'bs=512', 'if=/dev/zero', dd_device, 'count=33', - dd_seek, run_as_root=True, use_standard_locale=True) + + # Overwrite the Secondary GPT, do this only if there could be one + if dev_size > GPT_SIZE_SECTORS: + gpt_backup = dev_size - GPT_SIZE_SECTORS + dd_seek = 'seek=%i' % gpt_backup + dd_count = 'count=%s' % GPT_SIZE_SECTORS + utils.execute('dd', 'bs=512', 'if=/dev/zero', dd_device, dd_count, + dd_seek, run_as_root=True, use_standard_locale=True) + # Go ahead and let sgdisk run as well. utils.execute('sgdisk', '-Z', dev, run_as_root=True, use_standard_locale=True) diff --git a/ironic_lib/tests/test_disk_utils.py b/ironic_lib/tests/test_disk_utils.py index d163c8e2..c8fea87b 100644 --- a/ironic_lib/tests/test_disk_utils.py +++ b/ironic_lib/tests/test_disk_utils.py @@ -800,6 +800,58 @@ class DestroyMetaDataTestCase(base.IronicLibTestCase): disk_utils.destroy_disk_metadata(self.dev, self.node_uuid) mock_exec.assert_has_calls(expected_call) + def test_destroy_disk_metadata_ebr(self, mock_exec): + expected_calls = [mock.call('wipefs', '--force', '--all', 'fake-dev', + run_as_root=True, + use_standard_locale=True), + mock.call('blockdev', '--getsz', 'fake-dev', + check_exit_code=[0], + run_as_root=True), + mock.call('dd', 'bs=512', 'if=/dev/zero', + 'of=fake-dev', 'count=2', + run_as_root=True, + use_standard_locale=True), + mock.call('sgdisk', '-Z', 'fake-dev', + run_as_root=True, + use_standard_locale=True)] + mock_exec.side_effect = iter([ + (None, None), + ('2\n', None), # an EBR is 2 sectors + (None, None), + (None, None), + (None, None), + (None, None)]) + disk_utils.destroy_disk_metadata(self.dev, self.node_uuid) + mock_exec.assert_has_calls(expected_calls) + + def test_destroy_disk_metadata_tiny_partition(self, mock_exec): + expected_calls = [mock.call('wipefs', '--force', '--all', 'fake-dev', + run_as_root=True, + use_standard_locale=True), + mock.call('blockdev', '--getsz', 'fake-dev', + check_exit_code=[0], + run_as_root=True), + mock.call('dd', 'bs=512', 'if=/dev/zero', + 'of=fake-dev', 'count=33', + run_as_root=True, + use_standard_locale=True), + mock.call('dd', 'bs=512', 'if=/dev/zero', + 'of=fake-dev', 'count=33', 'seek=9', + run_as_root=True, + use_standard_locale=True), + mock.call('sgdisk', '-Z', 'fake-dev', + run_as_root=True, + use_standard_locale=True)] + mock_exec.side_effect = iter([ + (None, None), + ('42\n', None), + (None, None), + (None, None), + (None, None), + (None, None)]) + disk_utils.destroy_disk_metadata(self.dev, self.node_uuid) + mock_exec.assert_has_calls(expected_calls) + @mock.patch.object(utils, 'execute', autospec=True) class GetDeviceBlockSizeTestCase(base.IronicLibTestCase): diff --git a/releasenotes/notes/erase-tiny-partitions-c408a3a4afe60d44.yaml b/releasenotes/notes/erase-tiny-partitions-c408a3a4afe60d44.yaml new file mode 100644 index 00000000..360e3c79 --- /dev/null +++ b/releasenotes/notes/erase-tiny-partitions-c408a3a4afe60d44.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes cleaning errors when trying to erase a GPT from a partition + which is smaller than a GPT (33 sectors). +