From bd42df3ead360e5e74b4a16978b4be6d6034bb1a Mon Sep 17 00:00:00 2001 From: MacRimi Date: Fri, 30 Oct 2026 17:32:25 +0100 Subject: [PATCH] fix(monitor): keep the LVM filter when the scan names its devices --- AppImage/scripts/flask_server.py | 4 ++- AppImage/scripts/lvm_device_scope.py | 38 ++++++++++++++++++++++++++++ tests/test_lvm_disk_read_policy.py | 30 ++++++++++++++++++++-- tests/test_vm_disk_fsinfo.py | 36 ++++++++++++++++++++++++++ 4 files changed, 105 insertions(+), 3 deletions(-) create mode 100644 tests/test_vm_disk_fsinfo.py diff --git a/AppImage/scripts/flask_server.py b/AppImage/scripts/flask_server.py index 98d38f1c..c81246b1 100644 --- a/AppImage/scripts/flask_server.py +++ b/AppImage/scripts/flask_server.py @@ -2392,8 +2392,10 @@ def _compute_vm_disk_from_fsinfo(fsinfo): # real C:\ mount, and marking their signature eagerly would # suppress C:\ and yield None for the entire VM (bug reported # for a Windows 11 VM at 166 GB used / 1965 GB total). + # Some agents omit `dev`, so partitions of one disk share the + # disk fields; the filesystem name keeps /boot and / apart. d = disks[0] - sig = (d.get('bus'), d.get('target'), d.get('unit'), d.get('dev')) + sig = (d.get('bus'), d.get('target'), d.get('unit'), d.get('dev'), fs.get('name')) if sig in seen_disks: continue seen_disks.add(sig) diff --git a/AppImage/scripts/lvm_device_scope.py b/AppImage/scripts/lvm_device_scope.py index 43cf1896..c3c410bd 100644 --- a/AppImage/scripts/lvm_device_scope.py +++ b/AppImage/scripts/lvm_device_scope.py @@ -49,6 +49,40 @@ def _topology(): return types, parents, volumes +def _lvm_filters(): + """LVM's own global_filter and filter, as lists of (accept, pattern). + An explicit --devices list bypasses them, so a guest's volume group on + a zvol would otherwise appear on the host.""" + result = subprocess.run(["lvmconfig", "--type", "full", "devices/global_filter", "devices/filter"], + capture_output=True, text=True, timeout=5) + if result.returncode: + return None + filters = [] + for line in result.stdout.splitlines(): + name, _, value = line.strip().partition("=") + if name not in ("global_filter", "filter"): + continue + rules = [] + for item in re.findall(r'"((?:[^"\\]|\\.)*)"', value): + end = item.rfind(item[1]) if len(item) > 2 else -1 + if item[:1] not in ("a", "r") or end <= 1: + return None + rules.append((item[0] == "a", re.compile(item[2:end]))) + filters.append(rules) + return filters + + +def _accepted(path, filters): + """The first pattern a path matches decides; no match accepts it.""" + for rules in filters: + for accept, pattern in rules: + if pattern.search(path): + if not accept: + return False + break + return True + + def _disks_under(name, parents, seen=None): """The disks a device is built on: itself, or its parents' disks.""" seen = seen if seen is not None else set() @@ -92,9 +126,13 @@ def lvm_device_args(standby_check): _scanned_disks.update(disks - blocked) if not blocked: return [] + filters = _lvm_filters() + if filters is None: + return None # An explicit list makes LVM open every device in it, so only the # physical volumes are named: a RAID, multipath or encrypted volume # built on a blocked disk would read that disk too and stays out. + allowed = [path for path in allowed if _accepted(path, filters)] if not allowed: return None return ["--devices", ",".join(allowed)] diff --git a/tests/test_lvm_disk_read_policy.py b/tests/test_lvm_disk_read_policy.py index 26fd72df..c7502079 100644 --- a/tests/test_lvm_disk_read_policy.py +++ b/tests/test_lvm_disk_read_policy.py @@ -35,12 +35,38 @@ class LvmScopeTests(unittest.TestCase): self.modules.start() self.addCleanup(self.modules.stop) + lvmconfig = types.SimpleNamespace(returncode=0, stdout='global_filter=["r|/dev/zd.*|","r|/dev/rbd.*|"]\nfilter="a|.*|"\n') + def args(self, inventory=INVENTORY, code=0): - with patch.object(scope.subprocess, "run", return_value=types.SimpleNamespace(returncode=code, stdout=inventory)) as run: + def run(command, **_): + if command[0] == "lvmconfig": + return self.lvmconfig + return types.SimpleNamespace(returncode=code, stdout=inventory) + with patch.object(scope.subprocess, "run", side_effect=run) as mocked: args = scope.lvm_device_args(self.standby) - self.assertEqual(run.call_args.args[0], ["lsblk", "-J", "-l", "-o", "KNAME,TYPE,PKNAME,FSTYPE"]) + self.assertEqual(mocked.call_args_list[0].args[0], ["lsblk", "-J", "-l", "-o", "KNAME,TYPE,PKNAME,FSTYPE"]) return args + def test_volumes_lvm_filters_reject_are_not_named(self): + # A guest's volume group on a zvol is hidden by the Proxmox + # global_filter, which an explicit --devices list would bypass. + inventory = lsblk(("nvme0n1", "disk", None, None), ("nvme0n1p1", "part", "nvme0n1", "LVM2_member"), + ("sda", "disk", None, None), ("sda1", "part", "sda", "LVM2_member"), + ("zd48", "disk", None, None), ("zd48p3", "part", "zd48", "LVM2_member"), + ("rbd0", "disk", None, None), ("rbd0p1", "part", "rbd0", "LVM2_member")) + self.assertEqual(self.args(inventory), ["--devices", "/dev/nvme0n1p1"]) + self.excluded = {"sda", "nvme0n1"} + self.assertIsNone(self.args(inventory)) + + def test_an_unreadable_lvm_filter_skips_the_restricted_scan(self): + for output in (types.SimpleNamespace(returncode=5, stdout=""), + types.SimpleNamespace(returncode=0, stdout='global_filter=["x|/dev/zd.*|"]\n'), + types.SimpleNamespace(returncode=0, stdout='filter=["r|[|"]\n')): + self.lvmconfig = output + self.assertIsNone(self.args()) + self.excluded = set() + self.assertEqual(self.args(), []) + def test_excluded_disks_and_their_partitions_are_not_in_the_scan(self): self.assertEqual(self.args(), ["--devices", "/dev/nvme0n1p1,/dev/sdb1"]) self.assertNotIn((("sda",), {}), self.standby.call_args_list) diff --git a/tests/test_vm_disk_fsinfo.py b/tests/test_vm_disk_fsinfo.py new file mode 100644 index 00000000..ee17d83d --- /dev/null +++ b/tests/test_vm_disk_fsinfo.py @@ -0,0 +1,36 @@ +"""VM disk usage from the QEMU guest agent counts each filesystem once.""" +import ast +from pathlib import Path +import unittest + +SCRIPTS = Path(__file__).resolve().parents[1] / "AppImage/scripts" + + +def compute(): + tree = ast.parse((SCRIPTS / "flask_server.py").read_text()) + nodes = [node for node in tree.body + if (isinstance(node, ast.FunctionDef) and node.name == "_compute_vm_disk_from_fsinfo") + or (isinstance(node, ast.Assign) and any(getattr(t, "id", None) == "_VM_FS_SKIP_TYPES" for t in node.targets))] + namespace = {} + exec(compile(ast.Module(body=nodes, type_ignores=[]), "flask_server.py", "exec"), namespace) + return namespace["_compute_vm_disk_from_fsinfo"] + + +def fs(mountpoint, name, total, used, **disk): + return {"mountpoint": mountpoint, "name": name, "type": "ext4", "total-bytes": total, + "used-bytes": used, "disk": [{"bus": 0, "target": 0, "unit": 0, **disk}]} + + +class VmDiskFromFsinfo(unittest.TestCase): + def test_partitions_of_one_disk_without_dev_are_all_counted(self): + # Alpine's agent reports no `dev`: /boot and / share the disk fields. + result = compute()([fs("/boot", "sda1", 264371200, 29093888), fs("/", "sda3", 27736297472, 6119866368)]) + self.assertEqual(result, (29093888 + 6119866368, 264371200 + 27736297472)) + + def test_bind_mounts_of_one_filesystem_are_counted_once(self): + mounts = [fs(path, "sda8", 1000, 400, dev="/dev/sda8") for path in ("/mnt/data", "/etc/hosts", "/var/lib/docker")] + self.assertEqual(compute()(mounts), (400, 1000)) + + +if __name__ == "__main__": + unittest.main()