x/bareon
Code Issues Proposed changes

fuel_agent: removed reread_partitions method

Browse Source 
This reread_partitions method was a desperate attempt
to work around udev related "device is busy" error.
The correct way to deal with that stuff is to use
udevadm --settle which is to block thread until udev is
ready to handle events.

Closes-Bug: 1410471
Change-Id: Idb0dccb35aab10d02c5ad942fd30d52a461e1a0e
This commit is contained in:
Vladimir Kozhukalov 2015-01-14 10:20:50 +03:00
parent e808d85431
commit 2f6c37569a
3 changed files with 61 additions and 114 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
fuel_agent/manager.py
View File

@ -13,7 +13,6 @@
# limitations under the License. # limitations under the License.
import os import os
import time
from oslo.config import cfg from oslo.config import cfg
@ -80,30 +79,19 @@ class Manager(object):
lu.pvremove_all() lu.pvremove_all()
for parted in self.partition_scheme.parteds: for parted in self.partition_scheme.parteds:
for prt in parted.partitions:
# We wipe out the beginning of every new partition
# even before creating it. It allows us to avoid possible
# interactive dialog if some data (metadata or file system)
# present on this new partition and it also allows udev not
# hanging trying to parse this data.
utils.execute('dd', 'if=/dev/zero', 'bs=1M',
'seek=%s' % max(prt.begin - 1, 0), 'count=2',
'of=%s' % prt.device, check_exit_code=[0])
pu.make_label(parted.name, parted.label) pu.make_label(parted.name, parted.label)
for prt in parted.partitions: for prt in parted.partitions:
pu.make_partition(prt.device, prt.begin, prt.end, prt.type) pu.make_partition(prt.device, prt.begin, prt.end, prt.type)
# We wipe out the beginning of every new partition
# right after creating it. It allows us to avoid possible
# interactive dialog if some data (metadata or file system)
# present on this new partition.
timestamp = time.time()
while 1:
if time.time() > timestamp + 30:
raise errors.PartitionNotFoundError(
'Error while wiping data on partition %s.'
'Partition not found' % prt.name)
try:
utils.execute('test', '-e', prt.name,
check_exit_code=[0])
except errors.ProcessExecutionError:
time.sleep(1)
continue
else:
utils.execute('dd', 'if=/dev/zero', 'bs=1M', 'count=1',
'of=%s' % prt.name, check_exit_code=[0])
break
for flag in prt.flags: for flag in prt.flags:
pu.set_partition_flag(prt.device, prt.count, flag) pu.set_partition_flag(prt.device, prt.count, flag)
if prt.guid: if prt.guid:

110
fuel_agent/tests/test_partition_utils.py
View File

