From 89a58b3844c86111d4bc083a8ee1d4f84c84c133 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Wed, 21 Aug 2024 16:56:13 +0200 Subject: [PATCH] devices: add detectpv option to lvm2.lv to auto-discover PV This is needed for bootc where all mounts need to be from the same physical disk/loop so that bootupd works. The idea is that in the manifest the new option `detectv` is added and the parent device is the full disk instead of the PV partition that is currently used and that results in multiple loop devices for each partition. So a manifest now looks like: ```json "devices": { "disk": { "type": "org.osbuild.loopback", "options": { "filename": "disk.raw", "partscan": true } }, "rootlv": { "type": "org.osbuild.lvm2.lv", "parent": "disk", "options": { "volume": "rootlv", "detectpv": true } } }, ``` instead of the alternative way via the partition (like this): ```json "devices": { "device": { "type": "org.osbuild.lvm2.lv", "parent": "rootvg", "options": { "volume": "rootlv" } }, "rootvg": { "type": "org.osbuild.loopback", "options": { "filename": "disk.raw", "start": 3127296, "size": 17844191, "lock": true } } } ``` The approach via `detectpv` means that it is impossible to put two identically named logical volume in two different volume groups on the same "physical" disk (loop-device). If this is a problem we need a different approach, we could create the `vg_name` in `org.osbuild.lvm2.create` in a predicatable but non-clashing way, e.g. it could be a `{filename}-{start}-{size}` and we would add the name to `org.osbuild.lvm2.lv` in options. This would still mean that we cannot do two LVM builds in parallel on the same host with identical `{filename}-{start}-{size}` so also not an ideal solution. Or we just put a random vg_name in the manifest but that also means no parallel image builds for the same manifest on the same host. --- devices/org.osbuild.lvm2.lv | 36 ++++++++++++++++++++-- devices/test/test_lv.py | 60 +++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 devices/test/test_lv.py diff --git a/devices/org.osbuild.lvm2.lv b/devices/org.osbuild.lvm2.lv index 2f8fe4f4f..641f70daf 100755 --- a/devices/org.osbuild.lvm2.lv +++ b/devices/org.osbuild.lvm2.lv @@ -44,6 +44,11 @@ SCHEMA = """ "volume": { "type": "string", "description": "Logical volume to active" + }, + "detectpv": { + "type": "boolean", + "default": false, + "description": "Detect the PV for the LV automatically on the given device" } } """ @@ -105,6 +110,21 @@ class LVService(devices.DeviceService): msg = f"Failed to set LV device ({fullname}) status: {data}" raise RuntimeError(msg) + def auto_detect_volume_group(self, device, lv_name: str) -> str: + lvs_output = subprocess.check_output(["lvs", "--reportformat=json"]) + for lv in json.loads(lvs_output)["report"][0]["lv"]: + if lv["lv_name"] == lv_name: + vg_name = lv["vg_name"] + # cross check that is comes from the expected device + pvs_output = subprocess.check_output(["pvs", "--reportformat=json"]) + for pv in json.loads(pvs_output)["report"][0]["pv"]: + if pv["vg_name"] == vg_name: + if not pv["pv_name"].startswith(device): + print(f"WARNING: ignoring {pv['pv_name']} because it is not on {device}", file=sys.stderr) + else: + return vg_name + raise RuntimeError(f"cannot find {lv_name} on {device}") + def volume_group_for_device(self, device: str) -> Union[int, str]: # Find the volume group that belongs to the device specified via `parent` vg_name = None @@ -126,7 +146,7 @@ class LVService(devices.DeviceService): if res.returncode == 5: if count == 10: - raise RuntimeError("Could not find parent device") + raise RuntimeError(f"Could not find parent device") time.sleep(1 * count) count += 1 continue @@ -173,6 +193,7 @@ class LVService(devices.DeviceService): def open(self, devpath: str, parent: str, tree: str, options: Dict): lv = options["volume"] + detectpv = options.get("detectpv", False) assert not parent.startswith("/") parent_path = os.path.join("/dev", parent) @@ -182,7 +203,18 @@ class LVService(devices.DeviceService): # Find the volume group that belongs to the device specified # via `parent` - vg = self.volume_group_for_device(parent_path) + # + # In theory "detectpv" could be the default - but that would + # change how old manifests work which is risky so only use it + # for on newer manifests for now + if detectpv: + # "new" way to detct vg_name: device can be the full disk/loop that + # contains the PV + vg = self.auto_detect_volume_group(parent_path, lv) + else: + # "old" style of detecting the vg_name, needs the pv partition + # in parent_path + vg = self.volume_group_for_device(parent_path) # Activate the logical volume self.fullname = f"{vg}/{lv}" diff --git a/devices/test/test_lv.py b/devices/test/test_lv.py new file mode 100644 index 000000000..a95c19576 --- /dev/null +++ b/devices/test/test_lv.py @@ -0,0 +1,60 @@ +#!/usr/bin/python3 + +import pytest + +DEVICES_NAME = "org.osbuild.lvm2.lv" + +mocked_lvs_output = """ +{ + "report": [ + { + "lv": [ + {"lv_name":"rootlv", "vg_name":"1d2b2150-8de2-4b68-b387-f9bc709190e8", "lv_attr":"-wi-------", "lv_size":"3.86g", "pool_lv":"", "origin":"", "data_percent":"", "metadata_percent":"", "move_pv":"", "mirror_log":"", "copy_percent":"", "convert_lv":""} + ] + } + ] +} +""" + +mocked_pvs_output = """ +{ + "report": [ + { + "pv": [ + {"pv_name":"/dev/loop1p4", "vg_name":"1d2b2150-8de2-4b68-b387-f9bc709190e8", "pv_fmt":"lvm2", "pv_attr":"a--", "pv_size":"8.50g", "pv_free":"4.64g"} + ] + } + ] + } + +""" + + +def mocked_check_output(args): + if args[0] == "lvs": + return mocked_lvs_output + if args[0] == "pvs": + return mocked_pvs_output + pytest.fail(f"unexpected arg {args}") + return "" + + +def test_lvm2_lv_auto_detect_volume_group_happy(monkeypatch, devices_service): + monkeypatch.setattr("subprocess.check_output", mocked_check_output) + vg_name = devices_service.auto_detect_volume_group("/dev/loop1", "rootlv") + assert vg_name == "1d2b2150-8de2-4b68-b387-f9bc709190e8" + + +def test_lvm2_lv_auto_detect_volume_group_lv_not_found(monkeypatch, devices_service): + monkeypatch.setattr("subprocess.check_output", mocked_check_output) + with pytest.raises(RuntimeError) as exc: + devices_service.auto_detect_volume_group("/dev/loop1", "other-lv") + assert str(exc.value) == "cannot find other-lv on /dev/loop1" + + +def test_lvm2_lv_auto_detect_volume_group_wrong_device(monkeypatch, capfd, devices_service): + monkeypatch.setattr("subprocess.check_output", mocked_check_output) + with pytest.raises(RuntimeError) as exc: + devices_service.auto_detect_volume_group("/dev/other/loop", "rootlv") + assert str(exc.value) == "cannot find rootlv on /dev/other/loop" + assert capfd.readouterr().err == "WARNING: ignoring /dev/loop1p4 because it is not on /dev/other/loop\n"