New: [rhinstaller/blivet/pulls/182 master] device factory bug fixes

[dashea]

These were turned up in the course of writing unit tests for `DeviceFactory` (PR forthcoming).

[master 1/4] Run remove and append callbacks when replacing a parent.

[dashea]

From: David Lehman

---
 blivet/devices/lib.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/blivet/devices/lib.py b/blivet/devices/lib.py
index 5faada5..dbdcac7 100644
--- a/blivet/devices/lib.py
+++ b/blivet/devices/lib.py
@@ -177,4 +177,6 @@ def replace(self, x, y):
             raise ValueError("item to be replaced is not in the list")

idx = self.items.index(x)
+        self.removefunc(x)
+        self.appendfunc(y)
         self.items[idx] = y

[master 1/4] Run remove and append callbacks when replacing a parent.

[dashea]

This is either wrong or really confusing. The docstring of this method says: "*Replace the first instance of x with y, bypassing callbacks.*" and was intentionally not implemented as ``self.remove()`` followed by ``self.add()`` to ensure exactly that. So either the docstring and the implementation needs to change or any problematic call of this method should be replaced with ``remove(); add()``.

[master 1/4] Run remove and append callbacks when replacing a parent.

[dashea]

I didn't remember or notice the docstring. This will cause problems with raid as I have it. All we need is the `_addParent`/`_removeParent` methods from `Device` to run to do the basic tree accounting.

[master 2/4] Don't adjust actual lv size based on md metadata space requirements.

[dashea]

From: David Lehman

We adjusted the amount of disk space needed, but we didn't alter the
planned device size.
---
 blivet/devicefactory.py | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/blivet/devicefactory.py b/blivet/devicefactory.py
index 2096800..cd87194 100644
--- a/blivet/devicefactory.py
+++ b/blivet/devicefactory.py
@@ -1306,13 +1306,6 @@ def _get_child_factory_kwargs(self):

def _get_new_device(self, *args, **kwargs):
         """ Create and return the factory device as a StorageDevice. """
-
-        if self.container_raid_level and self.container_size in [SIZE_POLICY_AUTO,
-                                                                 SIZE_POLICY_MAX]:
-            # container pushed to the limit, but we need some extra space for
-            # metadata, so we need to make the LV smaller
-            extra_md_space = lvm.LVM_PE_SIZE * len(self.disks) * 5
-            kwargs["size"] -= extra_md_space
         return self.storage.newLV(*args, **kwargs)

def _set_name(self):

[master 2/4] Don't adjust actual lv size based on md metadata space requirements.

[dashea]

Shouldn't we alter the planned device size too instead? Or does this calculation happen in some other place? Or do we want to drop this extra space for metadata from our calculations? If yes, we need to remove it in a few more places.

[master 2/4] Don't adjust actual lv size based on md metadata space requirements.

[dashea]

There is a separate calculation for the required disk/pv space (`_get_total_space`), which includes extra space for metadata. Having created the needed space in the PVs, there is no need to adjust the device size at all.

[master 3/4] Fix container member switching when toggling member encryption.

[dashea]

From: David Lehman

We need the member we're removing to be a leaf device before we can
destroy it, so we have to remove it from the container first.
---
 blivet/devicefactory.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/blivet/devicefactory.py b/blivet/devicefactory.py
index cd87194..d522def 100644
--- a/blivet/devicefactory.py
+++ b/blivet/devicefactory.py
@@ -735,11 +735,13 @@ def _set_encryption(self):
             orig_device = self.device
             raw_device = self.raw_device
             leaf_format = self.device.format
+            if parent_container:
+                parent_container.parents.remove(orig_device)
             self.storage.destroyDevice(self.device)
             self.storage.formatDevice(self.raw_device, leaf_format)
             self.device = raw_device
             if parent_container:
-                parent_container.parents.replace(orig_device, self.device)
+                parent_container.parents.append(self.device)
         elif self.encrypted and not isinstance(self.device, LUKSDevice):
             orig_device = self.device
             leaf_format = self.device.format

[master 4/4] Set new raid level on correct device in MDFactory.

[dashea]

From: David Lehman

---
 blivet/devicefactory.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/blivet/devicefactory.py b/blivet/devicefactory.py
index d522def..2be78da 100644
--- a/blivet/devicefactory.py
+++ b/blivet/devicefactory.py
@@ -1630,7 +1630,7 @@ def _get_total_space(self):

def _set_raid_level(self):
         # set the new level
-        self.device.level = self.raid_level
+        self.raw_device.level = self.raid_level

# adjust the bitmap setting

[rhinstaller/blivet/pulls/182 master] device factory bug fixes

[dashea]

The rest looks good to me.