@ -14,7 +14,6 @@
import mock import mock
from oslotest import base as test_base from oslotest import base as test_base
import time
from fuel_agent import errors from fuel_agent import errors
from fuel_agent.utils import partition_utils as pu from fuel_agent.utils import partition_utils as pu
@ -30,28 +29,30 @@ class TestPartitionUtils(test_base.BaseTestCase):
pu.wipe('/dev/fake') pu.wipe('/dev/fake')
mock_label.assert_called_once_with('/dev/fake') mock_label.assert_called_once_with('/dev/fake')
@mock.patch.object(pu, 'reread_partitions')
@mock.patch.object(utils, 'execute') @mock.patch.object(utils, 'execute')
def test_make_label(self, mock_exec, mock_rerd): def test_make_label(self, mock_exec):
# should run parted OS command # should run parted OS command
# in order to create label on a device # in order to create label on a device
mock_exec.return_value = ('out', '') mock_exec.return_value = ('out', '')
# gpt by default # gpt by default
expected_calls = [
mock.call('udevadm', 'settle', '--quiet', check_exit_code=[0]),
mock.call('parted', '-s', '/dev/fake',
'mklabel', 'gpt', check_exit_code=[0, 1])]
pu.make_label('/dev/fake') pu.make_label('/dev/fake')
mock_exec.assert_called_once_with( self.assertEqual(mock_exec.call_args_list, expected_calls)
'parted', '-s', '/dev/fake',
'mklabel', 'gpt', check_exit_code=[0, 1])
mock_rerd.assert_called_once_with('/dev/fake', out='out')
mock_exec.reset_mock() mock_exec.reset_mock()
mock_rerd.reset_mock()
# label is set explicitly # label is set explicitly
expected_calls = [
mock.call('udevadm', 'settle', '--quiet', check_exit_code=[0]),
mock.call('parted', '-s', '/dev/fake',
'mklabel', 'msdos', check_exit_code=[0, 1])]
pu.make_label('/dev/fake', label='msdos') pu.make_label('/dev/fake', label='msdos')
mock_exec.assert_called_once_with( self.assertEqual(mock_exec.call_args_list, expected_calls)
'parted', '-s', '/dev/fake',
'mklabel', 'msdos', check_exit_code=[0, 1])
mock_rerd.assert_called_once_with('/dev/fake', out='out')
def test_make_label_wrong_label(self): def test_make_label_wrong_label(self):
# should check if label is valid # should check if label is valid
@ -59,28 +60,28 @@ class TestPartitionUtils(test_base.BaseTestCase):
self.assertRaises(errors.WrongPartitionLabelError, self.assertRaises(errors.WrongPartitionLabelError,
pu.make_label, '/dev/fake', 'wrong') pu.make_label, '/dev/fake', 'wrong')
@mock.patch.object(pu, 'reread_partitions')
@mock.patch.object(utils, 'execute') @mock.patch.object(utils, 'execute')
def test_set_partition_flag(self, mock_exec, mock_rerd): def test_set_partition_flag(self, mock_exec):
# should run parted OS command # should run parted OS command
# in order to set flag on a partition # in order to set flag on a partition
mock_exec.return_value = ('out', '') mock_exec.return_value = ('out', '')
# default state is 'on' # default state is 'on'
expected_calls = [
mock.call('udevadm', 'settle', '--quiet', check_exit_code=[0]),
mock.call('parted', '-s', '/dev/fake', 'set', '1', 'boot', 'on',
check_exit_code=[0, 1])]
pu.set_partition_flag('/dev/fake', 1, 'boot') pu.set_partition_flag('/dev/fake', 1, 'boot')
mock_exec.assert_called_once_with( self.assertEqual(mock_exec.call_args_list, expected_calls)
'parted', '-s', '/dev/fake', 'set', '1', 'boot', 'on',
check_exit_code=[0, 1])
mock_rerd.assert_called_once_with('/dev/fake', out='out')
mock_exec.reset_mock() mock_exec.reset_mock()
mock_rerd.reset_mock()
# if state argument is given use it # if state argument is given use it
expected_calls = [
mock.call('udevadm', 'settle', '--quiet', check_exit_code=[0]),
mock.call('parted', '-s', '/dev/fake', 'set', '1', 'boot', 'off',
check_exit_code=[0, 1])]
pu.set_partition_flag('/dev/fake', 1, 'boot', state='off') pu.set_partition_flag('/dev/fake', 1, 'boot', state='off')
mock_exec.assert_called_once_with( self.assertEqual(mock_exec.call_args_list, expected_calls)
'parted', '-s', '/dev/fake', 'set', '1', 'boot', 'off',
check_exit_code=[0, 1])
mock_rerd.assert_called_once_with('/dev/fake', out='out')
@mock.patch.object(utils, 'execute') @mock.patch.object(utils, 'execute')
def test_set_partition_flag_wrong_flag(self, mock_exec): def test_set_partition_flag_wrong_flag(self, mock_exec):
@ -98,10 +99,9 @@ class TestPartitionUtils(test_base.BaseTestCase):
pu.set_partition_flag, pu.set_partition_flag,
'/dev/fake', 1, 'boot', state='wrong') '/dev/fake', 1, 'boot', state='wrong')
@mock.patch.object(pu, 'reread_partitions')
@mock.patch.object(pu, 'info') @mock.patch.object(pu, 'info')
@mock.patch.object(utils, 'execute') @mock.patch.object(utils, 'execute')
def test_make_partition(self, mock_exec, mock_info, mock_rerd): def test_make_partition(self, mock_exec, mock_info):
# should run parted OS command # should run parted OS command
# in order to create new partition # in order to create new partition
mock_exec.return_value = ('out', '') mock_exec.return_value = ('out', '')
@ -111,15 +111,16 @@ class TestPartitionUtils(test_base.BaseTestCase):
{'begin': 0, 'end': 1000, 'fstype': 'free'}, {'begin': 0, 'end': 1000, 'fstype': 'free'},
] ]
} }
expected_calls = [
mock.call('udevadm', 'settle', '--quiet', check_exit_code=[0]),
mock.call('parted',
'-a', 'optimal',
'-s', '/dev/fake',
'unit', 'MiB',
'mkpart', 'primary', '100', '200',
check_exit_code=[0, 1])]
pu.make_partition('/dev/fake', 100, 200, 'primary') pu.make_partition('/dev/fake', 100, 200, 'primary')
mock_exec.assert_called_once_with( self.assertEqual(mock_exec.call_args_list, expected_calls)
'parted',
'-a', 'optimal',
'-s', '/dev/fake',
'unit', 'MiB',
'mkpart', 'primary', '100', '200',
check_exit_code=[0, 1])
mock_rerd.assert_called_once_with('/dev/fake', out='out')
@mock.patch.object(utils, 'execute') @mock.patch.object(utils, 'execute')
def test_make_partition_wrong_ptype(self, mock_exec): def test_make_partition_wrong_ptype(self, mock_exec):
@ -157,10 +158,9 @@ class TestPartitionUtils(test_base.BaseTestCase):
self.assertEqual(mock_info.call_args_list, self.assertEqual(mock_info.call_args_list,
[mock.call('/dev/fake')] * 3) [mock.call('/dev/fake')] * 3)
@mock.patch.object(pu, 'reread_partitions')
@mock.patch.object(pu, 'info') @mock.patch.object(pu, 'info')
@mock.patch.object(utils, 'execute') @mock.patch.object(utils, 'execute')
def test_remove_partition(self, mock_exec, mock_info, mock_rerd): def test_remove_partition(self, mock_exec, mock_info):
# should run parted OS command # should run parted OS command
# in order to remove partition # in order to remove partition
mock_exec.return_value = ('out', '') mock_exec.return_value = ('out', '')
@ -182,10 +182,12 @@ class TestPartitionUtils(test_base.BaseTestCase):
} }
] ]
} }
expected_calls = [
mock.call('udevadm', 'settle', '--quiet', check_exit_code=[0]),
mock.call('parted', '-s', '/dev/fake', 'rm', '1',
check_exit_code=[0])]
pu.remove_partition('/dev/fake', 1) pu.remove_partition('/dev/fake', 1)
mock_exec.assert_called_once_with( self.assertEqual(mock_exec.call_args_list, expected_calls)
'parted', '-s', '/dev/fake', 'rm', '1', check_exit_code=[0])
mock_rerd.assert_called_once_with('/dev/fake', out='out')
@mock.patch.object(pu, 'info') @mock.patch.object(pu, 'info')
@mock.patch.object(utils, 'execute') @mock.patch.object(utils, 'execute')
@ -216,9 +218,12 @@ class TestPartitionUtils(test_base.BaseTestCase):
@mock.patch.object(utils, 'execute') @mock.patch.object(utils, 'execute')
def test_set_gpt_type(self, mock_exec): def test_set_gpt_type(self, mock_exec):
pu.set_gpt_type('dev', 'num', 'type') pu.set_gpt_type('dev', 'num', 'type')
mock_exec.assert_called_once_with('sgdisk', expected_calls = [
'--typecode=%s:%s' % ('num', 'type'), mock.call('udevadm', 'settle', '--quiet', check_exit_code=[0]),
'dev', check_exit_code=[0]) mock.call('sgdisk',
'--typecode=%s:%s' % ('num', 'type'),
'dev', check_exit_code=[0])]
self.assertEqual(mock_exec.call_args_list, expected_calls)
@mock.patch.object(utils, 'execute') @mock.patch.object(utils, 'execute')
def test_info(self, mock_exec): def test_info(self, mock_exec):
@ -250,26 +255,3 @@ class TestPartitionUtils(test_base.BaseTestCase):
mock_exec.assert_called_once_with('parted', '-s', '/dev/fake', '-m', mock_exec.assert_called_once_with('parted', '-s', '/dev/fake', '-m',
'unit', 'MiB', 'print', 'free', 'unit', 'MiB', 'print', 'free',
check_exit_code=[0, 1]) check_exit_code=[0, 1])
@mock.patch.object(utils, 'execute')
def test_reread_partitions_ok(self, mock_exec):
pu.reread_partitions('/dev/fake', out='')
self.assertEqual(mock_exec.call_args_list, [])
@mock.patch.object(time, 'sleep')
@mock.patch.object(utils, 'execute')
def test_reread_partitions_device_busy(self, mock_exec, mock_sleep):
mock_exec.return_value = ('', '')
pu.reread_partitions('/dev/fake', out='_Device or resource busy_')
mock_exec_expected = [
mock.call('partprobe', '/dev/fake', check_exit_code=[0, 1]),
mock.call('partx', '-a', '/dev/fake', check_exit_code=[0, 1])
]
self.assertEqual(mock_exec.call_args_list, mock_exec_expected)
mock_sleep.assert_called_once_with(1)
@mock.patch.object(utils, 'execute')
def test_reread_partitions_timeout(self, mock_exec):
self.assertRaises(errors.BaseError, pu.reread_partitions,
'/dev/fake', out='Device or resource busy',
timeout=-40)

