Only the last two patches are even remotely interesting.
mulhern (8): Fix mdnominate error message. Use long messages for unittest errors. Still attempt to destroy even if remove failed. Add a test for mddetail on containers. Change Container.has_redundancy to True Add a test for activation. Revise mdadd. Add a bunch of tests for mdadd.
blivet/devicelibs/mdraid.py | 30 ++-- blivet/devicelibs/raid.py | 2 +- blivet/devices.py | 14 +- tests/devicelibs_test/mdraid_interrogate_test.py | 22 ++- tests/devicelibs_test/mdraid_test.py | 196 +++++++++++++++++------ tests/pylint/pylint-false-positives | 1 + 6 files changed, 197 insertions(+), 68 deletions(-)
Signed-off-by: mulhern amulhern@redhat.com --- blivet/devicelibs/mdraid.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/blivet/devicelibs/mdraid.py b/blivet/devicelibs/mdraid.py index 516bbba..8f81d1b 100644 --- a/blivet/devicelibs/mdraid.py +++ b/blivet/devicelibs/mdraid.py @@ -154,7 +154,7 @@ def mdnominate(device): try: mdadm(args) except MDRaidError as msg: - raise MDRaidError("mdadd failed for %s: %s" % (device, msg)) + raise MDRaidError("mdnominate failed for %s: %s" % (device, msg))
def mdadd(array, device, raid_devices=None): """ Add a device to an array.
Signed-off-by: mulhern amulhern@redhat.com --- blivet/udev.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/blivet/udev.py b/blivet/udev.py index 88f09ed..7e4f483 100644 --- a/blivet/udev.py +++ b/blivet/udev.py @@ -387,7 +387,9 @@ def device_get_md_level(info): return info.get("MD_LEVEL")
def device_get_md_devices(info): - """ Returns the number of devices in this devices's array. + """ Returns the number of active devices in this devices's array. + + Active devices are devices that are not spares or failed.
:param dict info: dictionary of name-value pairs as strings :returns: the number of devices belonging to this device's md array
Signed-off-by: mulhern amulhern@redhat.com --- blivet/fcoe.py | 1 - 1 file changed, 1 deletion(-)
diff --git a/blivet/fcoe.py b/blivet/fcoe.py index a4c3063..707ec50 100644 --- a/blivet/fcoe.py +++ b/blivet/fcoe.py @@ -20,7 +20,6 @@ import os from . import udev from . import util -#from pyanaconda import isys import logging import time from .i18n import _
--- tests/devicelibs_test/mdraid_test.py | 1 + 1 file changed, 1 insertion(+)
diff --git a/tests/devicelibs_test/mdraid_test.py b/tests/devicelibs_test/mdraid_test.py index 042f1eb..4081bd2 100755 --- a/tests/devicelibs_test/mdraid_test.py +++ b/tests/devicelibs_test/mdraid_test.py @@ -113,6 +113,7 @@ class SimpleRaidTest(MDRaidAsRootTestCase):
def __init__(self, methodName='runTest'): super(SimpleRaidTest, self).__init__(methodName=methodName, deviceSpec=[102400, 102400, 102400]) + self.longMessage = True
def testMDRaidAsRoot(self): ##
Signed-off-by: mulhern amulhern@redhat.com --- blivet/devicelibs/mdraid.py | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/blivet/devicelibs/mdraid.py b/blivet/devicelibs/mdraid.py index c71dd84..af04aa2 100644 --- a/blivet/devicelibs/mdraid.py +++ b/blivet/devicelibs/mdraid.py @@ -221,6 +221,22 @@ def mddeactivate(device): except MDRaidError as msg: raise MDRaidError("mddeactivate failed for %s: %s" % (device, msg))
+def canonicalizeUUID(a_uuid): + """ Converts uuids to canonical form. + + :param str a_uuid: the UUID + + :returns: a canonicalized UUID + :rtype: str + + mdadm's UUIDs are actual 128 bit uuids, but it formats them strangely. + This converts the uuids to canonical form. + Example: + mdadm UUID: '3386ff85:f5012621:4a435f06:1eb47236' + canonical UUID: '3386ff85-f501-2621-4a43-5f061eb47236' + """ + return str(uuid.UUID(a_uuid.replace(':', ''))) + def mdexamine(device): _vars = util.capture_output(["mdadm", "--examine", "--export", device]).split() @@ -248,13 +264,8 @@ def mdexamine(device): if name == "metadata": info["MD_METADATA"] = value
- # mdadm's UUIDs are actual 128 bit uuids, but it formats them strangely. - # This converts the uuids to canonical form. - # Example: - # mdadm UUID: '3386ff85:f5012621:4a435f06:1eb47236' - # canonical UUID: '3386ff85-f501-2621-4a43-5f061eb47236' for k, v in ((k,v) for (k,v) in info.iteritems() if k.endswith("UUID")): - info[k] = str(uuid.UUID(v.replace(':', ''))) + info[k] = canonicalizeUUID(v)
return info
--- tests/devicelibs_test/mdraid_test.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/tests/devicelibs_test/mdraid_test.py b/tests/devicelibs_test/mdraid_test.py index 4081bd2..0daa56e 100755 --- a/tests/devicelibs_test/mdraid_test.py +++ b/tests/devicelibs_test/mdraid_test.py @@ -57,11 +57,17 @@ class MDRaidAsRootTestCase(loopbackedtestcase.LoopBackedTestCase): def tearDown(self): try: mdraid.mddeactivate(self._dev_name) - for dev in self.loopDevices: - mdraid.mdremove(self._dev_name, dev, fail=True) - mdraid.mddestroy(dev) except MDRaidError: pass + for dev in self.loopDevices: + try: + mdraid.mdremove(self._dev_name, dev, fail=True) + except MDRaidError: + pass + try: + mdraid.mddestroy(dev) + except MDRaidError: + pass
super(MDRaidAsRootTestCase, self).tearDown()
Signed-off-by: mulhern amulhern@redhat.com --- tests/devicelibs_test/mdraid_interrogate_test.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/tests/devicelibs_test/mdraid_interrogate_test.py b/tests/devicelibs_test/mdraid_interrogate_test.py index 0909fe3..2178d79 100755 --- a/tests/devicelibs_test/mdraid_interrogate_test.py +++ b/tests/devicelibs_test/mdraid_interrogate_test.py @@ -174,6 +174,13 @@ class MDDetailTestCase(MDRaidInterrogateTestCase): 'WORKING DEVICES' ]
+ names_container = [ + 'RAID LEVEL', + 'WORKING DEVICES', + 'VERSION', + 'TOTAL DEVICES' + ] + def _testMDDetail(self, names, metadataVersion=None, level=None, spares=0): """ Test mddetail for a specified metadataVersion.
@@ -197,15 +204,17 @@ class MDDetailTestCase(MDRaidInterrogateTestCase): self._matchNames(info.keys(), names, ['RESYNC STATUS'])
# check names with predictable values - self.assertEqual(info['ACTIVE DEVICES'], str(len(self.loopDevices) - spares)) - self.assertEqual(info['FAILED DEVICES'], '0') self.assertEqual(info['RAID LEVEL'], str(level)) - self.assertEqual(info['SPARE DEVICES'], str(spares)) self.assertEqual(info['TOTAL DEVICES'], str(len(self.loopDevices))) self.assertEqual(info['WORKING DEVICES'], str(len(self.loopDevices)))
- # verify that uuid is in canonical form - self.assertEqual(str(uuid.UUID(info['UUID'])), info['UUID']) + if level is not raid.Container: + self.assertEqual(info['ACTIVE DEVICES'], str(len(self.loopDevices) - spares)) + self.assertEqual(info['FAILED DEVICES'], '0') + self.assertEqual(info['SPARE DEVICES'], str(spares)) + + # verify that uuid is in canonical form + self.assertEqual(str(uuid.UUID(info['UUID'])), info['UUID'])
def testMDDetail(self): self._testMDDetail(self.names) @@ -224,5 +233,8 @@ class MDDetailTestCase(MDRaidInterrogateTestCase): with self.assertRaisesRegexp(MDRaidError, "mddetail failed"): mdraid.mddetail(self.loopDevices[0])
+ def testMDDetailContainerDefault(self): + self._testMDDetail(self.names_container, level="container") + if __name__ == "__main__": unittest.main()
Signed-off-by: mulhern amulhern@redhat.com --- blivet/devices.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/blivet/devices.py b/blivet/devices.py index c3da7f6..3d57df9 100644 --- a/blivet/devices.py +++ b/blivet/devices.py @@ -552,7 +552,7 @@ class StorageDevice(Device):
self.exists = exists self.uuid = uuid - Device.__init__(self, name, parents=parents) + super(StorageDevice, self).__init__(name, parents=parents)
self._format = None self._size = Size(util.numeric_type(size))
Container is a tricky RAID level, but this makes MDRaidArrayDevice._add() do the right thing.
Signed-off-by: mulhern amulhern@redhat.com --- blivet/devicelibs/raid.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/blivet/devicelibs/raid.py b/blivet/devicelibs/raid.py index 50d19dc..7aca910 100644 --- a/blivet/devicelibs/raid.py +++ b/blivet/devicelibs/raid.py @@ -551,7 +551,7 @@ class Container(RAIDLevel): name = "container" names = [name] min_members = 1 - has_redundancy = property(lambda s: False) + has_redundancy = property(lambda s: True) is_uniform = property(lambda s: False)
def get_max_spares(self, member_count):
On Fri, 2014-07-25 at 12:39 -0400, mulhern wrote:
Container is a tricky RAID level, but this makes MDRaidArrayDevice._add() do the right thing.
I'm sorry, but I don't understand this change. It looks to me like there is a bug in MDRaidArrayDevice._add() if it requires a RAID level with no redundancy to report that it has redundancy. Or am I completely missing something?
Signed-off-by: mulhern amulhern@redhat.com --- blivet/devices.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/blivet/devices.py b/blivet/devices.py index 3d57df9..cb29931 100644 --- a/blivet/devices.py +++ b/blivet/devices.py @@ -3840,7 +3840,7 @@ class MDRaidArrayDevice(ContainerDevice): removing one of its devices is a bad idea. """ if not self.level.has_redundancy and self.exists and member.format.exists: - raise errors.DeviceError("cannot remove members from existing raid0") + raise errors.DeviceError("cannot remove members from existing %s array" % self.level)
super(MDRaidArrayDevice, self)._removeParent(member) self.memberDevices -= 1
mdadm --detail can give useful information about the number of spare devices and other things that mdadm --examine can not. According to the man page --examine applies to devices which are components of an array, while --detail applies to a whole array which is currently active.
Signed-off-by: mulhern amulhern@redhat.com --- blivet/devicelibs/mdraid.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
diff --git a/blivet/devicelibs/mdraid.py b/blivet/devicelibs/mdraid.py index af04aa2..a2745b6 100644 --- a/blivet/devicelibs/mdraid.py +++ b/blivet/devicelibs/mdraid.py @@ -21,6 +21,7 @@ #
import os +import re import uuid
from .. import util @@ -269,6 +270,35 @@ def mdexamine(device):
return info
+def mddetail(device): + """Run mdadm --detail in order to read information about an array. + + Note: The --export flag is not used. According to the man pages + the export flag just formats the output as key=value pairs for + easy import, but in the case of --detail it also omits the majority + of the information, including information of real use like the + number of spares in the array. + + :rtype: a dict of strings + :returns: a dict containing labels and values extracted from output + """ + lines = util.capture_output(["mdadm", "--detail", device]).split("\n") + info = {} + for (name, equals, value) in (line.strip().partition(":") for line in lines): + if equals and value.strip(): + name = name.strip().upper() + value = value.strip() + + info[name] = value + + # extract the UUID proper + the_uuid = re.match(r"(([a-f0-9]){8}:){3}([a-f0-9]){8}", info['UUID']) + + # canonicalize the extracted UUID + info['UUID'] = canonicalizeUUID(the_uuid.group()) + + return info + def md_node_from_name(name): named_path = "/dev/md/" + name try:
--- tests/devicelibs_test/mdraid_test.py | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/tests/devicelibs_test/mdraid_test.py b/tests/devicelibs_test/mdraid_test.py index 0daa56e..ad0c3c7 100755 --- a/tests/devicelibs_test/mdraid_test.py +++ b/tests/devicelibs_test/mdraid_test.py @@ -155,6 +155,8 @@ class SimpleRaidTest(MDRaidAsRootTestCase): with self.assertRaises(MDRaidError): mdraid.mdnominate(self.loopDevices[2])
+ info_pre = mdraid.mddetail(self._dev_name) + ## ## mddeactivate ## @@ -182,6 +184,17 @@ class SimpleRaidTest(MDRaidAsRootTestCase): with self.assertRaises(MDRaidError): mdraid.mdactivate("/dev/md1")
+ self.assertIsNone(mdraid.mdactivate(self._dev_name, array_uuid=info_pre['UUID'])) + time.sleep(2) + info_post = mdraid.mddetail(self._dev_name) + + # the array should remain the same across activations + for k in info_pre.keys(): + self.assertEqual(info_pre[k], info_post[k], msg="key: %s" % k) + + # deactivating the array one more time + self.assertIsNone(mdraid.mddeactivate(self._dev_name)) + ## ## mddestroy ##
Also, add some mddetail test cases and extend tests for mdremove and mdadd.
Signed-off-by: mulhern amulhern@redhat.com --- tests/devicelibs_test/mdraid_interrogate_test.py | 220 +++++++++++++++++++++++ tests/devicelibs_test/mdraid_test.py | 169 ++++++++--------- 2 files changed, 291 insertions(+), 98 deletions(-) create mode 100755 tests/devicelibs_test/mdraid_interrogate_test.py
diff --git a/tests/devicelibs_test/mdraid_interrogate_test.py b/tests/devicelibs_test/mdraid_interrogate_test.py new file mode 100755 index 0000000..4399da9 --- /dev/null +++ b/tests/devicelibs_test/mdraid_interrogate_test.py @@ -0,0 +1,220 @@ +#!/usr/bin/python +import unittest +import time +import uuid + +import blivet.devicelibs.raid as raid +import blivet.devicelibs.mdraid as mdraid + +from . import mdraid_test + + +class MDRaidInterrogateTestCase(mdraid_test.MDRaidAsRootTestCase): + + def _matchNames(self, found, expected, transient=None): + """ Match names found against expected names. + + :param found: a list of names found in result + :type found: list of str + :param expected: a list of expected names + :type expected: list of str + :param transient: a list of names that only sometimes appear + :type transient: list of str + """ + transient = transient or [] + + for n in (n for n in expected if n not in transient): + self.assertIn(n, found, msg="name '%s' not in info" % n) + + for n in (n for n in found if n not in transient): + self.assertIn(n, expected, msg="unexpected name '%s' in info" % n) + + +class MDExamineTestCase(MDRaidInterrogateTestCase): + + def __init__(self, methodName='runTest'): + super(MDExamineTestCase, self).__init__(methodName=methodName, deviceSpec=[102400, 102400, 102400]) + + names_0 = [ + 'DEVICE', + 'MD_DEVICES', + 'MD_EVENTS', + 'MD_LEVEL', + 'MD_UPDATE_TIME', + 'MD_UUID' + ] + + names_1 = [ + 'DEVICE', + 'MD_ARRAY_SIZE', + 'MD_DEV_UUID', + 'MD_DEVICES', + 'MD_EVENTS', + 'MD_LEVEL', + 'MD_METADATA', + 'MD_NAME', + 'MD_UPDATE_TIME', + 'MD_UUID' + ] + + names_container = [ + 'MD_DEVICES', + 'MD_LEVEL', + 'MD_METADATA', + 'MD_UUID' + ] + + def testMDExamineMDRaidArray(self): + mdraid.mdcreate(self._dev_name, raid.RAID1, self.loopDevices) + time.sleep(2) # wait for raid to settle + + # examining the array itself yield no data + info = mdraid.mdexamine(self._dev_name) + self.assertEqual(info, {}) + + def testMDExamineNonMDRaid(self): + # invoking mdexamine on a device that is not an array member yields {} + info = mdraid.mdexamine(self.loopDevices[0]) + self.assertEqual(info, {}) + + def _testMDExamine(self, names, metadataVersion=None, level=None, spares=0): + """ Test mdexamine for a specified metadataVersion. + + :param list names: mdexamine's expected list of names to return + :param str metadataVersion: the metadata version for the array + :param object level: any valid RAID level descriptor + :param int spares: the number of spares in this array + + Verifies that: + - exactly the predicted names are returned by mdexamine + - RAID level and number of devices are correct + - UUIDs have canonical form + """ + level = mdraid.RAID_levels.raidLevel(level or raid.RAID1) + mdraid.mdcreate(self._dev_name, level, self.loopDevices, metadataVer=metadataVersion, spares=spares) + time.sleep(2) # wait for raid to settle + + info = mdraid.mdexamine(self.loopDevices[0]) + + self._matchNames(info.keys(), names) + + # check names with predictable values + self.assertEqual(info['MD_DEVICES'], str(len(self.loopDevices) - spares)) + self.assertEqual(info['MD_LEVEL'], str(level)) + + # verify that uuids are in canonical form + for name in (k for k in iter(info.keys()) if k.endswith('UUID')): + self.assertEqual(str(uuid.UUID(info[name])), info[name]) + + def testMDExamineContainerDefault(self): + self._testMDExamine(self.names_container, level="container") + + def testMDExamineDefault(self): + self._testMDExamine(self.names_1) + + def testMDExamineSpares(self): + self._testMDExamine(self.names_1, spares=1) + + def testMDExamine0(self): + self._testMDExamine(self.names_0, metadataVersion='0') + + def testMDExamine0_90(self): + self._testMDExamine(self.names_0, metadataVersion='0.90') + + def testMDExamine1(self): + self._testMDExamine(self.names_1, metadataVersion='1') + + def testMDExamine1_2(self): + self._testMDExamine(self.names_1, metadataVersion='1.2') + +class MDDetailTestCase(MDRaidInterrogateTestCase): + + def __init__(self, methodName='runTest'): + super(MDDetailTestCase, self).__init__(methodName=methodName, deviceSpec=[102400, 102400, 102400]) + + names = [ + 'ACTIVE DEVICES', + 'ARRAY SIZE', + 'CREATION TIME', + 'EVENTS', + 'FAILED DEVICES', + 'NAME', + 'PERSISTENCE', + 'RAID DEVICES', + 'RAID LEVEL', + 'RESYNC STATUS', + 'SPARE DEVICES', + 'STATE', + 'TOTAL DEVICES', + 'UPDATE TIME', + 'USED DEV SIZE', + 'UUID', + 'VERSION', + 'WORKING DEVICES' + ] + + names_0 = [ + 'ACTIVE DEVICES', + 'ARRAY SIZE', + 'CREATION TIME', + 'EVENTS', + 'FAILED DEVICES', + 'PERSISTENCE', + 'PREFERRED MINOR', + 'RAID DEVICES', + 'RAID LEVEL', + 'RESYNC STATUS', + 'SPARE DEVICES', + 'STATE', + 'TOTAL DEVICES', + 'UPDATE TIME', + 'USED DEV SIZE', + 'UUID', + 'VERSION', + 'WORKING DEVICES' + ] + + def _testMDDetail(self, names, metadataVersion=None, level=None, spares=0): + """ Test mddetail for a specified metadataVersion. + + :param list names: mdexamine's expected list of names to return + :param str metadataVersion: the metadata version for the array + :param object level: any valid RAID level descriptor + :param int spares: the number of spares for this array + + Verifies that: + - exactly the predicted names are returned by mddetail + - UUIDs have canonical form + """ + level = mdraid.RAID_levels.raidLevel(level) if level is not None else raid.RAID1 + + mdraid.mdcreate(self._dev_name, level, self.loopDevices, metadataVer=metadataVersion, spares=spares) + time.sleep(2) # wait for raid to settle + + info = mdraid.mddetail(self._dev_name) + + # info contains values for exactly names + self._matchNames(info.keys(), names, ['RESYNC STATUS']) + + # check names with predictable values + self.assertEqual(info['ACTIVE DEVICES'], str(len(self.loopDevices) - spares)) + self.assertEqual(info['FAILED DEVICES'], '0') + self.assertEqual(info['RAID LEVEL'], str(level)) + self.assertEqual(info['SPARE DEVICES'], str(spares)) + self.assertEqual(info['TOTAL DEVICES'], str(len(self.loopDevices))) + self.assertEqual(info['WORKING DEVICES'], str(len(self.loopDevices))) + + # verify that uuid is in canonical form + self.assertEqual(str(uuid.UUID(info['UUID'])), info['UUID']) + + def testMDDetail(self): + self._testMDDetail(self.names) + + def testMDDetailSpares(self): + self._testMDDetail(self.names, spares=1) + + def testMDDetail0_90(self): + self._testMDDetail(self.names_0, metadataVersion='0.90') + +if __name__ == "__main__": + unittest.main() diff --git a/tests/devicelibs_test/mdraid_test.py b/tests/devicelibs_test/mdraid_test.py index 107423e..2ed7a2a 100755 --- a/tests/devicelibs_test/mdraid_test.py +++ b/tests/devicelibs_test/mdraid_test.py @@ -1,7 +1,6 @@ #!/usr/bin/python import unittest import time -import uuid
import blivet.devicelibs.raid as raid import blivet.devicelibs.mdraid as mdraid @@ -49,149 +48,123 @@ class MDRaidTestCase(unittest.TestCase): version="version"), mdraid.MD_SUPERBLOCK_SIZE)
- class MDRaidAsRootTestCase(loopbackedtestcase.LoopBackedTestCase):
- names_0 = [ - 'DEVICE', - 'MD_DEVICES', - 'MD_EVENTS', - 'MD_LEVEL', - 'MD_UPDATE_TIME', - 'MD_UUID' - ] - - names_1 = [ - 'DEVICE', - 'MD_ARRAY_SIZE', - 'MD_DEV_UUID', - 'MD_DEVICES', - 'MD_EVENTS', - 'MD_LEVEL', - 'MD_METADATA', - 'MD_NAME', - 'MD_UPDATE_TIME', - 'MD_UUID' - ] - - names_container = [ - 'MD_DEVICES', - 'MD_LEVEL', - 'MD_METADATA', - 'MD_UUID' - ] - - def __init__(self, methodName='runTest'): - """Set up the structure of the mdraid array.""" - super(MDRaidAsRootTestCase, self).__init__(methodName=methodName) + def __init__(self, methodName='runTest', deviceSpec=None): + super(MDRaidAsRootTestCase, self).__init__(methodName=methodName, deviceSpec=deviceSpec) self._dev_name = "/dev/md0"
def tearDown(self): try: mdraid.mddeactivate(self._dev_name) for dev in self.loopDevices: + mdraid.mdremove(self._dev_name, dev, fail=True) mdraid.mddestroy(dev) except MDRaidError: pass
super(MDRaidAsRootTestCase, self).tearDown()
- def testMDExamineMDRaidArray(self): - mdraid.mdcreate(self._dev_name, raid.RAID1, self.loopDevices) - # wait for raid to settle - time.sleep(2) - - # examining the array itself yield no data - info = mdraid.mdexamine(self._dev_name) - self.assertEqual(info, {}) +class RAID0Test(MDRaidAsRootTestCase):
- def testMDExamineNonMDRaid(self): - # invoking mdexamine on a device that is not an array member yields {} - info = mdraid.mdexamine(self.loopDevices[0]) - self.assertEqual(info, {}) - - def _testMDExamine(self, names, metadataVersion=None, level=None): - """ Test mdexamine for a specified metadataVersion. - - :param list names: mdexamine's expected list of names to return - :param str metadataVersion: the metadata version for the array - :param object level: any valid RAID level descriptor + def __init__(self, methodName='runTest'): + super(RAID0Test, self).__init__(methodName=methodName, deviceSpec=[102400, 102400, 102400])
- Verifies that: - - exactly the predicted names are returned by mdexamine - - RAID level and number of devices are correct - - UUIDs have canonical form - """ - level = mdraid.RAID_levels.raidLevel(level or raid.RAID1) - mdraid.mdcreate(self._dev_name, level, self.loopDevices, metadataVer=metadataVersion) - # wait for raid to settle - time.sleep(2) + def testGrow(self): + self.assertIsNone(mdraid.mdcreate(self._dev_name, raid.RAID0, self.loopDevices[:2])) + time.sleep(2) # wait for raid to settle
- info = mdraid.mdexamine(self.loopDevices[0]) + # it is not possible to add a managed device to a non-redundant array, + # but it can be grown, by specifying the desired number of members + self.assertIsNone(mdraid.mdadd(self._dev_name, self.loopDevices[2], raid_devices=3))
- # info contains values for exactly names - for n in names: - self.assertIn(n, info, msg="name '%s' not in info" % n) + def testGrowRAID1(self): + self.assertIsNone(mdraid.mdcreate(self._dev_name, raid.RAID1, self.loopDevices[:2])) + time.sleep(2) # wait for raid to settle
- for n in info.keys(): - self.assertIn(n, names, msg="unexpected name '%s' in info" % n) + # a RAID1 array can be grown as well as a RAID0 array + self.assertIsNone(mdraid.mdadd(self._dev_name, self.loopDevices[2], raid_devices=3))
- # check names with predictable values - self.assertEqual(info['MD_DEVICES'], '2') - self.assertEqual(info['MD_LEVEL'], str(level)) + def testGrowTooBig(self): + self.assertIsNone(mdraid.mdcreate(self._dev_name, raid.RAID0, self.loopDevices[:2])) + time.sleep(2) # wait for raid to settle
- # verify that uuids are in canonical form - for name in (k for k in iter(info.keys()) if k.endswith('UUID')): - self.assertTrue(str(uuid.UUID(info[name])) == info[name]) + # if more devices are specified than are available after the + # addition an error is raised + with self.assertRaises(MDRaidError): + mdraid.mdadd(self._dev_name, self.loopDevices[2], raid_devices=4)
- def testMDExamineContainerDefault(self): - self._testMDExamine(self.names_container, level="container") + def testGrowSmaller(self): + self.assertIsNone(mdraid.mdcreate(self._dev_name, raid.RAID0, self.loopDevices[:2])) + time.sleep(2) # wait for raid to settle
- def testMDExamineDefault(self): - self._testMDExamine(self.names_1) + # it is ok to grow an array smaller than its devices + self.assertIsNone(mdraid.mdadd(self._dev_name, self.loopDevices[2], raid_devices=2))
- def testMDExamine0(self): - self._testMDExamine(self.names_0, metadataVersion='0') + def testGrowSimple(self): + self.assertIsNone(mdraid.mdcreate(self._dev_name, raid.RAID0, self.loopDevices[:2])) + time.sleep(2) # wait for raid to settle
- def testMDExamine0_90(self): - self._testMDExamine(self.names_0, metadataVersion='0.90') + # try to simply add a device and things go wrong + with self.assertRaises(MDRaidError): + mdraid.mdadd(self._dev_name, self.loopDevices[2])
- def testMDExamine1(self): - self._testMDExamine(self.names_1, metadataVersion='1') +class SimpleRaidTest(MDRaidAsRootTestCase):
- def testMDExamine1_2(self): - self._testMDExamine(self.names_1, metadataVersion='1.2') + def __init__(self, methodName='runTest'): + super(SimpleRaidTest, self).__init__(methodName=methodName, deviceSpec=[102400, 102400, 102400])
def testMDRaidAsRoot(self): ## ## mdcreate ## # pass - self.assertEqual(mdraid.mdcreate(self._dev_name, raid.RAID1, self.loopDevices), None) + self.assertIsNone(mdraid.mdcreate(self._dev_name, raid.RAID1, self.loopDevices)) + time.sleep(2) # wait for raid to settle
# fail with self.assertRaises(MDRaidError): mdraid.mdcreate("/dev/md1", "raid1", ["/not/existing/dev0", "/not/existing/dev1"])
- ## - ## mddeactivate - ## - # pass - self.assertEqual(mdraid.mddeactivate(self._dev_name), None) - # fail with self.assertRaises(MDRaidError): - mdraid.mddeactivate("/not/existing/md") + mdraid.mdadd(self._dev_name, "/not/existing/device") + time.sleep(2) # wait for raid to settle + + # removing and re-adding a component device should succeed + self.assertIsNone(mdraid.mdremove(self._dev_name, self.loopDevices[2], fail=True)) + time.sleep(2) # wait for raid to settle + self.assertIsNone(mdraid.mdadd(self._dev_name, self.loopDevices[2])) + time.sleep(3) # wait for raid to settle + + # it is not possible to add a device that has already been added + with self.assertRaises(MDRaidError): + mdraid.mdadd(self._dev_name, self.loopDevices[2]) + + self.assertIsNone(mdraid.mdremove(self._dev_name, self.loopDevices[2], fail=True)) + time.sleep(2) # wait for raid to settle + + # can not re-add incrementally, because the array is active + with self.assertRaises(MDRaidError): + mdraid.mdadd(None, self.loopDevices[2], incremental=True)
## - ## mdadd + ## mddeactivate ## # pass - # TODO + self.assertIsNone(mdraid.mddeactivate(self._dev_name)) + + # once the array is deactivated, can add incrementally + self.assertIsNone(mdraid.mdadd(None, self.loopDevices[2], incremental=True)) + + # but cannot re-add twice + with self.assertRaises(MDRaidError): + mdraid.mdadd(None, self.loopDevices[2], incremental=True)
# fail with self.assertRaises(MDRaidError): - mdraid.mdadd(self._dev_name, "/not/existing/device") + mdraid.mddeactivate("/not/existing/md") +
## ## mdactivate @@ -207,7 +180,7 @@ class MDRaidAsRootTestCase(loopbackedtestcase.LoopBackedTestCase): ## # pass for dev in self.loopDevices: - self.assertEqual(mdraid.mddestroy(dev), None) + self.assertIsNone(mdraid.mddestroy(dev))
# pass # Note that these should fail because mdadm is unable to locate the
1) LinearRAID mdadm call will fail if raid_devices is passed. 2) RAID0 mdadm call will fail if the the raid_devices number is not exactly right, so use mddetail to find the current value and increment that by one.
Change calls accordingly. --- blivet/devicelibs/mdraid.py | 28 ++++++++++++++++++---------- blivet/devices.py | 14 +++++++++----- 2 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/blivet/devicelibs/mdraid.py b/blivet/devicelibs/mdraid.py index 8f81d1b..b572a2b 100644 --- a/blivet/devicelibs/mdraid.py +++ b/blivet/devicelibs/mdraid.py @@ -156,31 +156,39 @@ def mdnominate(device): except MDRaidError as msg: raise MDRaidError("mdnominate failed for %s: %s" % (device, msg))
-def mdadd(array, device, raid_devices=None): +def mdadd(array, device, grow_mode=False, raid_devices=None): """ Add a device to an array.
:param str array: path to the array to add the device to :param str device: path to the device to add to the array - :keyword int raid_devices: the number of active member devices + :keyword bool grow_mode: use grow mode + :keyword int raid_devices: the intended number of active member devices :rtype: NoneType :raises: MDRaidError
- The raid_devices parameter is used when adding devices to a raid + The grow_devices parameter is used when adding devices to a raid array that has no actual redundancy. In this case it is necessary to explicitly grow the array all at once rather than manage it in the sense of adding spares.
- Whether the new device will be added as a spare or an active member is - decided by mdadm. + If raid_devices is set, and grow_mode is True, then the intended + number of devices after this device is added is specified + using the --raid-devices flag. If grow is not True then raid_devices + is ignored. For linear arrays, specifying raid_devices will result + in a failure. + + Whether the new device will be added as a spare or an active member + when not in grow mode is decided by mdadm.
.. seealso:: mdnominate """ - if raid_devices is None: - args = [array, "--add"] + if grow_mode: + args = ["--grow", array] + if raid_devices: + args.extend(["--raid-devices", str(raid_devices)]) else: - args = ["--grow", array, "--raid-devices", str(raid_devices), "--add"] - - args.append(device) + args = [array] + args.extend(["--add", device])
try: mdadm(args) diff --git a/blivet/devices.py b/blivet/devices.py index 1028817..c7efd51 100644 --- a/blivet/devices.py +++ b/blivet/devices.py @@ -4033,12 +4033,16 @@ class MDRaidArrayDevice(ContainerDevice):
def _add(self, member): self.setup() - if self.level.has_redundancy: - raid_devices = None - else: - raid_devices = self.memberDevices
- mdraid.mdadd(self.path, member.path, raid_devices=raid_devices) + grow_mode = False + raid_devices = None + if not self.level.has_redundancy: + grow_mode = True + if self.level is not raid.Linear: + raid_devices = mdraid.mddetail(self.name)['RAID DEVICES'] + raid_devices = int(raid_devices) + 1 + + mdraid.mdadd(self.path, member.path, grow_mode=grow_mode, raid_devices=raid_devices)
@property def formatArgs(self):
Signed-off-by: mulhern amulhern@redhat.com --- tests/devicelibs_test/mdraid_test.py | 170 ++++++++++++++++++++++++++--------- tests/pylint/pylint-false-positives | 1 + 2 files changed, 128 insertions(+), 43 deletions(-)
diff --git a/tests/devicelibs_test/mdraid_test.py b/tests/devicelibs_test/mdraid_test.py index ad0c3c7..c1794a5 100755 --- a/tests/devicelibs_test/mdraid_test.py +++ b/tests/devicelibs_test/mdraid_test.py @@ -71,49 +71,133 @@ class MDRaidAsRootTestCase(loopbackedtestcase.LoopBackedTestCase):
super(MDRaidAsRootTestCase, self).tearDown()
-class RAID0Test(MDRaidAsRootTestCase): - - def __init__(self, methodName='runTest'): - super(RAID0Test, self).__init__(methodName=methodName, deviceSpec=[102400, 102400, 102400]) - - def testGrow(self): - self.assertIsNone(mdraid.mdcreate(self._dev_name, raid.RAID0, self.loopDevices[:2])) - time.sleep(2) # wait for raid to settle - - # it is not possible to add a managed device to a non-redundant array, - # but it can be grown, by specifying the desired number of members - self.assertIsNone(mdraid.mdadd(self._dev_name, self.loopDevices[2], raid_devices=3)) - - def testGrowRAID1(self): - self.assertIsNone(mdraid.mdcreate(self._dev_name, raid.RAID1, self.loopDevices[:2])) - time.sleep(2) # wait for raid to settle - - # a RAID1 array can be grown as well as a RAID0 array - self.assertIsNone(mdraid.mdadd(self._dev_name, self.loopDevices[2], raid_devices=3)) - - def testGrowTooBig(self): - self.assertIsNone(mdraid.mdcreate(self._dev_name, raid.RAID0, self.loopDevices[:2])) - time.sleep(2) # wait for raid to settle - - # if more devices are specified than are available after the - # addition an error is raised - with self.assertRaises(MDRaidError): - mdraid.mdadd(self._dev_name, self.loopDevices[2], raid_devices=4) - - def testGrowSmaller(self): - self.assertIsNone(mdraid.mdcreate(self._dev_name, raid.RAID0, self.loopDevices[:2])) - time.sleep(2) # wait for raid to settle - - # it is ok to grow an array smaller than its devices - self.assertIsNone(mdraid.mdadd(self._dev_name, self.loopDevices[2], raid_devices=2)) - - def testGrowSimple(self): - self.assertIsNone(mdraid.mdcreate(self._dev_name, raid.RAID0, self.loopDevices[:2])) - time.sleep(2) # wait for raid to settle - - # try to simply add a device and things go wrong - with self.assertRaises(MDRaidError): - mdraid.mdadd(self._dev_name, self.loopDevices[2]) +class TestCaseFactory(object): + + @staticmethod + def _minMembersForCreation(level): + min_members = level.min_members + return min_members if min_members > 1 else 2 + +class JustAddTestCaseFactory(TestCaseFactory): + + @staticmethod + def makeClass(name, level): + + def __init__(self, methodName='runTest'): + super(self.__class__, self).__init__(methodName=methodName, deviceSpec=[102400, 102400, 102400, 102400, 102400]) + self.longMessage = True + + def testAdd(self): + """ Tests adding, not growing, a device. """ + initial_members = TestCaseFactory._minMembersForCreation(level) + self.assertIsNone(mdraid.mdcreate(self._dev_name, level, self.loopDevices[:initial_members])) + time.sleep(2) # wait for raid to settle + + new_member = self.loopDevices[initial_members] + + if level.has_redundancy: + info_pre = mdraid.mddetail(self._dev_name) + self.assertIsNone(mdraid.mdadd(self._dev_name, new_member)) + info_post = mdraid.mddetail(self._dev_name) + keys = ['TOTAL DEVICES', 'WORKING DEVICES'] + for k in keys: + self.assertEqual(int(info_pre[k]) + 1, int(info_post[k]), msg="key: %s" % k) + if level is not raid.Container: + keys = ['ACTIVE DEVICES', 'SPARE DEVICES'] + self.assertEqual(sum(int(info_pre[k]) for k in keys) + 1, + sum(int(info_post[k]) for k in keys)) + self.assertEqual(info_pre['RAID DEVICES'], info_post['RAID DEVICES']) + dev_info = mdraid.mdexamine(new_member) + self.assertEqual(info_post['UUID'], dev_info['MD_UUID']) + else: + with self.assertRaises(MDRaidError): + mdraid.mdadd(self._dev_name, new_member) + + return type( + name, + (MDRaidAsRootTestCase,), + { + '__init__': __init__, + 'testAdd': testAdd, + }) + +class GrowTestCaseFactory(object): + + @staticmethod + def makeClass(name, level): + + def __init__(self, methodName='runTest'): + super(self.__class__, self).__init__(methodName=methodName, deviceSpec=[102400, 102400, 102400, 102400, 102400, 102400]) + self.longMessage = True + + def testGrow(self): + """ Tests growing a device by exactly 1. """ + initial_members = TestCaseFactory._minMembersForCreation(level) + self.assertIsNone(mdraid.mdcreate(self._dev_name, level, self.loopDevices[:initial_members])) + time.sleep(3) # wait for raid to settle + + new_member = self.loopDevices[initial_members] + info_pre = mdraid.mddetail(self._dev_name) + raid_devices = None if level is raid.Linear else initial_members + 1 + self.assertIsNone(mdraid.mdadd(self._dev_name, new_member, grow_mode=True, raid_devices=raid_devices)) + info_post = mdraid.mddetail(self._dev_name) + keys = ['RAID DEVICES', 'TOTAL DEVICES', 'WORKING DEVICES'] + for k in keys: + self.assertEqual(int(info_pre[k]) + 1, int(info_post[k]), msg="key: %s" % k) + if level is not raid.Container: + keys = ['ACTIVE DEVICES', 'SPARE DEVICES'] + self.assertEqual(sum(int(info_pre[k]) for k in keys) + 1, + sum(int(info_post[k]) for k in keys), + msg="%s" % " + ".join(keys)) + dev_info = mdraid.mdexamine(new_member) + self.assertEqual(info_post['UUID'], dev_info['MD_UUID'], msg="key: UUID") + + + def testGrowBig(self): + """ Test growing a device beyond its size. """ + initial_members = TestCaseFactory._minMembersForCreation(level) + self.assertIsNone(mdraid.mdcreate(self._dev_name, level, self.loopDevices[:initial_members])) + time.sleep(3) # wait for raid to settle + + new_member = self.loopDevices[initial_members] + if level is raid.Linear: + self.assertIsNone(mdraid.mdadd(self._dev_name, new_member, grow_mode=True)) + else: + with self.assertRaises(MDRaidError): + mdraid.mdadd(self._dev_name, new_member, grow_mode=True, raid_devices=initial_members + 2) + + def testGrowSmall(self): + """ Test decreasing size of device. """ + initial_members = TestCaseFactory._minMembersForCreation(level) + 1 + self.assertIsNone(mdraid.mdcreate(self._dev_name, level, self.loopDevices[:initial_members])) + time.sleep(2) # wait for raid to settle + + new_member = self.loopDevices[initial_members] + if level is not raid.Linear: + with self.assertRaises(MDRaidError): + mdraid.mdadd(self._dev_name, new_member, grow_mode=True, raid_devices=initial_members - 1) + + return type( + name, + (MDRaidAsRootTestCase,), + { + '__init__': __init__, + 'testGrow': testGrow, + 'testGrowBig' : testGrowBig, + 'testGrowSmall': testGrowSmall + }) + + +# make some test cases for every RAID level +levels = list(mdraid.RAID_levels) +levels.sort(cmp=lambda x, y: cmp(x.name, y.name)) # order tests predictably +for l in levels: + classname = "%sJustAddTestCase" % l + globals()[classname] = JustAddTestCaseFactory.makeClass(classname, l) + + if l is not raid.Container: + classname = "%sGrowTestCase" % l + globals()[classname] = GrowTestCaseFactory.makeClass(classname, l)
class SimpleRaidTest(MDRaidAsRootTestCase):
diff --git a/tests/pylint/pylint-false-positives b/tests/pylint/pylint-false-positives index 78e71ad..6ffaf7b 100644 --- a/tests/pylint/pylint-false-positives +++ b/tests/pylint/pylint-false-positives @@ -18,5 +18,6 @@ ^blivet/__init__.py:[[:digit:]]+: [W0612(unused-variable), enable_installer_mode] Unused variable 'ROOT_PATH'$ ^blivet/__init__.py:[[:digit:]]+: [W0612(unused-variable), enable_installer_mode] Unused variable 'shortProductName'$ ^dm.c: [[:digit:]]+: not running as root returning empty list$ +^tests/devicelibs_test/mdraid_test.py:[[:digit:]]+: [E1003(bad-super-call), ([[:alnum:].]+).__init__] Bad first argument YES given to super()$ ^tests/devicelibs_test/raid_test.py:[[:digit:]]+: [E1120(no-value-for-parameter), [[:alnum:].]+] No value [[:alnum:] ]+ 'member_count' in [[:alnum:] ]+ call$ ^tests/devicelibs_test/raid_test.py:[[:digit:]]+: [E1120(no-value-for-parameter), [[:alnum:].]+] No value [[:alnum:] ]+ 'smallest_member_size' in [[:alnum:] ]+ call$
On Fri, 2014-07-25 at 12:39 -0400, mulhern wrote:
Signed-off-by: mulhern amulhern@redhat.com
tests/devicelibs_test/mdraid_test.py | 170 ++++++++++++++++++++++++++--------- tests/pylint/pylint-false-positives | 1 + 2 files changed, 128 insertions(+), 43 deletions(-)
diff --git a/tests/devicelibs_test/mdraid_test.py b/tests/devicelibs_test/mdraid_test.py index ad0c3c7..c1794a5 100755 --- a/tests/devicelibs_test/mdraid_test.py +++ b/tests/devicelibs_test/mdraid_test.py @@ -71,49 +71,133 @@ class MDRaidAsRootTestCase(loopbackedtestcase.LoopBackedTestCase):
super(MDRaidAsRootTestCase, self).tearDown()-class RAID0Test(MDRaidAsRootTestCase):
- def __init__(self, methodName='runTest'):
super(RAID0Test, self).__init__(methodName=methodName, deviceSpec=[102400, 102400, 102400])- def testGrow(self):
self.assertIsNone(mdraid.mdcreate(self._dev_name, raid.RAID0, self.loopDevices[:2]))time.sleep(2) # wait for raid to settle# it is not possible to add a managed device to a non-redundant array,# but it can be grown, by specifying the desired number of membersself.assertIsNone(mdraid.mdadd(self._dev_name, self.loopDevices[2], raid_devices=3))- def testGrowRAID1(self):
self.assertIsNone(mdraid.mdcreate(self._dev_name, raid.RAID1, self.loopDevices[:2]))time.sleep(2) # wait for raid to settle# a RAID1 array can be grown as well as a RAID0 arrayself.assertIsNone(mdraid.mdadd(self._dev_name, self.loopDevices[2], raid_devices=3))- def testGrowTooBig(self):
self.assertIsNone(mdraid.mdcreate(self._dev_name, raid.RAID0, self.loopDevices[:2]))time.sleep(2) # wait for raid to settle# if more devices are specified than are available after the# addition an error is raisedwith self.assertRaises(MDRaidError):mdraid.mdadd(self._dev_name, self.loopDevices[2], raid_devices=4)- def testGrowSmaller(self):
self.assertIsNone(mdraid.mdcreate(self._dev_name, raid.RAID0, self.loopDevices[:2]))time.sleep(2) # wait for raid to settle# it is ok to grow an array smaller than its devicesself.assertIsNone(mdraid.mdadd(self._dev_name, self.loopDevices[2], raid_devices=2))- def testGrowSimple(self):
self.assertIsNone(mdraid.mdcreate(self._dev_name, raid.RAID0, self.loopDevices[:2]))time.sleep(2) # wait for raid to settle# try to simply add a device and things go wrongwith self.assertRaises(MDRaidError):mdraid.mdadd(self._dev_name, self.loopDevices[2])+class TestCaseFactory(object):
- @staticmethod
- def _minMembersForCreation(level):
min_members = level.min_membersreturn min_members if min_members > 1 else 2+class JustAddTestCaseFactory(TestCaseFactory):
- @staticmethod
- def makeClass(name, level):
def __init__(self, methodName='runTest'):super(self.__class__, self).__init__(methodName=methodName, deviceSpec=[102400, 102400, 102400, 102400, 102400])self.longMessage = Truedef testAdd(self):""" Tests adding, not growing, a device. """initial_members = TestCaseFactory._minMembersForCreation(level)self.assertIsNone(mdraid.mdcreate(self._dev_name, level, self.loopDevices[:initial_members]))time.sleep(2) # wait for raid to settlenew_member = self.loopDevices[initial_members]if level.has_redundancy:info_pre = mdraid.mddetail(self._dev_name)self.assertIsNone(mdraid.mdadd(self._dev_name, new_member))info_post = mdraid.mddetail(self._dev_name)keys = ['TOTAL DEVICES', 'WORKING DEVICES']for k in keys:self.assertEqual(int(info_pre[k]) + 1, int(info_post[k]), msg="key: %s" % k)if level is not raid.Container:keys = ['ACTIVE DEVICES', 'SPARE DEVICES']self.assertEqual(sum(int(info_pre[k]) for k in keys) + 1,sum(int(info_post[k]) for k in keys))self.assertEqual(info_pre['RAID DEVICES'], info_post['RAID DEVICES'])dev_info = mdraid.mdexamine(new_member)self.assertEqual(info_post['UUID'], dev_info['MD_UUID'])else:with self.assertRaises(MDRaidError):mdraid.mdadd(self._dev_name, new_member)return type(name,(MDRaidAsRootTestCase,),{'__init__': __init__,'testAdd': testAdd,})+class GrowTestCaseFactory(object):
- @staticmethod
- def makeClass(name, level):
def __init__(self, methodName='runTest'):super(self.__class__, self).__init__(methodName=methodName, deviceSpec=[102400, 102400, 102400, 102400, 102400, 102400])self.longMessage = Truedef testGrow(self):""" Tests growing a device by exactly 1. """initial_members = TestCaseFactory._minMembersForCreation(level)self.assertIsNone(mdraid.mdcreate(self._dev_name, level, self.loopDevices[:initial_members]))time.sleep(3) # wait for raid to settlenew_member = self.loopDevices[initial_members]info_pre = mdraid.mddetail(self._dev_name)raid_devices = None if level is raid.Linear else initial_members + 1self.assertIsNone(mdraid.mdadd(self._dev_name, new_member, grow_mode=True, raid_devices=raid_devices))info_post = mdraid.mddetail(self._dev_name)keys = ['RAID DEVICES', 'TOTAL DEVICES', 'WORKING DEVICES']for k in keys:self.assertEqual(int(info_pre[k]) + 1, int(info_post[k]), msg="key: %s" % k)if level is not raid.Container:keys = ['ACTIVE DEVICES', 'SPARE DEVICES']self.assertEqual(sum(int(info_pre[k]) for k in keys) + 1,sum(int(info_post[k]) for k in keys),msg="%s" % " + ".join(keys))dev_info = mdraid.mdexamine(new_member)self.assertEqual(info_post['UUID'], dev_info['MD_UUID'], msg="key: UUID")def testGrowBig(self):""" Test growing a device beyond its size. """initial_members = TestCaseFactory._minMembersForCreation(level)self.assertIsNone(mdraid.mdcreate(self._dev_name, level, self.loopDevices[:initial_members]))time.sleep(3) # wait for raid to settlenew_member = self.loopDevices[initial_members]if level is raid.Linear:self.assertIsNone(mdraid.mdadd(self._dev_name, new_member, grow_mode=True))else:with self.assertRaises(MDRaidError):mdraid.mdadd(self._dev_name, new_member, grow_mode=True, raid_devices=initial_members + 2)def testGrowSmall(self):""" Test decreasing size of device. """initial_members = TestCaseFactory._minMembersForCreation(level) + 1self.assertIsNone(mdraid.mdcreate(self._dev_name, level, self.loopDevices[:initial_members]))time.sleep(2) # wait for raid to settlenew_member = self.loopDevices[initial_members]if level is not raid.Linear:with self.assertRaises(MDRaidError):mdraid.mdadd(self._dev_name, new_member, grow_mode=True, raid_devices=initial_members - 1)return type(name,(MDRaidAsRootTestCase,),{'__init__': __init__,'testGrow': testGrow,'testGrowBig' : testGrowBig,'testGrowSmall': testGrowSmall})+# make some test cases for every RAID level +levels = list(mdraid.RAID_levels) +levels.sort(cmp=lambda x, y: cmp(x.name, y.name)) # order tests predictably
These two lines can be made into one as: levels = sorted(mdraid.RAID_levels, cmp=....)
+for l in levels:
- classname = "%sJustAddTestCase" % l
- globals()[classname] = JustAddTestCaseFactory.makeClass(classname, l)
- if l is not raid.Container:
classname = "%sGrowTestCase" % lglobals()[classname] = GrowTestCaseFactory.makeClass(classname, l)class SimpleRaidTest(MDRaidAsRootTestCase):
diff --git a/tests/pylint/pylint-false-positives b/tests/pylint/pylint-false-positives index 78e71ad..6ffaf7b 100644 --- a/tests/pylint/pylint-false-positives +++ b/tests/pylint/pylint-false-positives @@ -18,5 +18,6 @@ ^blivet/__init__.py:[[:digit:]]+: [W0612(unused-variable), enable_installer_mode] Unused variable 'ROOT_PATH'$ ^blivet/__init__.py:[[:digit:]]+: [W0612(unused-variable), enable_installer_mode] Unused variable 'shortProductName'$ ^dm.c: [[:digit:]]+: not running as root returning empty list$ +^tests/devicelibs_test/mdraid_test.py:[[:digit:]]+: [E1003(bad-super-call), ([[:alnum:].]+).__init__] Bad first argument YES given to super()$ ^tests/devicelibs_test/raid_test.py:[[:digit:]]+: [E1120(no-value-for-parameter), [[:alnum:].]+] No value [[:alnum:] ]+ 'member_count' in [[:alnum:] ]+ call$ ^tests/devicelibs_test/raid_test.py:[[:digit:]]+: [E1120(no-value-for-parameter), [[:alnum:].]+] No value [[:alnum:] ]+ 'smallest_member_size' in [[:alnum:] ]+ call$
Otherwise this patch is really a big dirty hack, I'd say, but I cannot think of any better solution and I think we can live with something like this is tests.
Incremental mode is really considerably different from add mode.
For one thing, it may cause the array to be started if it was not already, based on the number of devices in the array and the number that ought to be in the array.
Signed-off-by: mulhern amulhern@redhat.com --- blivet/devicelibs/mdraid.py | 32 +++++++++++++++++++++----------- blivet/devices.py | 2 +- tests/devicelibs_test/mdraid_test.py | 10 +++++----- 3 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/blivet/devicelibs/mdraid.py b/blivet/devicelibs/mdraid.py index a2745b6..494fa48 100644 --- a/blivet/devicelibs/mdraid.py +++ b/blivet/devicelibs/mdraid.py @@ -125,12 +125,30 @@ def mddestroy(device): except MDRaidError as msg: raise MDRaidError("mddestroy failed for %s: %s" % (device, msg))
-def mdadd(array, device, incremental=False, raid_devices=None): +def mdnominate(device): + """ Attempt to add a device to the array to which it belongs. + + Belonging is determined by mdadm's rules. + + May start the array once a sufficient number of devices are added + to the array. + + :param str device: path to the device to add + :rtype: NoneType + :raises: MDRaidError + """ + args = ['--incremental', '--quiet', device] + + try: + mdadm(args) + except MDRaidError as msg: + raise MDRaidError("mdadd failed for %s: %s" % (device, msg)) + +def mdadd(array, device, raid_devices=None): """ Add a device to an array.
:param str array: path to the array to add the device to :param str device: path to the device to add to the array - :keyword bool incremental: add the device incrementally (see note below) :keyword int raid_devices: the number of active member devices
The raid_devices parameter is used when adding devices to a raid @@ -140,16 +158,8 @@ def mdadd(array, device, incremental=False, raid_devices=None):
Whether the new device will be added as a spare or an active member is decided by mdadm. - - .. note:: - - Incremental add is used during block device discovery and is a - different operation than changing the member set of an array. - """ - if incremental: - args = ["--incremental", "--quiet"] - elif raid_devices is None: + if raid_devices is None: args = [array, "--add"] else: args = ["--grow", array, "--raid-devices", str(raid_devices), "--add"] diff --git a/blivet/devices.py b/blivet/devices.py index cb29931..f40ddd6 100644 --- a/blivet/devices.py +++ b/blivet/devices.py @@ -3813,7 +3813,7 @@ class MDRaidArrayDevice(ContainerDevice):
if self.spares <= 0: try: - mdraid.mdadd(None, member.path, incremental=True) + mdraid.mdnominate(member.path) # mdadd causes udev events udev.settle() except errors.MDRaidError as e: diff --git a/tests/devicelibs_test/mdraid_test.py b/tests/devicelibs_test/mdraid_test.py index 2ed7a2a..042f1eb 100755 --- a/tests/devicelibs_test/mdraid_test.py +++ b/tests/devicelibs_test/mdraid_test.py @@ -144,9 +144,9 @@ class SimpleRaidTest(MDRaidAsRootTestCase): self.assertIsNone(mdraid.mdremove(self._dev_name, self.loopDevices[2], fail=True)) time.sleep(2) # wait for raid to settle
- # can not re-add incrementally, because the array is active + # can not re-add in incremental mode because the array is active with self.assertRaises(MDRaidError): - mdraid.mdadd(None, self.loopDevices[2], incremental=True) + mdraid.mdnominate(self.loopDevices[2])
## ## mddeactivate @@ -154,12 +154,12 @@ class SimpleRaidTest(MDRaidAsRootTestCase): # pass self.assertIsNone(mdraid.mddeactivate(self._dev_name))
- # once the array is deactivated, can add incrementally - self.assertIsNone(mdraid.mdadd(None, self.loopDevices[2], incremental=True)) + # once the array is deactivated, can add in incremental mode + self.assertIsNone(mdraid.mdnominate(self.loopDevices[2]))
# but cannot re-add twice with self.assertRaises(MDRaidError): - mdraid.mdadd(None, self.loopDevices[2], incremental=True) + mdraid.mdnominate(self.loopDevices[2])
# fail with self.assertRaises(MDRaidError):
----- Original Message -----
From: "mulhern" amulhern@redhat.com To: anaconda-patches@lists.fedorahosted.org Cc: "mulhern" amulhern@redhat.com Sent: Friday, July 25, 2014 12:39:14 PM Subject: [blivet:master 0/8] mdadd fixing patch set
Only the last two patches are even remotely interesting.
mulhern (8): Fix mdnominate error message. Use long messages for unittest errors. Still attempt to destroy even if remove failed. Add a test for mddetail on containers. Change Container.has_redundancy to True Add a test for activation. Revise mdadd. Add a bunch of tests for mdadd.
blivet/devicelibs/mdraid.py | 30 ++-- blivet/devicelibs/raid.py | 2 +- blivet/devices.py | 14 +- tests/devicelibs_test/mdraid_interrogate_test.py | 22 ++- tests/devicelibs_test/mdraid_test.py | 196 +++++++++++++++++------ tests/pylint/pylint-false-positives | 1 + 6 files changed, 197 insertions(+), 68 deletions(-)
-- 1.9.3
Sorry, I managed to put my new patches in with some old ones and sent them all. Please get rid of the ones that are not listed here, you've already reviewed them.
- mulhern
On Fri, 2014-07-25 at 13:09 -0400, Anne Mulhern wrote:
----- Original Message -----
From: "mulhern" amulhern@redhat.com To: anaconda-patches@lists.fedorahosted.org Cc: "mulhern" amulhern@redhat.com Sent: Friday, July 25, 2014 12:39:14 PM Subject: [blivet:master 0/8] mdadd fixing patch set
Only the last two patches are even remotely interesting.
mulhern (8): Fix mdnominate error message. Use long messages for unittest errors. Still attempt to destroy even if remove failed. Add a test for mddetail on containers. Change Container.has_redundancy to True Add a test for activation. Revise mdadd. Add a bunch of tests for mdadd.
Apart from the two comments these look good to me.
anaconda-patches@lists.fedorahosted.org
-
Anne Mulhern -
Vratislav Podzimek