Check the base device if the read-only file cannot be read
For some drives, the partition e.g. `/dev/sda1` will not have the 'ro' file which can result in a metadata erasure failure but the base device (`/dev/sda`) will have this file. Add an additional check for the base device. Change-Id: Ia01bdbf82cee6ce15fabdc42f9c23036df55b4c5 Story: 2008696 Task: 42004
This commit is contained in:
parent
7931ccedfb
commit
4afe4f6069
@ -24,6 +24,7 @@ import re
|
|||||||
import shlex
|
import shlex
|
||||||
import shutil
|
import shutil
|
||||||
import stat
|
import stat
|
||||||
|
import string
|
||||||
import time
|
import time
|
||||||
|
|
||||||
from ironic_lib import disk_utils
|
from ironic_lib import disk_utils
|
||||||
@ -1447,23 +1448,32 @@ class GenericHardwareManager(HardwareManager):
|
|||||||
|
|
||||||
return 'linux_raid_member' in out
|
return 'linux_raid_member' in out
|
||||||
|
|
||||||
def _is_read_only_device(self, block_device):
|
def _is_read_only_device(self, block_device, partition=False):
|
||||||
"""Check if a block device is read-only.
|
"""Check if a block device is read-only.
|
||||||
|
|
||||||
Checks the device read-only flag in order to identify virtual
|
Checks the device read-only flag in order to identify virtual
|
||||||
and firmware driven devices that block write device access.
|
and firmware driven devices that block write device access.
|
||||||
|
|
||||||
:param block_device: a BlockDevice object
|
:param block_device: a BlockDevice object
|
||||||
|
:param partition: if True, this device is a partition
|
||||||
:returns: True if the device is read-only.
|
:returns: True if the device is read-only.
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
dev_name = str(block_device.name)[5:]
|
dev_name = os.path.basename(block_device.name)
|
||||||
|
if partition:
|
||||||
|
# Check the base device
|
||||||
|
dev_name = dev_name.rstrip(string.digits)
|
||||||
|
|
||||||
with open('/sys/block/%s/ro' % dev_name, 'r') as f:
|
with open('/sys/block/%s/ro' % dev_name, 'r') as f:
|
||||||
flag = f.read().strip()
|
flag = f.read().strip()
|
||||||
if flag == '1':
|
if flag == '1':
|
||||||
return True
|
return True
|
||||||
except IOError as e:
|
except IOError as e:
|
||||||
|
# Check underlying device as the file may exist there
|
||||||
|
if (not partition and dev_name[-1].isdigit()
|
||||||
|
and 'nvme' not in dev_name):
|
||||||
|
return self._is_read_only_device(block_device, partition=True)
|
||||||
|
|
||||||
LOG.warning("Could not determine if %(name)s is a"
|
LOG.warning("Could not determine if %(name)s is a"
|
||||||
"read-only device. Error: %(err)s",
|
"read-only device. Error: %(err)s",
|
||||||
{'name': block_device.name, 'err': e})
|
{'name': block_device.name, 'err': e})
|
||||||
|
@ -2184,6 +2184,26 @@ class TestGenericHardwareManager(base.IronicAgentTest):
|
|||||||
mock_open.assert_called_once_with(
|
mock_open.assert_called_once_with(
|
||||||
'/sys/block/sdfake/ro', 'r')
|
'/sys/block/sdfake/ro', 'r')
|
||||||
|
|
||||||
|
def test__is_read_only_device_partition_error(self):
|
||||||
|
device = hardware.BlockDevice('/dev/sdfake1', 'fake', 1024, False)
|
||||||
|
with mock.patch(
|
||||||
|
'builtins.open', side_effect=IOError,
|
||||||
|
autospec=True) as mock_open:
|
||||||
|
self.assertFalse(self.hardware._is_read_only_device(device))
|
||||||
|
mock_open.assert_has_calls([
|
||||||
|
mock.call('/sys/block/sdfake1/ro', 'r'),
|
||||||
|
mock.call('/sys/block/sdfake/ro', 'r')])
|
||||||
|
|
||||||
|
def test__is_read_only_device_partition_ok(self):
|
||||||
|
fileobj = mock.mock_open(read_data='1\n')
|
||||||
|
device = hardware.BlockDevice('/dev/sdfake1', 'fake', 1024, False)
|
||||||
|
reads = [IOError, '1']
|
||||||
|
with mock.patch(
|
||||||
|
'builtins.open', fileobj, create=True) as mock_open:
|
||||||
|
mock_dev_file = mock_open.return_value.read
|
||||||
|
mock_dev_file.side_effect = reads
|
||||||
|
self.assertTrue(self.hardware._is_read_only_device(device))
|
||||||
|
|
||||||
@mock.patch.object(il_utils, 'try_execute', autospec=True)
|
@mock.patch.object(il_utils, 'try_execute', autospec=True)
|
||||||
@mock.patch.object(utils, 'execute', autospec=True)
|
@mock.patch.object(utils, 'execute', autospec=True)
|
||||||
def test_get_bmc_address(self, mocked_execute, mte):
|
def test_get_bmc_address(self, mocked_execute, mte):
|
||||||
|
@ -0,0 +1,7 @@
|
|||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
Fixes an issue where metadata erasure cleaning fails for partitions
|
||||||
|
because the read-only file isn't found, while it is available at the
|
||||||
|
base device. Adds a check for the base device file on failure. See
|
||||||
|
`story 2008696 <https://storyboard.openstack.org/#!/story/2008696>`_.
|
Loading…
x
Reference in New Issue
Block a user