33
fuel_agent/utils/partition_utils.py
View File

@ -12,8 +12,6 @@
# See the License for the specific language governing permissions and # See the License for the specific language governing permissions and
# limitations under the License. # limitations under the License.
import time
from fuel_agent import errors from fuel_agent import errors
from fuel_agent.openstack.common import log as logging from fuel_agent.openstack.common import log as logging
from fuel_agent.utils import utils from fuel_agent.utils import utils
@ -79,10 +77,10 @@ def make_label(dev, label='gpt'):
if label not in ('gpt', 'msdos'): if label not in ('gpt', 'msdos'):
raise errors.WrongPartitionLabelError( raise errors.WrongPartitionLabelError(
'Wrong partition label type: %s' % label) 'Wrong partition label type: %s' % label)
utils.execute('udevadm', 'settle', '--quiet', check_exit_code=[0])
out, err = utils.execute('parted', '-s', dev, 'mklabel', label, out, err = utils.execute('parted', '-s', dev, 'mklabel', label,
check_exit_code=[0, 1]) check_exit_code=[0, 1])
LOG.debug('Parted output: \n%s' % out) LOG.debug('Parted output: \n%s' % out)
reread_partitions(dev, out=out)
def set_partition_flag(dev, num, flag, state='on'): def set_partition_flag(dev, num, flag, state='on'):
@ -107,10 +105,10 @@ def set_partition_flag(dev, num, flag, state='on'):
if state not in ('on', 'off'): if state not in ('on', 'off'):
raise errors.WrongPartitionSchemeError( raise errors.WrongPartitionSchemeError(
'Wrong partition flag state: %s' % state) 'Wrong partition flag state: %s' % state)
utils.execute('udevadm', 'settle', '--quiet', check_exit_code=[0])
out, err = utils.execute('parted', '-s', dev, 'set', str(num), out, err = utils.execute('parted', '-s', dev, 'set', str(num),
flag, state, check_exit_code=[0, 1]) flag, state, check_exit_code=[0, 1])
LOG.debug('Parted output: \n%s' % out) LOG.debug('Parted output: \n%s' % out)
reread_partitions(dev, out=out)
def set_gpt_type(dev, num, type_guid): def set_gpt_type(dev, num, type_guid):
@ -127,6 +125,7 @@ def set_gpt_type(dev, num, type_guid):
# TODO(kozhukalov): check whether type_guid is valid # TODO(kozhukalov): check whether type_guid is valid
LOG.debug('Setting partition GUID: dev=%s num=%s guid=%s' % LOG.debug('Setting partition GUID: dev=%s num=%s guid=%s' %
(dev, num, type_guid)) (dev, num, type_guid))
utils.execute('udevadm', 'settle', '--quiet', check_exit_code=[0])
utils.execute('sgdisk', '--typecode=%s:%s' % (num, type_guid), utils.execute('sgdisk', '--typecode=%s:%s' % (num, type_guid),
dev, check_exit_code=[0]) dev, check_exit_code=[0])
@ -150,11 +149,11 @@ def make_partition(dev, begin, end, ptype):
'Invalid boundaries: begin and end ' 'Invalid boundaries: begin and end '
'are not inside available free space') 'are not inside available free space')
utils.execute('udevadm', 'settle', '--quiet', check_exit_code=[0])
out, err = utils.execute( out, err = utils.execute(
'parted', '-a', 'optimal', '-s', dev, 'unit', 'MiB', 'parted', '-a', 'optimal', '-s', dev, 'unit', 'MiB',
'mkpart', ptype, str(begin), str(end), check_exit_code=[0, 1]) 'mkpart', ptype, str(begin), str(end), check_exit_code=[0, 1])
LOG.debug('Parted output: \n%s' % out) LOG.debug('Parted output: \n%s' % out)
reread_partitions(dev, out=out)
def remove_partition(dev, num): def remove_partition(dev, num):
@ -162,28 +161,6 @@ def remove_partition(dev, num):
if not any(x['fstype'] != 'free' and x['num'] == num if not any(x['fstype'] != 'free' and x['num'] == num
for x in info(dev)['parts']): for x in info(dev)['parts']):
raise errors.PartitionNotFoundError('Partition %s not found' % num) raise errors.PartitionNotFoundError('Partition %s not found' % num)
utils.execute('udevadm', 'settle', '--quiet', check_exit_code=[0])
out, err = utils.execute('parted', '-s', dev, 'rm', out, err = utils.execute('parted', '-s', dev, 'rm',
str(num), check_exit_code=[0]) str(num), check_exit_code=[0])
reread_partitions(dev, out=out)
def reread_partitions(dev, out='Device or resource busy', timeout=30):
# The reason for this method to exist is that old versions of parted
# use ioctl(fd, BLKRRPART, NULL) to tell Linux to re-read partitions.
# This system call does not work sometimes. So we try to re-read partition
# table several times. Besides partprobe uses BLKPG instead, which
# is better than BLKRRPART for this case. BLKRRPART tells Linux to re-read
# partitions while BLKPG tells Linux which partitions are available
# BLKPG is usually used as a fallback system call.
begin = time.time()
while 'Device or resource busy' in out:
if time.time() > begin + timeout:
raise errors.BaseError('Unable to re-read partition table on'
'device %s' % dev)
LOG.debug('Last time output contained "Device or resource busy". '
'Trying to re-read partition table on device %s' % dev)
out, err = utils.execute('partprobe', dev, check_exit_code=[0, 1])
LOG.debug('Partprobe output: \n%s' % out)
pout, perr = utils.execute('partx', '-a', dev, check_exit_code=[0, 1])
LOG.debug('Partx output: \n%s' % pout)
time.sleep(1)