From 628dbe3aa3eecabd42a080b8ec95c85e358af4db Mon Sep 17 00:00:00 2001 From: MacRimi <123239993+MacRimi@users.noreply.github.com> Date: Sat, 17 Oct 2026 00:03:39 +0200 Subject: [PATCH] Name only physical volumes in a restricted LVM scan and keep its own reads from counting as activity - An explicit --devices list makes LVM open every device in it, while a plain scan opens only known volumes; the list now names only LVM physical volumes, read from udev by lsblk, so no other disk is touched - RAID, multipath and encrypted volumes follow the disks they are built on, instead of turning the whole check off - After each pvs, lvs or vgs run, the disks it read take their counters as the new baseline, so an unused disk can become idle --- AppImage/scripts/disk_temperature_history.py | 10 +++ AppImage/scripts/flask_server.py | 7 +- AppImage/scripts/health_monitor.py | 3 + AppImage/scripts/lvm_device_scope.py | 91 ++++++++++++++++---- tests/test_lvm_disk_read_policy.py | 47 ++++++++-- 5 files changed, 131 insertions(+), 27 deletions(-) diff --git a/AppImage/scripts/disk_temperature_history.py b/AppImage/scripts/disk_temperature_history.py index baecafbd..2100c355 100644 --- a/AppImage/scripts/disk_temperature_history.py +++ b/AppImage/scripts/disk_temperature_history.py @@ -526,6 +526,16 @@ def disk_read_policy(disk_name: str, excluded: Optional[set] = None) -> str: return IDLE +def settle_own_reads(disk_names) -> None: + """A reader that has just read these disks itself takes their counters as + the new baseline, so its own I/O never counts as activity.""" + stats = _read_diskstats() + with _cache_lock: + for name in disk_names: + if name in _last_io and name in stats: + _last_io[name] = stats[name] + + def is_disk_idle(disk_name: str) -> bool: """True while the disk is being left alone for having no I/O.""" ts = _idle_state.get(disk_name) diff --git a/AppImage/scripts/flask_server.py b/AppImage/scripts/flask_server.py index 3e96e874..d8be04d6 100644 --- a/AppImage/scripts/flask_server.py +++ b/AppImage/scripts/flask_server.py @@ -4188,13 +4188,16 @@ def get_system_disks(): pass try: - # 4. Check LVM physical volumes. `--devices` (added when a disk - # is parked) keeps pvs from scanning — and waking — standby HDDs. + # 4. Check LVM physical volumes. `--devices` keeps pvs off the disks + # that are excluded, idle or asleep. dev_args = _lvm_device_args() result = subprocess.run( ['pvs', *dev_args, '--noheadings', '-o', 'pv_name,vg_name'], capture_output=True, text=True, timeout=5 ) if dev_args is not None else None + if result is not None: + from lvm_device_scope import settle_scan_reads + settle_scan_reads() if result is not None and result.returncode == 0: for line in result.stdout.strip().split('\n'): if line.strip(): diff --git a/AppImage/scripts/health_monitor.py b/AppImage/scripts/health_monitor.py index 81037aec..076a9cda 100644 --- a/AppImage/scripts/health_monitor.py +++ b/AppImage/scripts/health_monitor.py @@ -2286,6 +2286,8 @@ class HealthMonitor: text=True, timeout=3 ) + from lvm_device_scope import settle_scan_reads + settle_scan_reads() if result.returncode != 0: return {'status': 'WARNING', 'reason': 'lvs command failed'} @@ -2310,6 +2312,7 @@ class HealthMonitor: text=True, timeout=3 ) + settle_scan_reads() if vg_result.returncode == 0 and vg_result.stdout.strip(): return {'status': 'WARNING', 'reason': 'No active LVM volumes detected'} else: diff --git a/AppImage/scripts/lvm_device_scope.py b/AppImage/scripts/lvm_device_scope.py index 710957ae..43cf1896 100644 --- a/AppImage/scripts/lvm_device_scope.py +++ b/AppImage/scripts/lvm_device_scope.py @@ -11,11 +11,56 @@ # Description: # Applies the automatic disk-read policy to LVM metadata scans in both # Monitor readers. Excluded and idle disks receive no power-mode probe; -# their partitions are excluded too. No eligible devices means no scan. +# every device built on them is excluded too, and the scan's own reads +# are not taken as activity. No eligible devices means no scan. # ========================================================== +import json import re import subprocess +import threading + +_NAME = re.compile(r"[A-Za-z0-9._+-]+") +_scan_lock = threading.Lock() +_scanned_disks = set() + + +def _topology(): + """Every block device by kernel name: its type, its parents and whether + it is an LVM physical volume. lsblk reads this from udev, not the disks. + A RAID or multipath device appears once per member.""" + result = subprocess.run(["lsblk", "-J", "-l", "-o", "KNAME,TYPE,PKNAME,FSTYPE"], + capture_output=True, text=True, timeout=5) + if result.returncode: + return None + types, parents, volumes = {}, {}, set() + for device in json.loads(result.stdout).get("blockdevices") or []: + name, parent = device.get("kname"), device.get("pkname") + if not isinstance(name, str) or not _NAME.fullmatch(name): + return None + types[name] = device.get("type") + parents.setdefault(name, set()) + if parent is not None: + if not isinstance(parent, str) or not _NAME.fullmatch(parent): + return None + parents[name].add(parent) + if device.get("fstype") == "LVM2_member" and device.get("type") != "lvm": + volumes.add(name) + return types, parents, volumes + + +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() + if name in seen: + return set() + seen.add(name) + if not parents.get(name): + return {name} + found = set() + for parent in parents[name]: + found |= _disks_under(parent, parents, seen) + return found def lvm_device_args(standby_check): @@ -27,22 +72,12 @@ def lvm_device_args(standby_check): """ try: from disk_temperature_history import disk_read_policy, excluded_disk_names - result = subprocess.run(["lsblk", "-rno", "NAME,TYPE,PKNAME"], - capture_output=True, text=True, timeout=5) - if result.returncode: + topology = _topology() + if topology is None: return None - nodes = [] - for line in result.stdout.splitlines(): - parts = line.split() - if len(parts) < 2 or parts[1] not in ("disk", "part"): - continue - name = parts[0] - base = name if parts[1] == "disk" else parts[2] if len(parts) > 2 else "" - if not re.fullmatch(r"[A-Za-z0-9._+-]+", name) or not re.fullmatch(r"[A-Za-z0-9._+-]+", base): - return None - nodes.append((name, base)) - disks = {name for name, base in nodes if name == base} - if not disks or any(base not in disks for _, base in nodes): + types, parents, volumes = topology + disks = {name for name, kind in types.items() if kind == "disk"} + if not disks: return None excluded = excluded_disk_names() blocked = set() @@ -51,13 +86,31 @@ def lvm_device_args(standby_check): # excluded/idle disk. Some USB bridges do not honour -n standby. if disk_read_policy(disk, excluded) != "read" or standby_check(disk): blocked.add(disk) + allowed = sorted(f"/dev/{name}" for name in volumes if not (_disks_under(name, parents) & blocked)) + with _scan_lock: + _scanned_disks.clear() + _scanned_disks.update(disks - blocked) if not blocked: return [] - devices = [f"/dev/{name}" for name, base in nodes if base not in blocked] - if not devices: + # 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. + if not allowed: return None - return ["--devices", ",".join(dict.fromkeys(devices))] + return ["--devices", ",".join(allowed)] except Exception: # Unavailable exclusions, topology or policy are not permission to # open devices that may have been deliberately excluded. return None + + +def settle_scan_reads(): + """The LVM scan reads the disks it is allowed to. Those reads are its + own, so they must not make an unused disk look busy at the next check.""" + try: + from disk_temperature_history import settle_own_reads + with _scan_lock: + disks = set(_scanned_disks) + settle_own_reads(disks) + except Exception: + pass diff --git a/tests/test_lvm_disk_read_policy.py b/tests/test_lvm_disk_read_policy.py index 03e23c61..30495950 100644 --- a/tests/test_lvm_disk_read_policy.py +++ b/tests/test_lvm_disk_read_policy.py @@ -1,5 +1,6 @@ """Automatic LVM readers must not bypass disk exclusions/idle policy.""" import ast +import json from pathlib import Path import subprocess import sys @@ -11,7 +12,14 @@ SCRIPTS = Path(__file__).resolve().parents[1] / "AppImage/scripts" sys.path.insert(0, str(SCRIPTS)) import lvm_device_scope as scope -INVENTORY = "nvme0n1 disk\nnvme0n1p1 part nvme0n1\nsda disk\nsda1 part sda\nsdb disk\nsdb1 part sdb\n" +def lsblk(*devices): + """lsblk -J -l output from (kname, type, pkname, fstype) rows.""" + return json.dumps({"blockdevices": [dict(zip(("kname", "type", "pkname", "fstype"), row)) for row in devices]}) + + +INVENTORY = lsblk(("nvme0n1", "disk", None, None), ("nvme0n1p1", "part", "nvme0n1", "LVM2_member"), + ("sda", "disk", None, None), ("sda1", "part", "sda", "LVM2_member"), + ("sdb", "disk", None, None), ("sdb1", "part", "sdb", "LVM2_member")) class LvmScopeTests(unittest.TestCase): @@ -29,24 +37,24 @@ class LvmScopeTests(unittest.TestCase): def args(self, inventory=INVENTORY, code=0): with patch.object(scope.subprocess, "run", return_value=types.SimpleNamespace(returncode=code, stdout=inventory)) as run: args = scope.lvm_device_args(self.standby) - self.assertEqual(run.call_args.args[0], ["lsblk", "-rno", "NAME,TYPE,PKNAME"]) + self.assertEqual(run.call_args.args[0], ["lsblk", "-J", "-l", "-o", "KNAME,TYPE,PKNAME,FSTYPE"]) return args def test_excluded_disks_and_their_partitions_are_not_in_the_scan(self): - self.assertEqual(self.args(), ["--devices", "/dev/nvme0n1,/dev/nvme0n1p1,/dev/sdb,/dev/sdb1"]) + self.assertEqual(self.args(), ["--devices", "/dev/nvme0n1p1,/dev/sdb1"]) self.assertNotIn((("sda",), {}), self.standby.call_args_list) self.assertEqual({call.args[0] for call in self.standby.call_args_list}, {"nvme0n1", "sdb"}) def test_idle_disks_receive_no_power_query(self): self.excluded = set() self.policy.side_effect = lambda disk, excluded: "idle" if disk == "sda" else "read" - self.assertNotIn("/dev/sda", self.args()[1]) + self.assertNotIn("/dev/sda1", self.args()[1]) self.assertEqual({call.args[0] for call in self.standby.call_args_list}, {"nvme0n1", "sdb"}) def test_a_standby_disk_is_excluded_even_when_its_io_changed(self): self.excluded = set() self.standby.side_effect = lambda disk: disk == "sdb" - self.assertEqual(self.args(), ["--devices", "/dev/nvme0n1,/dev/nvme0n1p1,/dev/sda,/dev/sda1"]) + self.assertEqual(self.args(), ["--devices", "/dev/nvme0n1p1,/dev/sda1"]) def test_all_allowed_disks_keep_existing_scan_behaviour(self): self.excluded = set() @@ -63,7 +71,8 @@ class LvmScopeTests(unittest.TestCase): self.assertIsNone(self.args()) def test_bad_inventory_or_policy_never_authorizes_an_unrestricted_scan(self): - for inventory, code in (("", 0), (INVENTORY, 1), ("sda1 part sda", 0), ("../sda disk", 0)): + for inventory, code in (("", 0), (INVENTORY, 1), ("sda1 part sda", 0), + (lsblk(("sda1", "part", "sda", None)), 0), (lsblk(("../sda", "disk", None, None)), 0)): self.assertIsNone(self.args(inventory, code)) for error in (FileNotFoundError(), subprocess.TimeoutExpired("lsblk", 5)): with patch.object(scope.subprocess, "run", side_effect=error): @@ -72,6 +81,32 @@ class LvmScopeTests(unittest.TestCase): self.assertIsNone(self.args()) + def test_raid_multipath_and_encrypted_devices_follow_their_disks(self): + inventory = lsblk( + ("sda", "disk", None, None), ("sda1", "part", "sda", "linux_raid_member"), + ("sdb", "disk", None, None), ("sdb1", "part", "sdb", "linux_raid_member"), + ("md0", "raid1", "sda1", "LVM2_member"), ("md0", "raid1", "sdb1", "LVM2_member"), + ("sdc", "disk", None, None), ("sdc1", "part", "sdc", "crypto_LUKS"), + ("dm-0", "crypt", "sdc1", "LVM2_member"), ("dm-1", "lvm", "dm-0", "ext4"), + ("sdd", "disk", None, None), ("sde", "disk", None, None), + ("dm-2", "mpath", "sdd", None), ("dm-2", "mpath", "sde", None), ("dm-3", "part", "dm-2", "LVM2_member")) + self.excluded = {"sdb"} + # md0 reads sdb as well, so it stays out; the LUKS volume and the + # multipath partition are built only on readable disks. Disks that + # hold no physical volume are never named, so LVM does not open them. + self.assertEqual(self.args(inventory), ["--devices", "/dev/dm-0,/dev/dm-3"]) + self.excluded = set() + self.assertEqual(self.args(inventory), []) + + def test_the_scan_settles_its_own_reads_on_the_disks_it_read(self): + self.excluded = {"sda"} + settled = Mock() + self.disk_module.settle_own_reads = settled + self.args() + scope.settle_scan_reads() + settled.assert_called_once_with({"nvme0n1", "sdb"}) + + class LvmReaderTests(unittest.TestCase): def extract(self, file, name, owner=None, namespace=None): tree = ast.parse((SCRIPTS / file).read_text())