From f19140993e94be9e58c8a01c18f1907792f59927 Mon Sep 17 00:00:00 2001

From: Vojtech Trefny <vtrefny@redhat.com>

Date: Wed, 5 Aug 2020 13:44:38 +0200

Subject: [PATCH] Fix ignoring disk devices with parents or children

For disk-like devices like multipath we should allow to ignore

these by simply ignoring the mpath device or by ignoring all of its

drives.

- when ignoring the "mpatha" device we should also ignore "sda" and

"sdb"

- when ignoring both "sda" and "sdb" we should also ignore "mpatha"

- when ignoring only "sda" we should not ignore "mpatha" (we don't

want to deal with an "incomplete" multipath device in the tree)

This is consistent with the existing behaviour when using exclusive

disks (or "ignoredisks --only-use" in kickstart).

Resolves: rhbz#1866243

---

 blivet/devicetree.py     |  51 ++++++++-----

 tests/devicetree_test.py | 157 ++++++++++++++++++++++++++++-----------

 2 files changed, 146 insertions(+), 62 deletions(-)

diff --git a/blivet/devicetree.py b/blivet/devicetree.py

index 5cc360e1..2afb0d0e 100644

--- a/blivet/devicetree.py

+++ b/blivet/devicetree.py

@@ -907,31 +907,48 @@ class DeviceTreeBase(object):

                 hidden.add_hook(new=False)

                 lvm.lvm_cc_removeFilterRejectRegexp(hidden.name)

+    def _disk_in_taglist(self, disk, taglist):

+        # Taglist is a list containing mix of disk names and tags into which disk may belong.

+        # Check if it does. Raise ValueError if unknown tag is encountered.

+        if disk.name in taglist:

+            return True

+        tags = [t[1:] for t in taglist if t.startswith("@")]

+        for tag in tags:

+            if tag not in Tags.__members__:

+                raise ValueError("unknown ignoredisk tag '@%s' encountered" % tag)

+            if Tags(tag) in disk.tags:

+                return True

+        return False

+

     def _is_ignored_disk(self, disk):

         """ Checks config for lists of exclusive and ignored disks

             and returns if the given one should be ignored

         """

-

-        def disk_in_taglist(disk, taglist):

-            # Taglist is a list containing mix of disk names and tags into which disk may belong.

-            # Check if it does. Raise ValueError if unknown tag is encountered.

-            if disk.name in taglist:

-                return True

-            tags = [t[1:] for t in taglist if t.startswith("@")]

-            for tag in tags:

-                if tag not in Tags.__members__:

-                    raise ValueError("unknown ignoredisk tag '@%s' encountered" % tag)

-                if Tags(tag) in disk.tags:

-                    return True

-            return False

-

-        return ((self.ignored_disks and disk_in_taglist(disk, self.ignored_disks)) or

-                (self.exclusive_disks and not disk_in_taglist(disk, self.exclusive_disks)))

+        return ((self.ignored_disks and self._disk_in_taglist(disk, self.ignored_disks)) or

+                (self.exclusive_disks and not self._disk_in_taglist(disk, self.exclusive_disks)))

     def _hide_ignored_disks(self):

         # hide any subtrees that begin with an ignored disk

         for disk in [d for d in self._devices if d.is_disk]:

-            if self._is_ignored_disk(disk):

+            is_ignored = self.ignored_disks and self._disk_in_taglist(disk, self.ignored_disks)

+            is_exclusive = self.exclusive_disks and self._disk_in_taglist(disk, self.exclusive_disks)

+

+            if is_ignored:

+                if len(disk.children) == 1:

+                    if not all(self._is_ignored_disk(d) for d in disk.children[0].parents):

+                        raise DeviceTreeError("Including only a subset of raid/multipath member disks is not allowed.")

+

+                    # and also children like fwraid or mpath

+                    self.hide(disk.children[0])

+

+                # this disk is ignored: ignore it and all it's potential parents

+                for p in disk.parents:

+                    self.hide(p)

+

+                # and finally hide the disk itself

