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

Anne Mulhern amulhern at redhat.com
Mon Aug 18 13:37:11 UTC 2014





----- Original Message -----
> From: "Vratislav Podzimek" <vpodzime at redhat.com>
> To: "anaconda patch review" <anaconda-patches at lists.fedorahosted.org>
> Sent: Tuesday, August 12, 2014 3:07:53 AM
> Subject: Re: [blivet:master 7/9] Make has_redundancy a method rather than a	property.
> 
> 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
> 
> 
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> 

Done.

- mulhern


More information about the anaconda-patches mailing list