[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