openstack
/
ironic-lib
Code Issues Proposed changes

use wipefs to erase FS meta information 

destroy_disk_metadata should destroy a bit more data than expected if we
want to be sure grub won't conflict with remaining bits of filesystem.
grub probes the hard drive to find remaining partition or filesystem. If
it find something, it will refuse to install itself on the disk.

By using wipefs instead of dd, we not only erase the MBR and partition
table but also all trace of filesystem, raid or partition-table signatures
(magic strings). This without extra Python code.

wipefs is part of util-linux which is available on all the Linux
distribution.

References:
https://review.openstack.org/#/c/284347/
https://bugzilla.redhat.com/show_bug.cgi?id=1310883
Partial-Bug: 1550604

Change-Id: I39637e22c344703ad48fc271f6f866aa018bbdd1
This commit is contained in:
Gonéri Le Bouder 2016-02-26 11:27:54 -05:00
parent cfd7a91fa2
commit 042aa9ab5a
3 changed files with 19 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
etc/rootwrap.d/ironic-lib.filters
View File

@ -2,12 +2,17 @@
# The following commands should be used in filters for disk manipulation.
# This file should be owned by (and only-writeable by) the root user.
# NOTE:
# if you update this file, you will also need to adjust the
# ironic-lib.filters from the ironic module.
[Filters]
# ironic_lib/disk_utils.py
blkid: CommandFilter, blkid, root
blockdev: CommandFilter, blockdev, root
hexdump: CommandFilter, hexdump, root
qemu-img: CommandFilter, qemu-img, root
wipefs: CommandFilter, wipefs, root
# ironic_lib/utils.py
mkswap: CommandFilter, mkswap, root

47
ironic_lib/disk_utils.py
View File

@ -284,49 +284,20 @@ def get_dev_block_size(dev):
def destroy_disk_metadata(dev, node_uuid):
"""Destroy metadata structures on node's disk.
Ensure that node's disk appears to be blank without zeroing the entire
drive. To do this we will zero the first 18KiB to clear MBR / GPT data
and the last 18KiB to clear GPT and other metadata like LVM, veritas,
MDADM, DMRAID, etc.
Ensure that node's disk magic strings are wiped without zeroing the
entire drive. To do this we use the wipefs tool from util-linux.
:param dev: Path for the device to work on.
:param node_uuid: Node's uuid. Used for logging.
"""
# NOTE(NobodyCam): This is needed to work around bug:
# https://bugs.launchpad.net/ironic/+bug/1317647
LOG.debug("Start destroy disk metadata for node %(node)s.",
{'node': node_uuid})
try:
utils.dd('/dev/zero', dev, 'bs=512', 'count=36')
except processutils.ProcessExecutionError as err:
with excutils.save_and_reraise_exception():
LOG.error(_LE("Failed to erase beginning of disk for node "
"%(node)s. Command: %(command)s. Error: %(error)s."),
{'node': node_uuid,
'command': err.cmd,
'error': err.stderr})
# now wipe the end of the disk.
# get end of disk seek value
try:
block_sz = get_dev_block_size(dev)
except processutils.ProcessExecutionError as err:
with excutils.save_and_reraise_exception():
LOG.error(_LE("Failed to get disk block count for node %(node)s. "
"Command: %(command)s. Error: %(error)s."),
{'node': node_uuid,
'command': err.cmd,
'error': err.stderr})
else:
seek_value = block_sz - 36
try:
utils.dd('/dev/zero', dev, 'bs=512', 'count=36',
'seek=%d' % seek_value)
except processutils.ProcessExecutionError as err:
with excutils.save_and_reraise_exception():
LOG.error(_LE("Failed to erase the end of the disk on node "
"%(node)s. Command: %(command)s. "
"Error: %(error)s."),
{'node': node_uuid,
'command': err.cmd,
'error': err.stderr})
utils.execute('wipefs', '--all', dev,
run_as_root=True,
check_exit_code=[0],
use_standard_locale=True)
LOG.info(_LI("Disk metadata on %(dev)s successfully destroyed for node "
"%(node)s"), {'dev': dev, 'node': node_uuid})

32
ironic_lib/tests/test_disk_utils.py
View File

@ -276,7 +276,6 @@ class MakePartitionsTestCase(test_base.BaseTestCase):
self.assertEqual(expected_result, result)
@mock.patch.object(disk_utils, 'get_dev_block_size')
@mock.patch.object(utils, 'execute')
class DestroyMetaDataTestCase(test_base.BaseTestCase):
@ -285,39 +284,19 @@ class DestroyMetaDataTestCase(test_base.BaseTestCase):
self.dev = 'fake-dev'
self.node_uuid = "12345678-1234-1234-1234-1234567890abcxyz"
def test_destroy_disk_metadata(self, mock_exec, mock_gz):
mock_gz.return_value = 64
expected_calls = [mock.call('dd', 'if=/dev/zero', 'of=fake-dev',
'bs=512', 'count=36', run_as_root=True,
check_exit_code=[0],
use_standard_locale=True),
mock.call('dd', 'if=/dev/zero', 'of=fake-dev',
'bs=512', 'count=36', 'seek=28',
def test_destroy_disk_metadata(self, mock_exec):
expected_calls = [mock.call('wipefs', '--all', 'fake-dev',
run_as_root=True,
check_exit_code=[0],
use_standard_locale=True)]
disk_utils.destroy_disk_metadata(self.dev, self.node_uuid)
mock_exec.assert_has_calls(expected_calls)
self.assertTrue(mock_gz.called)
def test_destroy_disk_metadata_get_dev_size_fail(self, mock_exec, mock_gz):
mock_gz.side_effect = processutils.ProcessExecutionError
expected_call = [mock.call('dd', 'if=/dev/zero', 'of=fake-dev',
'bs=512', 'count=36', run_as_root=True,
check_exit_code=[0],
use_standard_locale=True)]
self.assertRaises(processutils.ProcessExecutionError,
disk_utils.destroy_disk_metadata,
self.dev,
self.node_uuid)
mock_exec.assert_has_calls(expected_call)
def test_destroy_disk_metadata_dd_fail(self, mock_exec, mock_gz):
def test_destroy_disk_metadata_wipefs_fail(self, mock_exec):
mock_exec.side_effect = processutils.ProcessExecutionError
expected_call = [mock.call('dd', 'if=/dev/zero', 'of=fake-dev',
'bs=512', 'count=36', run_as_root=True,
expected_call = [mock.call('wipefs', '--all', 'fake-dev',
run_as_root=True,
check_exit_code=[0],
use_standard_locale=True)]
self.assertRaises(processutils.ProcessExecutionError,
@ -325,7 +304,6 @@ class DestroyMetaDataTestCase(test_base.BaseTestCase):
self.dev,
self.node_uuid)
mock_exec.assert_has_calls(expected_call)
self.assertFalse(mock_gz.called)
@mock.patch.object(utils, 'execute')