+                self.hide(disk)

+

+            if self.exclusive_disks and not is_exclusive:

                 ignored = True

                 # If the filter allows all members of a fwraid or mpath, the

                 # fwraid or mpath itself is implicitly allowed as well. I don't

diff --git a/tests/devicetree_test.py b/tests/devicetree_test.py

index a8f369cf..6032e7f6 100644

--- a/tests/devicetree_test.py

+++ b/tests/devicetree_test.py

@@ -370,51 +370,6 @@ class DeviceTreeTestCase(unittest.TestCase):

         self.assertTrue(sdb in tree.devices)

         self.assertTrue(sdc in tree.devices)

-        # now test exclusive_disks special cases for multipath

-        sda.format = get_format("multipath_member", exists=True)

-        sdb.format = get_format("multipath_member", exists=True)

-        sdc.format = get_format("multipath_member", exists=True)

-        mpatha = MultipathDevice("mpatha", parents=[sda, sdb, sdc])

-        tree._add_device(mpatha)

-

-        tree.ignored_disks = []

-        tree.exclusive_disks = ["mpatha"]

-

-        with patch.object(tree, "hide") as hide:

-            tree._hide_ignored_disks()

-            self.assertFalse(hide.called)

-

-        tree._hide_ignored_disks()

-        self.assertTrue(sda in tree.devices)

-        self.assertTrue(sdb in tree.devices)

-        self.assertTrue(sdc in tree.devices)

-        self.assertTrue(mpatha in tree.devices)

-

-        # all members in exclusive_disks implies the mpath in exclusive_disks

-        tree.exclusive_disks = ["sda", "sdb", "sdc"]

-        with patch.object(tree, "hide") as hide:

-            tree._hide_ignored_disks()

-            self.assertFalse(hide.called)

-

-        tree._hide_ignored_disks()

-        self.assertTrue(sda in tree.devices)

-        self.assertTrue(sdb in tree.devices)

-        self.assertTrue(sdc in tree.devices)

-        self.assertTrue(mpatha in tree.devices)

-

-        tree.exclusive_disks = ["sda", "sdb"]

-        with patch.object(tree, "hide") as hide:

-            tree._hide_ignored_disks()

-            hide.assert_any_call(mpatha)

-            hide.assert_any_call(sdc)

-

-        # verify that hide works as expected

-        tree._hide_ignored_disks()

-        self.assertTrue(sda in tree.devices)

-        self.assertTrue(sdb in tree.devices)

-        self.assertFalse(sdc in tree.devices)

-        self.assertFalse(mpatha in tree.devices)

-

     def test_get_related_disks(self):

         tree = DeviceTree()

@@ -447,3 +402,115 @@ class DeviceTreeTestCase(unittest.TestCase):

         tree.unhide(sda)

         self.assertEqual(tree.get_related_disks(sda), set([sda, sdb]))

         self.assertEqual(tree.get_related_disks(sdb), set([sda, sdb]))

+

+

+class DeviceTreeIgnoredExclusiveMultipathTestCase(unittest.TestCase):

+

+    def setUp(self):

+        self.tree = DeviceTree()

+

+        self.sda = DiskDevice("sda")

+        self.sdb = DiskDevice("sdb")

+        self.sdc = DiskDevice("sdc")

+

+        self.tree._add_device(self.sda)

+        self.tree._add_device(self.sdb)

+        self.tree._add_device(self.sdc)

+

+        self.assertTrue(self.sda in self.tree.devices)

+        self.assertTrue(self.sdb in self.tree.devices)

+        self.assertTrue(self.sdc in self.tree.devices)

+

+        # now test exclusive_disks special cases for multipath

+        self.sda.format = get_format("multipath_member", exists=True)

+        self.sdb.format = get_format("multipath_member", exists=True)

+        self.sdc.format = get_format("multipath_member", exists=True)

+        self.mpatha = MultipathDevice("mpatha", parents=[self.sda, self.sdb, self.sdc])

+        self.tree._add_device(self.mpatha)

