From d2185b1316cf72e1e899eb309e4bffcdd7ff8a0e Mon Sep 17 00:00:00 2001 From: Alexander Gordeev Date: Thu, 5 May 2016 15:52:40 +0300 Subject: [PATCH] Add option to choose partition alignment mode Under certain circumstances due to that alignment, the end of particular partition could cross 1M boundary. And due to actual partition' bounderies being rounded up, fuel-agent mistakenly assumes that partition couldn't fit within provided boundaries and raises errors.WrongPartitionSchemeError. However, some h/w data storages are well known for reporting relatively huge optimal IO sizes (16M or even bigger), so the 1M room can't be enough. Thus, optimal aligning is not an option. In such cases, it's better to proceed with minimal mode. It's the minimum aligment needed to align the partition properly to physical blocks which avoids performance degradation. Partial-Bug: #1543233 DocImpact Conflicts: bareon/drivers/deploy/nailgun.py fuel_agent/tests/test_manager.py Change-Id: I1dd94731f497f3deb47cec9a23957e3706c2fb3b --- bareon/actions/partitioning.py | 9 ++++- bareon/objects/partition/parted.py | 6 ++-- bareon/tests/test_do_partitioning.py | 49 ++++++++++++++++------------ bareon/tests/test_partition.py | 8 ++--- bareon/tests/test_partition_utils.py | 29 ++++++++++++++++ bareon/utils/partition.py | 20 ++++++++++-- 6 files changed, 91 insertions(+), 30 deletions(-) diff --git a/bareon/actions/partitioning.py b/bareon/actions/partitioning.py index c0b1214..8d3bf9f 100644 --- a/bareon/actions/partitioning.py +++ b/bareon/actions/partitioning.py @@ -54,6 +54,12 @@ opts = [ help='Allow to skip MD containers (fake raid leftovers) while ' 'cleaning the rest of MDs', ), + cfg.StrOpt( + 'partition_alignment', + default='optimal', + help='Set alignment for newly created partitions, valid alignment ' + 'types are: none, cylinder, minimal, optimal' + ), ] CONF = cfg.CONF @@ -101,7 +107,8 @@ class PartitioningAction(base.BaseAction): for parted in parteds: pu.make_label(parted.name, parted.label) 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, + alignment=CONF.partition_alignment) utils.udevadm_trigger_blocks() for flag in prt.flags: pu.set_partition_flag(prt.device, prt.count, flag) diff --git a/bareon/objects/partition/parted.py b/bareon/objects/partition/parted.py index ba30fe3..90c214d 100644 --- a/bareon/objects/partition/parted.py +++ b/bareon/objects/partition/parted.py @@ -91,8 +91,10 @@ class Parted(base.Serializable): if not self.partitions: return 1 if self.partitions[-1] == self.extended: - return self.partitions[-1].begin - return self.partitions[-1].end + # NOTE(agordeev): this 1M room could be enough for minimal + # partition alignment mode for the most of cases. + return self.partitions[-1].begin + 1 + return self.partitions[-1].end + 1 def next_name(self): if self.next_type() == 'extended': diff --git a/bareon/tests/test_do_partitioning.py b/bareon/tests/test_do_partitioning.py index bf5f36f..48e4400 100644 --- a/bareon/tests/test_do_partitioning.py +++ b/bareon/tests/test_do_partitioning.py @@ -114,19 +114,21 @@ class TestPartitioningAction(unittest2.TestCase): mock_pu.make_label.call_args_list) mock_pu_mp_expected_calls = [ - mock.call('/dev/sda', 1, 25, 'primary'), - mock.call('/dev/sda', 25, 225, 'primary'), - mock.call('/dev/sda', 225, 425, 'primary'), - mock.call('/dev/sda', 425, 625, 'primary'), - mock.call('/dev/sda', 625, 20063, 'primary'), - mock.call('/dev/sda', 20063, 65660, 'primary'), - mock.call('/dev/sda', 65660, 65680, 'primary'), - mock.call('/dev/sdb', 1, 25, 'primary'), - mock.call('/dev/sdb', 25, 225, 'primary'), - mock.call('/dev/sdb', 225, 65196, 'primary'), - mock.call('/dev/sdc', 1, 25, 'primary'), - mock.call('/dev/sdc', 25, 225, 'primary'), - mock.call('/dev/sdc', 225, 65196, 'primary')] + mock.call('/dev/sda', 1, 25, 'primary', alignment='optimal'), + mock.call('/dev/sda', 26, 226, 'primary', alignment='optimal'), + mock.call('/dev/sda', 227, 427, 'primary', alignment='optimal'), + mock.call('/dev/sda', 428, 628, 'primary', alignment='optimal'), + mock.call('/dev/sda', 629, 20067, 'primary', alignment='optimal'), + mock.call('/dev/sda', 20068, 65665, 'primary', + alignment='optimal'), + mock.call('/dev/sda', 65666, 65686, 'primary', + alignment='optimal'), + mock.call('/dev/sdb', 1, 25, 'primary', alignment='optimal'), + mock.call('/dev/sdb', 26, 226, 'primary', alignment='optimal'), + mock.call('/dev/sdb', 227, 65198, 'primary', alignment='optimal'), + mock.call('/dev/sdc', 1, 25, 'primary', alignment='optimal'), + mock.call('/dev/sdc', 26, 226, 'primary', alignment='optimal'), + mock.call('/dev/sdc', 227, 65198, 'primary', alignment='optimal')] self.assertEqual(mock_pu_mp_expected_calls, mock_pu.make_partition.call_args_list) @@ -249,14 +251,19 @@ class TestManagerMultipathPartition(unittest2.TestCase): mock.call('/dev/sdc', 'gpt')]) self.assertEqual(mock_pu.make_partition.mock_calls, [ - mock.call('/dev/mapper/12312', 1, 25, 'primary'), - mock.call('/dev/mapper/12312', 25, 225, 'primary'), - mock.call('/dev/mapper/12312', 225, 425, 'primary'), - mock.call('/dev/mapper/12312', 425, 625, 'primary'), - mock.call('/dev/mapper/12312', 625, 645, 'primary'), - mock.call('/dev/sdc', 1, 25, 'primary'), - mock.call('/dev/sdc', 25, 225, 'primary'), - mock.call('/dev/sdc', 225, 425, 'primary')]) + mock.call('/dev/mapper/12312', 1, 25, 'primary', + alignment='optimal'), + mock.call('/dev/mapper/12312', 26, 226, 'primary', + alignment='optimal'), + mock.call('/dev/mapper/12312', 227, 427, 'primary', + alignment='optimal'), + mock.call('/dev/mapper/12312', 428, 628, 'primary', + alignment='optimal'), + mock.call('/dev/mapper/12312', 629, 649, 'primary', + alignment='optimal'), + mock.call('/dev/sdc', 1, 25, 'primary', alignment='optimal'), + mock.call('/dev/sdc', 26, 226, 'primary', alignment='optimal'), + mock.call('/dev/sdc', 227, 427, 'primary', alignment='optimal')]) self.assertEqual(mock_pu.set_partition_flag.mock_calls, [ mock.call('/dev/mapper/12312', 1, 'bios_grub'), diff --git a/bareon/tests/test_partition.py b/bareon/tests/test_partition.py index f996dee..9c3cacb 100644 --- a/bareon/tests/test_partition.py +++ b/bareon/tests/test_partition.py @@ -286,15 +286,15 @@ class TestParted(unittest2.TestCase): def test_next_begin_last_extended_partition(self): self.prtd.partitions.append( - objects.Partition('name', 'count', 'device', 'begin', 'end', + objects.Partition('name', 'count', 'device', 1, 100, 'extended')) - self.assertEqual('begin', self.prtd.next_begin()) + self.assertEqual(2, self.prtd.next_begin()) def test_next_begin_no_last_extended_partition(self): self.prtd.partitions.append( - objects.Partition('name', 'count', 'device', 'begin', 'end', + objects.Partition('name', 'count', 'device', 1, 100, 'primary')) - self.assertEqual('end', self.prtd.next_begin()) + self.assertEqual(101, self.prtd.next_begin()) def test_next_count_no_logical(self): self.assertEqual(1, self.prtd.next_count('primary')) diff --git a/bareon/tests/test_partition_utils.py b/bareon/tests/test_partition_utils.py index 1a2ff1b..fa0c234 100644 --- a/bareon/tests/test_partition_utils.py +++ b/bareon/tests/test_partition_utils.py @@ -134,6 +134,35 @@ class TestPartitionUtils(unittest2.TestCase): self.assertEqual(mock_exec_expected_calls, mock_exec.call_args_list) mock_rerd.assert_called_once_with('/dev/fake', out='out') + @mock.patch.object(utils, 'udevadm_settle') + @mock.patch.object(pu, 'reread_partitions') + @mock.patch.object(pu, 'info') + @mock.patch.object(utils, 'execute') + def test_make_partition_minimal(self, mock_exec, mock_info, mock_rerd, + mock_udev): + # should run parted OS command + # in order to create new partition + mock_exec.return_value = ('out', '') + + mock_info.return_value = { + 'parts': [ + {'begin': 0, 'end': 1000, 'fstype': 'free'}, + ] + } + pu.make_partition('/dev/fake', 100, 200, 'primary', + alignment='minimal') + mock_exec_expected_calls = [ + mock.call('parted', '-a', 'minimal', '-s', '/dev/fake', 'unit', + 'MiB', 'mkpart', 'primary', '100', '200', + check_exit_code=[0, 1])] + mock_udev.assert_called_once_with() + self.assertEqual(mock_exec_expected_calls, mock_exec.call_args_list) + mock_rerd.assert_called_once_with('/dev/fake', out='out') + + def test_make_partition_wrong_alignment(self): + self.assertRaises(errors.WrongPartitionSchemeError, pu.make_partition, + '/dev/fake', 1, 10, 'primary', 'invalid') + @mock.patch.object(utils, 'execute') def test_make_partition_wrong_ptype(self, mock_exec): # should check if partition type is one of diff --git a/bareon/utils/partition.py b/bareon/utils/partition.py index 490ed38..c13caf7 100644 --- a/bareon/utils/partition.py +++ b/bareon/utils/partition.py @@ -19,6 +19,7 @@ from bareon.openstack.common import log as logging from bareon.utils import utils LOG = logging.getLogger(__name__) +PARTITION_ALIGMENT = ('none', 'cylinder', 'minimal', 'optimal') def parse_partition_info(parted_output): @@ -159,12 +160,27 @@ def set_gpt_type(dev, num, type_guid): dev, check_exit_code=[0]) -def make_partition(dev, begin, end, ptype): +def make_partition(dev, begin, end, ptype, alignment='optimal'): + """Creates a partition on the device. + + :param dev: A device file, e.g. /dev/sda. + :param begin: Beginning of the partition. + :param end: Ending of the partition. + :param ptype: Partition type: primary or logical. + :param alignment: Set alignment mode for newly created partitions, + valid alignment types are: none, cylinder, minimal, optimal. For more + information about this you can find in GNU parted manual. + + :returns: None + """ LOG.debug('Trying to create a partition: dev=%s begin=%s end=%s' % (dev, begin, end)) if ptype not in ('primary', 'logical'): raise errors.WrongPartitionSchemeError( 'Wrong partition type: %s' % ptype) + if alignment not in PARTITION_ALIGMENT: + raise errors.WrongPartitionSchemeError( + 'Wrong partition alignment requested: %s' % alignment) # check begin >= end if begin >= end: @@ -180,7 +196,7 @@ def make_partition(dev, begin, end, ptype): utils.udevadm_settle() out, err = utils.execute( - 'parted', '-a', 'optimal', '-s', dev, 'unit', 'MiB', + 'parted', '-a', alignment, '-s', dev, 'unit', 'MiB', 'mkpart', ptype, str(begin), str(end), check_exit_code=[0, 1]) LOG.debug('Parted output: \n%s' % out) reread_partitions(dev, out=out)