[blivet:master 05/26] No longer use _standard_levels as the default set of RAID levels.
Vratislav Podzimek
vpodzime at redhat.com
Fri May 30 07:14:29 UTC 2014
On Thu, 2014-05-29 at 14:47 -0400, mulhern wrote:
> The notion of a "standard" RAID level is being eroded too rapidly to make it
> really useful. ALL_LEVELS is a better notion.
> This change has the added benefit of simplifying the RAIDlevels constructor.
> Also, make _raid_levels property a set instead of a list since no real
> ordering make sense.
I believe the ordering makes sense -- users would be quite confused if
we offered them to choose from the following RAID levels:
1. RAID 4
2. RAID 0
3. RAID 5
4. RAID 10
...
Maybe the RAID levels don't have to be ordered internally, but when
queried, they should be returned as a list from the lowest number to the
biggest one. Anaconda does the ordering by its Glade files, but that
doesn't have to be the case of other tools using blivet.
>
> Signed-off-by: mulhern <amulhern at redhat.com>
> ---
> blivet/devicelibs/mdraid.py | 2 +-
> blivet/devicelibs/raid.py | 77 ++++++++++++--------------------------
> tests/devicelibs_test/raid_test.py | 13 ++++---
> 3 files changed, 31 insertions(+), 61 deletions(-)
>
> diff --git a/blivet/devicelibs/mdraid.py b/blivet/devicelibs/mdraid.py
> index aeb554c..2106548 100644
> --- a/blivet/devicelibs/mdraid.py
> +++ b/blivet/devicelibs/mdraid.py
> @@ -44,7 +44,7 @@ class MDRaidLevels(raid.RAIDLevels):
> except AttributeError:
> return False
>
> -_RAID_levels = MDRaidLevels()
> +_RAID_levels = MDRaidLevels(["raid0", "raid1", "raid4", "raid5", "raid6", "raid10"])
>
> class Container(object):
> name = "container"
> diff --git a/blivet/devicelibs/raid.py b/blivet/devicelibs/raid.py
> index bdc17f7..86b584e 100644
> --- a/blivet/devicelibs/raid.py
> +++ b/blivet/devicelibs/raid.py
> @@ -245,38 +245,24 @@ class RAIDLevels(object):
> for its RAID levels.
> """
>
> - def __init__(self, levels=True):
> + def __init__(self, levels=None):
> """Add the specified standard levels to the levels in this object.
>
> - :param levels: the standard levels to be added to this object
> - :type levels: bool or a list of valid RAID level descriptors
> + :param levels: the levels to be added to this object
> + :type levels: list of valid RAID level descriptors
>
> If levels is True, add all standard levels. Else, levels
> must be a list of valid level descriptors of standard levels.
> Duplicate descriptors are ignored.
> """
> - self._raid_levels = []
> - if levels is False:
> - pass
> - elif levels is True:
> - for l in self.standard_levels:
> - self.addRaidLevel(l)
> - else:
> - try:
> - for level in levels:
> - matches = [l for l in self.standard_levels if level in l.names]
> - if len(matches) != 1:
> - raise RaidError("invalid standard RAID level descriptor %s" % level)
> - else:
> - self.addRaidLevel(matches[0])
> - except TypeError:
> - raise RaidError("levels must be a boolean or an iterable")
> -
> -
> - _standard_levels = []
> -
> - standard_levels = property(lambda s: s._standard_levels,
> - doc="any standard RAID level classes defined in this package.")
> + levels = levels or []
> + self._raid_levels = set()
> + for level in levels:
> + matches = [l for l in ALL_LEVELS if level in l.names]
> + if len(matches) != 1:
> + raise RaidError("invalid standard RAID level descriptor %s" % level)
> + else:
> + self.addRaidLevel(matches[0])
>
> @classmethod
> def isRaidLevel(cls, level):
> @@ -291,12 +277,11 @@ class RAIDLevels(object):
> The name property must be defined; it should be one of the
> elements in the names list.
>
> - All RAID objects in standard_levels are guaranteed to pass these
> + All RAID objects that extend RAIDlevel are guaranteed to pass these
> minimum requirements.
>
> This method should not be overridden in any subclass so that it
> - is so restrictive that a RAID object in standard_levels does
> - not satisfy it.
> + is so restrictive that a RAIDlevel object does not satisfy it.
> """
> try:
> name = level.names[0]
> @@ -307,23 +292,6 @@ class RAIDLevels(object):
> return False
> return True
>
> - @classmethod
> - def addRAIDLevelToStandardLevels(cls, level):
> - """Adds this RAID level to the internal list of standard RAID levels
> - defined in this package.
> -
> - :param level: an object representing a RAID level
> - :type level: object
> -
> - Raises a RaidError if level is not a valid RAID level.
> -
> - Does not allow duplicate level objects.
> - """
> - if not cls.isRaidLevel(level):
> - raise RaidError('level is not a valid RAID level')
> - if not level in cls._standard_levels:
> - cls._standard_levels.append(level)
> -
> def raidLevel(self, descriptor):
> """Return RAID object corresponding to descriptor.
>
> @@ -338,7 +306,7 @@ class RAIDLevels(object):
> raise RaidError("invalid RAID level descriptor %s" % descriptor)
>
> def addRaidLevel(self, level):
> - """Adds level to the list of levels if it is not already there.
> + """Adds level to levels if it is not already there.
>
> :param object level: an object representing a RAID level
>
> @@ -348,12 +316,13 @@ class RAIDLevels(object):
> """
> if not self.isRaidLevel(level):
> raise RaidError("level is not a valid RAID level")
> - if not level in self._raid_levels:
> - self._raid_levels.append(level)
> + self._raid_levels.add(level)
>
> def __iter__(self):
> return iter(self._raid_levels)
>
> +ALL_LEVELS = RAIDLevels()
> +
> class RAID0(RAIDLevel):
>
> level = property(lambda s: "0")
> @@ -376,7 +345,7 @@ class RAID0(RAIDLevel):
> return member_count * 16
>
> RAID0 = RAID0()
> -RAIDLevels.addRAIDLevelToStandardLevels(RAID0)
> +ALL_LEVELS.addRaidLevel(RAID0)
>
> class RAID1(RAIDLevel):
> level = property(lambda s: "1")
> @@ -399,7 +368,7 @@ class RAID1(RAIDLevel):
> return None
>
> RAID1 = RAID1()
> -RAIDLevels.addRAIDLevelToStandardLevels(RAID1)
> +ALL_LEVELS.addRaidLevel(RAID1)
>
> class RAID4(RAIDLevel):
> level = property(lambda s: "4")
> @@ -422,7 +391,7 @@ class RAID4(RAIDLevel):
> return (member_count - 1) * 16
>
> RAID4 = RAID4()
> -RAIDLevels.addRAIDLevelToStandardLevels(RAID4)
> +ALL_LEVELS.addRaidLevel(RAID4)
>
> class RAID5(RAIDLevel):
> level = property(lambda s: "5")
> @@ -445,7 +414,7 @@ class RAID5(RAIDLevel):
> return (member_count - 1) * 16
>
> RAID5 = RAID5()
> -RAIDLevels.addRAIDLevelToStandardLevels(RAID5)
> +ALL_LEVELS.addRaidLevel(RAID5)
>
> class RAID6(RAIDLevel):
> level = property(lambda s: "6")
> @@ -468,7 +437,7 @@ class RAID6(RAIDLevel):
> return None
>
> RAID6 = RAID6()
> -RAIDLevels.addRAIDLevelToStandardLevels(RAID6)
> +ALL_LEVELS.addRaidLevel(RAID6)
>
> class RAID10(RAIDLevel):
> level = property(lambda s: "10")
> @@ -491,4 +460,4 @@ class RAID10(RAIDLevel):
> return None
>
> RAID10 = RAID10()
> -RAIDLevels.addRAIDLevelToStandardLevels(RAID10)
> +ALL_LEVELS.addRaidLevel(RAID10)
> diff --git a/tests/devicelibs_test/raid_test.py b/tests/devicelibs_test/raid_test.py
> index 7c7501b..e192aea 100644
> --- a/tests/devicelibs_test/raid_test.py
> +++ b/tests/devicelibs_test/raid_test.py
> @@ -1,6 +1,7 @@
> #!/usr/bin/python
> import unittest
>
> +import blivet.devicelibs.mdraid as mdraid
> import blivet.devicelibs.raid as raid
> import blivet.errors as errors
> from blivet.size import Size
> @@ -8,8 +9,8 @@ from blivet.size import Size
> class RaidTestCase(unittest.TestCase):
>
> def setUp(self):
> - self.levels = raid.RAIDLevels()
> - self.levels_none = raid.RAIDLevels(False)
> + self.levels = raid.RAIDLevels(["raid0", "raid1", "raid4", "raid5", "raid6", "raid10"])
> + self.levels_none = raid.RAIDLevels([])
> self.levels_some = raid.RAIDLevels(["mirror", 6])
>
> def testRaid(self):
> @@ -99,7 +100,7 @@ class RaidTestCase(unittest.TestCase):
> ##
> ## size
> ##
> - for r in raid.RAIDLevels():
> + for r in [l for l in raid.ALL_LEVELS if l is not mdraid.Container]:
This and all the other similar lines should use generator comprehension
instead of list comprehension.
> self.assertEqual(r.get_size([ Size("32MiB"),
> Size("128MiB"),
> Size("128MiB"),
> @@ -109,7 +110,7 @@ class RaidTestCase(unittest.TestCase):
> lambda x: Size(0)),
> r.get_net_array_size(4, Size("32MiB")))
>
> - for r in raid.RAIDLevels():
> + for r in [l for l in raid.ALL_LEVELS if l is not mdraid.Container]:
> self.assertEqual(r.get_size([ Size("32MiB"),
> Size("128MiB"),
> Size("128MiB"),
> @@ -119,7 +120,7 @@ class RaidTestCase(unittest.TestCase):
> lambda x: Size(0)),
> r.get_net_array_size(5, Size("32MiB")))
>
> - for r in raid.RAIDLevels():
> + for r in [l for l in raid.ALL_LEVELS if l is not mdraid.Container]:
> self.assertEqual(r.get_size([ Size("32MiB"),
> Size("128MiB"),
> Size("128MiB"),
> @@ -129,7 +130,7 @@ class RaidTestCase(unittest.TestCase):
> lambda x: Size("32MiB")),
> 0)
>
> - for r in raid.RAIDLevels():
> + for r in [l for l in raid.ALL_LEVELS if l is not mdraid.Container]:
> self.assertEqual(r.get_size([ Size("32MiB"),
> Size("128MiB"),
> Size("128MiB"),
--
Vratislav Podzimek
Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
More information about the anaconda-patches
mailing list