+

+    def test_exclusive_disks_multipath_1(self):

+        # multipath is exclusive -> all disks should be exclusive

+        self.tree.ignored_disks = []

+        self.tree.exclusive_disks = ["mpatha"]

+

+        with patch.object(self.tree, "hide") as hide:

+            self.tree._hide_ignored_disks()

+            self.assertFalse(hide.called)

+

+        self.tree._hide_ignored_disks()

+        self.assertTrue(self.sda in self.tree.devices)

+        self.assertTrue(self.sdb in self.tree.devices)

+        self.assertTrue(self.sdc in self.tree.devices)

+        self.assertTrue(self.mpatha in self.tree.devices)

+

+    def test_exclusive_disks_multipath_2(self):

+        # all disks exclusive -> mpath should also be exclusive

+        self.tree.exclusive_disks = ["sda", "sdb", "sdc"]

+        with patch.object(self.tree, "hide") as hide:

+            self.tree._hide_ignored_disks()

+            self.assertFalse(hide.called)

+

+        self.tree._hide_ignored_disks()

+        self.assertTrue(self.sda in self.tree.devices)

+        self.assertTrue(self.sdb in self.tree.devices)

+        self.assertTrue(self.sdc in self.tree.devices)

+        self.assertTrue(self.mpatha in self.tree.devices)

+

+    def test_exclusive_disks_multipath_3(self):

+        # some disks exclusive -> mpath should be hidden

+        self.tree.exclusive_disks = ["sda", "sdb"]

+        with patch.object(self.tree, "hide") as hide:

+            self.tree._hide_ignored_disks()

+            hide.assert_any_call(self.mpatha)

+            hide.assert_any_call(self.sdc)

+

+        # verify that hide works as expected

+        self.tree._hide_ignored_disks()

+        self.assertTrue(self.sda in self.tree.devices)

+        self.assertTrue(self.sdb in self.tree.devices)

+        self.assertFalse(self.sdc in self.tree.devices)

+        self.assertFalse(self.mpatha in self.tree.devices)

+

+    def test_ignored_disks_multipath_1(self):

+        # mpatha ignored -> disks should be hidden

+        self.tree.ignored_disks = ["mpatha"]

+        self.tree.exclusive_disks = []

+

+        with patch.object(self.tree, "hide") as hide:

+            self.tree._hide_ignored_disks()

+            hide.assert_any_call(self.mpatha)

+            hide.assert_any_call(self.sda)

+            hide.assert_any_call(self.sdb)

+            hide.assert_any_call(self.sdc)

+

+        self.tree._hide_ignored_disks()

+        self.assertFalse(self.sda in self.tree.devices)

+        self.assertFalse(self.sdb in self.tree.devices)

+        self.assertFalse(self.sdc in self.tree.devices)

+        self.assertFalse(self.mpatha in self.tree.devices)

+

+    def test_ignored_disks_multipath_2(self):

+        # all disks ignored -> mpath should be hidden

+        self.tree.ignored_disks = ["sda", "sdb", "sdc"]

+        self.tree.exclusive_disks = []

+

+        with patch.object(self.tree, "hide") as hide:

+            self.tree._hide_ignored_disks()

+            hide.assert_any_call(self.mpatha)

+            hide.assert_any_call(self.sda)

+            hide.assert_any_call(self.sdb)

+            hide.assert_any_call(self.sdc)

+

+        self.tree._hide_ignored_disks()

+        self.assertFalse(self.sda in self.tree.devices)

+        self.assertFalse(self.sdb in self.tree.devices)

+        self.assertFalse(self.sdc in self.tree.devices)

+        self.assertFalse(self.mpatha in self.tree.devices)

+

+    def test_ignored_disks_multipath_3(self):

+        # some disks ignored -> error

+        self.tree.ignored_disks = ["sda", "sdb"]

+        self.tree.exclusive_disks = []

+

+        with self.assertRaises(DeviceTreeError):

+            self.tree._hide_ignored_disks()

-- 

2.25.4