From 59ea39661ee17bc1e556bf04a3104c56fb73c0e6 Mon Sep 17 00:00:00 2001
From: Michael Vogt <michael.vogt@gmail.com>
Date: Mon, 26 Aug 2024 09:38:34 +0200
Subject: [PATCH] devices: extract `get_parent_path()`, use for
 manage_devices_file()

This commit extract a helper `get_parent_path()` that is unit
tested and also uses this generated parent_path for the call
to manage_devices_file to be consistent with the exiting behavior
of only including the device that actually contains the VG.
---
 devices/org.osbuild.lvm2.lv | 22 ++++++++++++++++------
 devices/test/test_lv.py     | 12 ++++++++++++
 2 files changed, 28 insertions(+), 6 deletions(-)
 create mode 100644 devices/test/test_lv.py

diff --git a/devices/org.osbuild.lvm2.lv b/devices/org.osbuild.lvm2.lv
index 8de7fc2b0..c8b01432e 100755
--- a/devices/org.osbuild.lvm2.lv
+++ b/devices/org.osbuild.lvm2.lv
@@ -53,6 +53,20 @@ SCHEMA = """
 """
 
 
+def get_parent_path(parent: str, options: Dict) -> str:
+    """ get_parent_path returns the full path to the block device for parent
+
+    Note that the options can influence the behavior via the vg partition
+    number option.
+    """
+    assert not parent.startswith("/")
+    parent_path = os.path.join("/dev", parent)
+    part = options.get("vg_partnum")
+    if part:
+        parent_path += f"p{part}"
+    return parent_path
+
+
 class LVService(devices.DeviceService):
 
     def __init__(self, args):
@@ -175,20 +189,16 @@ class LVService(devices.DeviceService):
 
         return major, minor
 
-    def open(self, devpath: str, parent: str, tree: str, options: Dict):
+    def open(self, devpath: str, parent: str, tree: str, options: Dict) -> Dict:
         lv = options["volume"]
 
-        assert not parent.startswith("/")
-        parent_path = os.path.join("/dev", parent)
+        parent_path = get_parent_path(parent, options)
 
         # Add the device to a lvm devices file on supported systems
         self.manage_devices_file(parent_path)
 
         # Find the volume group that belongs to the device specified
         # via `parent`
-        part = options.get("vg_partnum")
-        if part:
-            parent_path += f"p{part}"
         vg = self.volume_group_for_device(parent_path)
 
         # Activate the logical volume
diff --git a/devices/test/test_lv.py b/devices/test/test_lv.py
new file mode 100644
index 000000000..0e345d243
--- /dev/null
+++ b/devices/test/test_lv.py
@@ -0,0 +1,12 @@
+import pytest
+
+DEVICES_NAME = "org.osbuild.lvm2.lv"
+
+
+@pytest.mark.parametrize("parent,options,expected_parent_path", [
+    ("loop2", {}, "/dev/loop2"),
+    ("loop1", {"vg_partnum": 2}, "/dev/loop1p2"),
+])
+def test_lvm2_lv_get_parent_path(devices_module, parent, options, expected_parent_path):
+    pp = devices_module.get_parent_path(parent, options)
+    assert pp == expected_parent_path