[blivet:all] Take care when checking relationship of parent and child UUIDs (#1151649)

Anne Mulhern amulhern at redhat.com
Thu Oct 16 11:37:40 UTC 2014





----- Original Message -----
> From: "Vratislav Podzimek" <vpodzime at redhat.com>
> To: "anaconda patch review" <anaconda-patches at lists.fedorahosted.org>
> Sent: Thursday, October 16, 2014 3:19:51 AM
> Subject: Re: [blivet:all] Take care when checking relationship of parent and	child UUIDs (#1151649)
> 
> On Wed, 2014-10-15 at 15:37 -0400, mulhern wrote:
> > Related: rhbz#1151649
> > 
> > We are in the unfortunate position of not being able to assume that
> > the member's format has an attribute w/ the name specified by
> > self._formatUUIDAttr. This problem arises because MDRaidArrayDevices w/
> > type mdcontainer lack some of the properties of more typical
> > MDRaidArrayDevices.
> > 
> > Abstract all this checking into a single method, so that the problem
> > doesn't
> > pop up again later when removing or adding (for real, not just in the
> > in memory model).
> > 
> > Preserve all semantics, except for not raising an AttributeError if the
> > field does not exist.
> > 
> > Distinguish between situation where the data is untrustworthy or
> > unavailable and where it is considered reliable enough that verifying
> > its correctness makes sense.
> > 
> > Don't assume that the problem can only be due to an oddity of mdcontainer,
> > it may crop up elsewhere as well, so be robust.
> > 
> > Signed-off-by: mulhern <amulhern at redhat.com>
> > ---
> >  blivet/devices.py | 59
> >  +++++++++++++++++++++++++++++++++++++++++++++++--------
> >  1 file changed, 51 insertions(+), 8 deletions(-)
> > 
> > diff --git a/blivet/devices.py b/blivet/devices.py
> > index 1925970..f87eae4 100644
> > --- a/blivet/devices.py
> > +++ b/blivet/devices.py
> > @@ -2421,6 +2421,48 @@ class ContainerDevice(StorageDevice):
> >              return "Member format %(format)s is not a subtype of expected
> >              format %(expected)s." % {'format' : member.format, 'expected'
> >              : self.formatClass}
> >          return None
> >  
> > +    def _verifyMemberUuid(self, member, expect_equality=True,
> > require_existence=True):
> > +        """ Whether the member's array UUID has the proper relationship
> > +            with its array's UUID.
> > +
> > +            :param member: the member device to add
> > +            :type member: :class:`.StorageDevice`
> > +            :param bool expect_equality: if True, expect UUIDs to be
> > equal, otherwise, expect them to be unequal
> > +            :param bool require_existence: if True, checking UUIDs is only
> > meaningful if member format exists
> > +            :returns: error msg if the UUIDs lack the correct relationship
> > +            :rtype: str or NoneType
> > +        """
> > +        if not self._formatUUIDAttr:
> > +            log.info("No attribute name corresponding to member's array
> > UUID.")
> > +            return None
> > +
> > +        if not hasattr(member.format, self._formatUUIDAttr):
> > +            log.warning("Attribute name (%s) which specifies member
> > format's array UUID does not exist for this object (%s).",
> > self._formatUUIDAttr, member)
> > +            return None
> > +
> > +        member_fmt_uuid = getattr(member.format, self._formatUUIDAttr)
> > +
> > +        # If either UUID can not be obtained, nothing to check.
> > +        if not member_fmt_uuid or not self.uuid:
> > +            log.warning("At least one UUID missing.")
> > +            return None
> > +
> > +        # Below this line, the data obtained is considered to be correct.
> > +
> > +        # If existence is required and not present, nothing to check
> > +        if require_existence and not member.format.exists:
> > +            return None
> > +
> > +        uuids_equal = member_fmt_uuid == self.uuid
> > +
> > +        if expect_equality and not uuids_equal:
> > +            return "Member format's UUID %(uuid)s does not match expected
> > UUID %(expected)s." % {'uuid' : member_fmt_uuid, 'expected' : self.uuid}
> > +
> > +        if not expect_equality and uuids_equal:
> > +            return "Member format's UUID %(uuid)s matches expected UUID
> > %(expected)s." % {'uuid' : member_fmt_uuid, 'expected' : self.uuid}
> Should these two error messages be translated? If not, I think the
> %(name)s notation just makes the line too long without real value for
> the reader of the code as it is quite clear which item goes where in the
> string.
> 

Changed in working copy.

- mulhern

> --
> Vratislav Podzimek
> 
> Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
> 
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> 


More information about the anaconda-patches mailing list