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

mulhern amulhern at redhat.com
Wed Oct 15 19:37:15 UTC 2014


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}
+
+        return None
+
     def _addParent(self, member):
         """ Add a member device to the container.
 
@@ -2436,8 +2478,8 @@ class ContainerDevice(StorageDevice):
         if error:
             raise ValueError(error)
 
-        if member.format.exists and self.uuid and self._formatUUIDAttr and \
-           getattr(member.format, self._formatUUIDAttr) != self.uuid:
+        error = self._verifyMemberUuid(member)
+        if error:
             raise ValueError("cannot add member with mismatched UUID")
 
         super(ContainerDevice, self)._addParent(member)
@@ -2464,9 +2506,9 @@ class ContainerDevice(StorageDevice):
         if not self.exists:
             raise errors.DeviceError("device has not been created", self.name)
 
-        if member.format.exists and self.uuid and self._formatUUIDAttr and \
-           getattr(member.format, self._formatUUIDAttr) == self.uuid:
-            log.error("cannot re-add member: %s", member)
+        error = self._verifyMemberUuid(member, expect_equality=False)
+        if error:
+            log.error("cannot re-add member: %s (%s)", member, error)
             raise ValueError("cannot add members that are already part of the container")
 
         self._add(member)
@@ -2497,9 +2539,10 @@ class ContainerDevice(StorageDevice):
         if not self.exists:
             raise errors.DeviceError("device has not been created", self.name)
 
-        if self._formatUUIDAttr and self.uuid and \
-           getattr(member.format, self._formatUUIDAttr) != self.uuid:
-            log.error("cannot remove non-member: %s (%s/%s)", member, getattr(member.format, self._formatUUIDAttr), self.uuid)
+
+        error = self._verifyMemberUuid(member, require_existence=False)
+        if error:
+            log.error("cannot remove non-member: %s (%s)", member, error)
             raise ValueError("cannot remove members that are not part of the container")
 
         self._remove(member)
-- 
1.9.3



More information about the anaconda-patches mailing list