[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