Skip to content

Commit

Permalink
devices: add detectpv option to lvm2.lv to auto-discover PV
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mvo5 committed Aug 21, 2024
1 parent 53761f9 commit 89a58b3
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 2 deletions.
36 changes: 34 additions & 2 deletions devices/org.osbuild.lvm2.lv
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
"""
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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}"
Expand Down
60 changes: 60 additions & 0 deletions devices/test/test_lv.py
Original file line number Diff line number Diff line change
@@ -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"

0 comments on commit 89a58b3

Please sign in to comment.