[blivet:master 7/9] Make has_redundancy a method rather than a property.

Vratislav Podzimek vpodzime at redhat.com
Tue Aug 12 07:07:53 UTC 2014


On Mon, 2014-08-04 at 11:56 -0400, mulhern wrote:
> Originally, has_redundancy was set to False for Container so that it
> could be a simple property. But this feels like a cheap trick, since
> whether or not a Container level has intrinsic redundancy is not
> a reasonable question. Now, raise an exception for Container.has_redundancy.
> 
> Change calling code appropriately. making sure that the exception is not
> raised by that code and also that reasonably behavior occurs.
> ---
>  blivet/devicelibs/raid.py | 49 ++++++++++++++++++++++++++++++++++++-----------
>  blivet/devices.py         |  6 +++---
>  2 files changed, 41 insertions(+), 14 deletions(-)
> 
> diff --git a/blivet/devicelibs/raid.py b/blivet/devicelibs/raid.py
> index 50d19dc..d231c80 100644
> --- a/blivet/devicelibs/raid.py
> +++ b/blivet/devicelibs/raid.py
> @@ -48,8 +48,17 @@ class RAIDLevel(object):
>      names = abc.abstractproperty(doc="List of recognized names for this level.")
>      min_members = abc.abstractproperty(doc=
>         "The minimum number of members required to make a fully functioning array.")
> -    has_redundancy = abc.abstractproperty(doc=
> -       "Whether this RAID level incorporates inherent redundancy.")
> +    @abc.abstractmethod
> +    def has_redundancy(self):
> +        """ Whether this RAID level incorporates inherent redundancy.
> +
> +            Note that for some RAID levels, the notion of redundancy is
> +            meaningless.
> +
> +            :rtype: boolean
> +            :returns: True if this RAID level has inherent redundancy
> +        """
> +        raise NotImplementedError()
>  
>      is_uniform = abc.abstractproperty(doc=
>         "Whether data is uniformly distributed across all devices.")
> @@ -389,7 +398,9 @@ class RAID0(RAIDn):
>      level = property(lambda s: "0")
>      min_members = property(lambda s: 2)
>      nick = property(lambda s: "stripe")
> -    has_redundancy = property(lambda s: False)
> +
> +    def has_redundancy(self):
> +        return False
>  
>      def _get_max_spares(self, member_count):
>          return 0
> @@ -416,7 +427,9 @@ class RAID1(RAIDn):
>      level = property(lambda s: "1")
>      min_members = property(lambda s: 2)
>      nick = property(lambda s: "mirror")
> -    has_redundancy = property(lambda s: True)
> +
> +    def has_redundancy(self):
> +        return True
>  
>      def _get_max_spares(self, member_count):
>          return member_count - self.min_members
> @@ -443,7 +456,9 @@ class RAID4(RAIDn):
>      level = property(lambda s: "4")
>      min_members = property(lambda s: 3)
>      nick = property(lambda s: None)
> -    has_redundancy = property(lambda s: True)
> +
> +    def has_redundancy(self):
> +        return True
>  
>      def _get_max_spares(self, member_count):
>          return member_count - self.min_members
> @@ -470,7 +485,9 @@ class RAID5(RAIDn):
>      level = property(lambda s: "5")
>      min_members = property(lambda s: 3)
>      nick = property(lambda s: None)
> -    has_redundancy = property(lambda s: True)
> +
> +    def has_redundancy(self):
> +        return True
>  
>      def _get_max_spares(self, member_count):
>          return member_count - self.min_members
> @@ -497,7 +514,9 @@ class RAID6(RAIDn):
>      level = property(lambda s: "6")
>      min_members = property(lambda s: 4)
>      nick = property(lambda s: None)
> -    has_redundancy = property(lambda s: True)
> +
> +    def has_redundancy(self):
> +        return True
>  
>      def _get_max_spares(self, member_count):
>          return member_count - self.min_members
> @@ -524,7 +543,9 @@ class RAID10(RAIDn):
>      level = property(lambda s: "10")
>      min_members = property(lambda s: 4)
>      nick = property(lambda s: None)
> -    has_redundancy = property(lambda s: True)
> +
> +    def has_redundancy(self):
> +        return True
>  
>      def _get_max_spares(self, member_count):
>          return member_count - self.min_members
> @@ -551,9 +572,11 @@ class Container(RAIDLevel):
>      name = "container"
>      names = [name]
>      min_members = 1
> -    has_redundancy = property(lambda s: False)
>      is_uniform = property(lambda s: False)
>  
> +    def has_redundancy(self):
> +        raise NotImplementedError("redundancy is not a concept that applies to containers")
> +
>      def get_max_spares(self, member_count):
>          # pylint: disable=unused-argument
>          raise RaidError("get_max_spares is not defined for level container")
> @@ -579,9 +602,11 @@ class ErsatzRAID(RAIDLevel):
>          distinct subclasses which have different names.
>      """
>      min_members = 1
> -    has_redundancy = property(lambda s: False)
>      is_uniform = property(lambda s: False)
>  
> +    def has_redundancy(self):
> +        return False
> +
>      def get_max_spares(self, member_count):
>          return member_count - self.min_members
>  
> @@ -631,9 +656,11 @@ class Dup(RAIDLevel):
>      name = 'dup'
>      names = [name]
>      min_members = 1
> -    has_redundancy = property(lambda s: True)
>      is_uniform = property(lambda s: False)
>  
> +    def has_redundancy(self):
> +        return True
> +
>  Dup = Dup()
>  ALL_LEVELS.addRaidLevel(Dup)
>  
> diff --git a/blivet/devices.py b/blivet/devices.py
> index c238e94..c4259f5 100644
> --- a/blivet/devices.py
> +++ b/blivet/devices.py
> @@ -3679,7 +3679,7 @@ class MDRaidArrayDevice(ContainerDevice):
>  
>              If the array has no redundancy, a bitmap is just pointless.
>          """
> -        return self.level.has_redundancy and self.size >= 1000 and  self.format.type != "swap"
> +        return not self.level is raid.Container and self.level.has_redundancy() and self.size >= 1000 and  self.format.type != "swap"
It may be a "sci-fi category" concern, but I think we should call
self.level.has_redundancy() in a try-except block and then use the value
instead of prepending it with the 'self.level is raid.Container' check
and relying on lazy evaluation. In case there appears some other RAID
level with no redundancy concept, the line above will not work.

>  
>      def getSuperBlockSize(self, raw_array_size):
>          """Estimate the superblock size for a member of an array,
> @@ -3862,7 +3862,7 @@ class MDRaidArrayDevice(ContainerDevice):
>              appears to have formatting and therefore probably data on it,
>              removing one of its devices is a bad idea.
>          """
> -        if not self.level.has_redundancy and self.exists and member.format.exists:
> +        if not self.level is raid.Container and not self.level.has_redundancy() and self.exists and member.format.exists:
>              raise errors.DeviceError("cannot remove members from existing %s array" % self.level)
>  
>          super(MDRaidArrayDevice, self)._removeParent(member)
> @@ -4057,7 +4057,7 @@ class MDRaidArrayDevice(ContainerDevice):
>  
>      def _add(self, member):
>          self.setup()
> -        if self.level.has_redundancy:
> +        if self.level is raid.Container or self.level.has_redundancy():
The same applies here.

-- 
Vratislav Podzimek

Anaconda Rider | RHCE | Red Hat, Inc. | Brno - Czech Republic




More information about the anaconda-patches mailing list