Run fstrim in a VM via the guest agent

f on the info screen runs fstrim in a running VM with the guest agent
enabled. A container gets an info note (no guest agent; use pct
fstrim on the node). A stopped VM or one without an agent gets a
warning, and nothing runs. Otherwise a confirm comes first, then
agent ping and fstrim. The result reports the trimmed total
(GiB/MiB), the number of paths and any per-path errors.

Review fixes:
- Odd agent output no longer turns a finished trim into "fstrim
  failed". Unreadable output gives "no results". Odd entries are
  repaired: non-dict entries are dropped, a non-int trimmed becomes 0
  and a missing path becomes "?". Well-formed entries are returned
  unchanged.
- fstrim gets a 30-minute CLI timeout instead of 330 s, because a big
  disk can take longer.
- Notifications are never parsed as markup (GuestApp.notify defaults
  to markup=False). Guest names, paths and CLI or agent errors are
  untrusted text, so a "[/b]" in them could break the toast.
- f before the info screen has loaded says "still loading" instead of
  a wrong "not running".
- "1 path" in the singular.

Not run live: the issue asks for approval before fstrim on VM 108.
Only the harmless agent ping was run there, and it answered.

Closes #19

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

README.md

@@ -46,6 +46,7 @@ uv run cv4pve-tui
 | `Enter` then `n` | in info: edit network (bridge, VLAN, firewall, IPv4/IPv6; VM IP via cloud-init at next boot); `u` there = undo the last network change of this guest (this session only) |
 | `Enter` then `o` | in info: edit autostart, startup order (order, up and down delay) and tags (one confirmed `set config` call; empty fields remove the setting) |
 | `Enter` then `p` / `R` / `D` | in info: take a snapshot (name, description, VM: include RAM) / roll back to the snapshot under the cursor (confirms; starts the guest again if it is running) / delete the snapshot under the cursor (confirms) |
+| `Enter` then `f` | in info: fstrim in a running VM with the guest agent enabled (confirms; containers: use `pct fstrim` on the node) |
 | `t` | shell in a running LXC container (`ssh -t root@<node> pct enter <vmid>`) |
 | `l` | tasks: recent cluster tasks (start, duration, node, type, VMID, user, status; `!` marks failures); `Enter` shows the log of the task under the cursor, `g` reloads. In info: `l` = only this guest's tasks |
 | `U` | package updates: pending updates per online node (node, package, current, new, origin, priority), read-only; `g` reloads. The list is as of the node's last `apt update`. The frame's bottom border shows the count |

docs/plans/guest-tui-mvp.md

@@ -697,3 +697,41 @@ CLI facts (read-only probes, PVE 9.2):
   or None, `tags` string or None); invalid -> error notify, dialog stays, no call; no
   change -> info notify; else `ConfirmScreen` titled "Boot and tags" listing old -> new
   per key; yes -> busy guard, `cli.set_net(guest, changes)` via start_job, info reloads.
+
+---
+
+# fstrim in a VM via the guest agent (issue #19)
+
+## Decision
+
+Key `f` on the info screen, VMs only. Not run live (the issue asks for approval first);
+tested with mocks.
+
+CLI facts:
+
+- `do vm agent ping <node> <vmid> -o json` -> `[{"key": "result", "value": "{}"}]` when the
+  agent answers (verified read-only on VM 108); non-zero exit when it does not.
+- `do vm agent fstrim <node> <vmid> -o json` -> the same key/value shape; QEMU guest
+  agent's result is `{"paths": [{"path": "/", "trimmed": <bytes>, "minimum": 0}, ...]}`
+  (a path may carry `"error"` instead of `trimmed`). Not verified live.
+- VM config `agent` = `enabled=1` (or `1`) when the agent is enabled.
+
+### `cli.py`
+
+- `agent_enabled(config) -> bool` - `agent` is `1` or starts with `1` / has `enabled=1`.
+- `agent_ping(guest)` -> `do vm agent ping <node> <vmid> -o json`; CliError if the agent
+  does not answer.
+- `fstrim(guest) -> list[dict]` -> `do vm agent fstrim <node> <vmid> -o json`; returns
+  the `paths` list parsed from the `result` value (`[]` if missing or unparseable).
+
+### `app.py` (UI contract)
+
+- `f` on the info screen:
+  - container -> info notify that containers have no guest agent (`pct fstrim` on the
+    node instead); nothing runs;
+  - VM not running, or `agent_enabled` false -> warning notify saying why; nothing runs;
+  - otherwise `ConfirmScreen` titled "fstrim" naming the VM and saying it frees unused
+    blocks on thin-provisioned storage and adds I/O load while it runs;
+  - yes -> busy guard, worker: `cli.agent_ping` then `cli.fstrim`; done notify with the
+    trimmed total in GiB/MiB and the number of paths (errors per path mentioned);
+    CliError -> error notify (ping failure says the agent did not answer).

src/cv4pve_tui/app.py

@@ -718,6 +718,7 @@ class InfoScreen(ModalScreen):
         ("R", "rollback", "Roll back"),
         ("D", "delete_snapshot", "Delete snapshot"),
         ("l", "tasks", "Tasks"),
+        ("f", "fstrim", "fstrim"),
     ]
     DEFAULT_CSS = """
     InfoScreen DataTable { height: auto; max-height: 12; }
@@ -1064,6 +1065,64 @@ class InfoScreen(ModalScreen):
             ),
         )
 
+    def action_fstrim(self) -> None:
+        who = f"{self.guest['name']} ({self.guest['vmid']})"
+        if not self.config:  # running/agent are only known once the screen has loaded
+            self.app.notify("Info is still loading; try again in a moment")
+            return
+        if cli.kind(self.guest) == "ct":
+            self.app.notify(
+                "Containers have no guest agent; run pct fstrim on the node instead"
+            )
+            return
+        if not self.running:
+            self.app.notify(
+                f"{who} is not running; fstrim needs a running VM", severity="warning"
+            )
+            return
+        if not cli.agent_enabled(self.config):
+            self.app.notify(
+                f"The guest agent is not enabled for {who}", severity="warning"
+            )
+            return
+
+        def job() -> None:
+            try:
+                cli.agent_ping(self.guest)
+            except cli.CliError as exc:
+                raise cli.CliError(
+                    f"the guest agent did not answer: {_message(exc)}"
+                ) from exc
+            paths = cli.fstrim(self.guest)
+            if not paths:
+                self.app.call_from_thread(
+                    self.app.notify,
+                    f"fstrim finished in {who}; the agent reported no results",
+                )
+                return
+            total = sum(p.get("trimmed") or 0 for p in paths)
+            size = (
+                f"{total / 2**30:.1f} GiB"
+                if total >= 2**30
+                else f"{total / 2**20:.1f} MiB"
+            )
+            errs = "".join(
+                f"; {p.get('path', '?')}: {p['error']}" for p in paths if p.get("error")
+            )
+            self.app.call_from_thread(
+                self.app.notify,
+                f"Trimmed {size} in {len(paths)} path{'s' if len(paths) != 1 else ''}{errs}",
+            )
+
+        self.app.push_screen(
+            ConfirmScreen(
+                f"Run fstrim in {who}? It frees unused blocks on thin-provisioned "
+                "storage and adds I/O load while it runs.",
+                "fstrim",
+            ),
+            lambda ok: self.app.start_job(self.guest, "fstrim", job) if ok else None,
+        )
+
 
 def _when(epoch: int | None) -> str:
     return datetime.fromtimestamp(epoch).strftime("%Y-%m-%d %H:%M") if epoch else "-"
@@ -1163,6 +1222,11 @@ class GuestApp(App):
     #filter { display: none; }
     """
 
+    def notify(self, message: str, **kwargs) -> None:
+        # messages carry guest names, paths and CLI/agent errors: never parse them as markup
+        kwargs.setdefault("markup", False)
+        super().notify(message, **kwargs)
+
     def compose(self) -> ComposeResult:
         yield Header()
         with Vertical(id="guests_section") as section:

src/cv4pve_tui/cli.py

@@ -28,7 +28,7 @@ class CliError(Exception):
         return str(self)
 
 
-def run(*args: str) -> str:
+def run(*args: str, timeout: int = 330) -> str:
     try:
         proc = subprocess.run(
             ["cv4pve-cli", *args],
@@ -36,7 +36,7 @@ def run(*args: str) -> str:
             capture_output=True,
             text=True,
             stdin=subprocess.DEVNULL,
-            timeout=330,
+            timeout=timeout,
         )
     except (OSError, subprocess.TimeoutExpired) as exc:
         raise CliError(str(exc)) from exc
@@ -236,6 +236,58 @@ def backup_now(guest: dict, storage: str) -> None:
     )  # fmt: skip
 
 
+def agent_enabled(config: dict) -> bool:
+    """VM option `agent`: `1`, `1,<opt>=...` or `enabled=1,...`."""
+    parts = str(config.get("agent") or "").split(",")
+    return parts[0] == "1" or "enabled=1" in parts
+
+
+def agent_ping(guest: dict) -> None:
+    """CliError if the guest agent does not answer."""
+    run("do", "vm", "agent", "ping", guest["node"], str(guest["vmid"]), "-o", "json")
+
+
+def fstrim(guest: dict) -> list[dict]:
+    """Trim the guest's filesystems; the per-path results, `[]` if they cannot be read.
+
+    Well-formed entries come back as the agent sent them. Odd ones are repaired, not
+    fatal - the trim has already happened when the output is parsed: non-dict entries
+    are dropped, a non-int `trimmed` becomes 0 and a missing `path` becomes "?".
+    """
+    args = (
+        "do",
+        "vm",
+        "agent",
+        "fstrim",
+        guest["node"],
+        str(guest["vmid"]),
+        "-o",
+        "json",
+    )
+    out = run(
+        *args, timeout=1800
+    )  # a large disk can take longer than the default 330 s
+    try:
+        items = json.loads(out)
+        value = {x["key"]: x["value"] for x in items}.get("result") or "{}"
+        result = value if isinstance(value, dict) else json.loads(value)
+        paths = result.get("paths") if isinstance(result, dict) else None
+    except (ValueError, TypeError, KeyError, AttributeError):
+        return []
+    found = []
+    for p in paths if isinstance(paths, list) else []:
+        if not isinstance(p, dict):
+            continue
+        p = dict(p)
+        if "trimmed" in p and (
+            not isinstance(p["trimmed"], int) or isinstance(p["trimmed"], bool)
+        ):
+            p["trimmed"] = 0
+        p.setdefault("path", "?")
+        found.append(p)
+    return found
+
+
 def parse_net(value: str) -> dict:
     """`k=v,k=v` to an ordered dict; a VM's leading `<model>=<MAC>` is just the first pair."""
     return dict(item.partition("=")[::2] for item in value.split(",") if item)

tests/test_app.py

@@ -4,6 +4,7 @@ All cli functions are monkeypatched: cv4pve-cli is never executed.
 """
 
 import contextlib
+import re
 import subprocess
 import threading
 from datetime import datetime
@@ -89,6 +90,15 @@ class Fake:
         self.nodes_error, self.updates_errors = None, {}  # node -> exception
         self.nodes_calls, self.updates_calls = 0, []
         self.updates_hook = None
+        # issue #19 (fstrim via the guest agent)
+        self.events = []  # "ping"/"fstrim" in call order
+        self.ping_error, self.fstrim_error, self.fstrim_hook, self.ping_hook = (
+            None,
+            None,
+            None,
+            None,
+        )
+        self.fstrim_paths = [{"path": "/", "trimmed": 2**30, "minimum": 0}]
 
     def list_guests(self):
         self.list_calls += 1
@@ -259,6 +269,21 @@ class Fake:
             raise self.updates_errors[node]
         return rows
 
+    def agent_ping(self, guest):
+        self.events.append(("ping", guest["vmid"]))
+        if self.ping_hook:
+            self.ping_hook()
+        if self.ping_error:
+            raise self.ping_error
+
+    def fstrim(self, guest):
+        self.events.append(("fstrim", guest["vmid"]))
+        if self.fstrim_hook:
+            self.fstrim_hook()
+        if self.fstrim_error:
+            raise self.fstrim_error
+        return [dict(p) for p in self.fstrim_paths]
+
     def task_log(self, upid):
         self.log_calls.append(upid)
         if self.log_error:
@@ -324,6 +349,8 @@ def fake(monkeypatch):
         "task_log",
         "nodes",
         "package_updates",
+        "agent_ping",
+        "fstrim",
     ):
         monkeypatch.setattr(cli, name, getattr(f, name), raising=False)
     for name in ("list_guests", "guest_config", "disks", "action", "set_resources",
@@ -4195,3 +4222,242 @@ async def test_boot_set_net_failure_notifies_error_and_releases_busy(fake):
         assert any("unable to apply onboot" in m for m in errors(app))
         assert app.busy == set() and app.is_running
         assert len(fake.set_calls) == 1
+
+
+# --- issue #19: fstrim via the guest agent (spec: "fstrim in a VM via the guest agent") -----
+
+
+def agent_vm(fake, config="enabled=1", status="running"):
+    fake.guests = [dict(VM, status=status)]
+    fake.live_status = {108: status}
+    if config is not None:
+        fake.net_configs[108] = {"agent": config}
+
+
+def notes(app):
+    return " | ".join(m for m, _ in app.seen).lower()
+
+
+async def press_f(app, pilot):
+    await pilot.press("f")
+    await settle(app, pilot)
+
+
+@pytest.mark.parametrize("live", [None, "running"])
+async def test_fstrim_on_container_notifies_pct_fstrim_and_runs_nothing(fake, live):
+    # spec: container -> info notify (pct fstrim on the node instead); nothing runs
+    if live:
+        fake.live_status = {100: live}
+    app = make_app()
+    async with app.run_test(size=SIZE) as pilot:
+        await open_info(app, pilot, down=1)
+        await press_f(app, pilot)
+        assert any("pct fstrim" in m and sev == "information" for m, sev in app.seen)
+        assert fake.events == []
+        assert isinstance(app.screen, appmod.InfoScreen)
+
+
+async def test_fstrim_on_stopped_vm_warns_and_runs_nothing(fake):
+    # spec: VM not running -> warning notify saying why; nothing runs
+    agent_vm(fake, status="stopped")
+    app = make_app()
+    async with app.run_test(size=SIZE) as pilot:
+        await open_info(app, pilot)
+        await press_f(app, pilot)
+        assert any("running" in m.lower() and sev == "warning" for m, sev in app.seen)
+        assert fake.events == []
+        assert isinstance(app.screen, appmod.InfoScreen)
+
+
+@pytest.mark.parametrize("config", [None, "enabled=0", "0"])
+async def test_fstrim_on_vm_without_agent_warns_and_runs_nothing(fake, config):
+    # spec: agent_enabled false -> warning notify saying why; nothing runs
+    agent_vm(fake, config=config)
+    app = make_app()
+    async with app.run_test(size=SIZE) as pilot:
+        await open_info(app, pilot)
+        await press_f(app, pilot)
+        assert any("agent" in m.lower() and sev == "warning" for m, sev in app.seen)
+        assert fake.events == []
+        assert isinstance(app.screen, appmod.InfoScreen)
+
+
+async def test_fstrim_asks_confirm_titled_fstrim_naming_vm_before_any_call(fake):
+    # spec: ConfirmScreen titled "fstrim" naming the VM; nothing runs before yes
+    agent_vm(fake)
+    app = make_app()
+    async with app.run_test(size=SIZE) as pilot:
+        await open_info(app, pilot)
+        await press_f(app, pilot)
+        assert isinstance(app.screen, ConfirmScreen)
+        assert app.screen.title_text == "fstrim"
+        assert "docker" in text_of(app.screen)
+        assert fake.events == []
+
+
+async def test_fstrim_yes_pings_then_trims_once_busy_then_done_with_size_and_count(
+    fake,
+):
+    # spec: yes -> busy guard, agent_ping then fstrim; done notify with GiB total and path count
+    agent_vm(fake)
+    fake.fstrim_paths = [
+        {"path": "/", "trimmed": 2**30, "minimum": 0},
+        {"path": "/boot", "error": "not supported"},
+    ]
+    busy_seen = []
+    fake.fstrim_hook = lambda: busy_seen.append(set(app.busy))
+    app = make_app()
+    async with app.run_test(size=SIZE) as pilot:
+        await open_info(app, pilot)
+        await press_f(app, pilot)
+        await press_button(app, pilot, "#yes")
+        assert fake.events == [("ping", 108), ("fstrim", 108)]
+        assert busy_seen and busy_seen[0], "guest must be busy while fstrim runs"
+        assert app.busy == set()
+        done = [m for m, sev in app.seen if "1.0 GiB" in m]
+        assert done, app.seen
+        assert any(re.search(r"\b2\b", m) for m in done), "path count missing"
+        assert "not supported" in notes(app), "per-path error must be mentioned"
+        assert errors(app) == []
+
+
+async def test_fstrim_small_total_is_shown_in_mib(fake):
+    # spec: trimmed total in GiB/MiB
+    agent_vm(fake)
+    fake.fstrim_paths = [{"path": "/", "trimmed": 5 * 2**20, "minimum": 0}]
+    app = make_app()
+    async with app.run_test(size=SIZE) as pilot:
+        await open_info(app, pilot)
+        await press_f(app, pilot)
+        await press_button(app, pilot, "#yes")
+        assert "mib" in notes(app) and "gib" not in notes(app)
+        assert errors(app) == []
+
+
+@pytest.mark.parametrize("how", ["no", "escape"])
+async def test_fstrim_declined_makes_no_call(fake, how):
+    # spec: ConfirmScreen; declining runs nothing
+    agent_vm(fake)
+    app = make_app()
+    async with app.run_test(size=SIZE) as pilot:
+        await open_info(app, pilot)
+        await press_f(app, pilot)
+        assert isinstance(app.screen, ConfirmScreen)
+        if how == "no":
+            await press_button(app, pilot, "#no")
+        else:
+            await pilot.press("escape")
+            await settle(app, pilot)
+        assert fake.events == []
+        assert not isinstance(app.screen, ConfirmScreen)
+        assert app.busy == set()
+
+
+async def test_fstrim_ping_failure_errors_agent_did_not_answer_and_skips_fstrim(fake):
+    # spec: CliError -> error notify (ping failure says the agent did not answer)
+    agent_vm(fake)
+    fake.ping_error = cli_error("Error: VM 108 qmp command failed - timeout")
+    app = make_app()
+    async with app.run_test(size=SIZE) as pilot:
+        await open_info(app, pilot)
+        await press_f(app, pilot)
+        await press_button(app, pilot, "#yes")
+        assert fake.events == [("ping", 108)]
+        assert any("agent" in m.lower() and "answer" in m.lower() for m in errors(app))
+        assert app.busy == set() and app.is_running
+
+
+async def test_fstrim_failure_notifies_error_and_releases_busy(fake):
+    # spec: CliError -> error notify
+    agent_vm(fake)
+    fake.fstrim_error = cli_error("Error: fstrim command failed")
+    app = make_app()
+    async with app.run_test(size=SIZE) as pilot:
+        await open_info(app, pilot)
+        await press_f(app, pilot)
+        await press_button(app, pilot, "#yes")
+        assert fake.events == [("ping", 108), ("fstrim", 108)]
+        assert any("fstrim command failed" in m for m in errors(app))
+        assert app.busy == set() and app.is_running
+        assert not any("GiB" in m or "MiB" in m for m, _ in app.seen)
+
+
+async def test_fstrim_while_guest_busy_warns_and_makes_no_second_call(fake):
+    # spec: busy guard
+    agent_vm(fake)
+    started, release = threading.Event(), threading.Event()
+    fake.ping_hook = lambda: (started.set(), release.wait(10))
+    app = make_app()
+    async with app.run_test(size=SIZE) as pilot:
+        await open_info(app, pilot)
+        try:
+            await press_f(app, pilot)
+            app.screen.query_one("#yes", Button).press()
+            for _ in range(200):
+                if started.is_set():
+                    break
+                await pilot.pause(0.01)
+            assert started.is_set()
+            await pilot.press("f")
+            await pilot.pause()
+            if isinstance(app.screen, ConfirmScreen):  # refusal may come at "yes"
+                app.screen.query_one("#yes", Button).press()
+                await pilot.pause()
+            assert any(sev == "warning" for _, sev in app.seen)
+            assert fake.events.count(("ping", 108)) == 1
+        finally:
+            release.set()
+        await settle(app, pilot)
+        assert fake.events.count(("ping", 108)) == 1
+        assert fake.events.count(("fstrim", 108)) == 1
+
+
+# --- #19 review findings ----------------------------------------------------------------
+
+
+async def test_notifications_never_parse_markup(fake):
+    # guest names, paths and agent errors are untrusted text; notify must show them literally
+    app = make_app()
+    async with app.run_test(size=SIZE) as pilot:
+        await settle(app, pilot)
+        app.notify("path [/b] failed")
+        await pilot.pause()
+        assert app.is_running
+        assert all(n.markup is False for n in app._notifications)
+
+
+async def test_fstrim_without_results_says_so_and_singular_path(fake):
+    agent_vm(fake)
+    app = make_app()
+    async with app.run_test(size=SIZE) as pilot:
+        fake.fstrim_paths = []
+        await open_info(app, pilot)
+        await press_f(app, pilot)
+        await press_button(app, pilot, "#yes")
+        assert any("no result" in m.lower() for m, _ in app.seen)
+        fake.fstrim_paths = [{"path": "/", "trimmed": 2**20, "error": None}]
+        await press_f(app, pilot)
+        await press_button(app, pilot, "#yes")
+        assert any("in 1 path" in m and "paths" not in m for m, _ in app.seen)
+
+
+async def test_fstrim_before_the_info_screen_loaded_says_still_loading(
+    fake, monkeypatch
+):
+    agent_vm(fake)
+    release = threading.Event()
+    real = cli.guest_config
+    monkeypatch.setattr(cli, "guest_config", lambda g: (release.wait(10), real(g))[1])
+    app = make_app()
+    async with app.run_test(size=SIZE) as pilot:
+        await pilot.pause()
+        await pilot.press("enter")
+        await pilot.pause()
+        try:
+            await pilot.press("f")
+            await pilot.pause()
+            assert any("loading" in m.lower() for m, _ in app.seen)
+            assert fake.events == []
+        finally:
+            release.set()
+        await settle(app, pilot)

tests/test_cli.py

@@ -1504,3 +1504,120 @@ def test_tags_value_is_idempotent_property(text):
     else:
         assert cli.tags_value(once) == once
         assert len(set(once.split(";"))) == len(once.split(";"))  # no duplicates
+
+
+# --- issue #19: guest-agent fstrim (spec: "fstrim in a VM via the guest agent") ----------
+
+
+@pytest.mark.parametrize(
+    "agent,expected",
+    [
+        ("enabled=1", True),
+        ("1", True),
+        ("1,fstrim_cloned_disks=1", True),
+        ("enabled=0", False),
+        ("0", False),
+        (None, False),
+    ],
+)
+def test_agent_enabled(agent, expected):
+    config = {} if agent is None else {"agent": agent}
+    assert cli.agent_enabled(config) is expected
+
+
+def test_agent_ping_exact_argv(fake):
+    fake.result = (json.dumps([{"key": "result", "value": "{}"}]), "", 0)
+    cli.agent_ping(VMID108)
+    assert fake.argv == [
+        "cv4pve-cli", "do", "vm", "agent", "ping", "mainframe", "108", "-o", "json",
+    ]  # fmt: skip
+
+
+def test_agent_ping_nonzero_exit_raises_clierror(monkeypatch):
+    FakeRun(monkeypatch, "Error: QEMU guest agent is not running", returncode=5)
+    with pytest.raises(cli.CliError):
+        cli.agent_ping(VMID108)
+
+
+FSTRIM_PATHS = [
+    {"path": "/", "trimmed": 1073741824, "minimum": 0},
+    {"path": "/boot", "error": "not supported"},
+]
+
+
+def fstrim_stdout(value):
+    return json.dumps([{"key": "result", "value": value}])
+
+
+def test_fstrim_exact_argv_and_parses_paths(fake):
+    fake.result = (fstrim_stdout(json.dumps({"paths": FSTRIM_PATHS})), "", 0)
+    assert cli.fstrim(VMID108) == FSTRIM_PATHS
+    assert fake.argv == [
+        "cv4pve-cli", "do", "vm", "agent", "fstrim", "mainframe", "108", "-o", "json",
+    ]  # fmt: skip
+
+
+@pytest.mark.parametrize(
+    "stdout",
+    [
+        "[]",
+        json.dumps([{"key": "other", "value": "{}"}]),
+        fstrim_stdout("not json at all"),
+        fstrim_stdout("{}"),
+        fstrim_stdout(""),
+    ],
+)
+def test_fstrim_missing_or_garbage_result_is_empty_list(fake, stdout):
+    fake.result = (stdout, "", 0)
+    assert cli.fstrim(VMID108) == []
+
+
+def test_fstrim_nonzero_exit_raises_clierror(monkeypatch):
+    FakeRun(monkeypatch, "Error: fstrim failed", returncode=5)
+    with pytest.raises(cli.CliError):
+        cli.fstrim(VMID108)
+
+
+# --- #19 review findings ----------------------------------------------------------------
+
+VM19 = {"vmid": 108, "node": "mainframe", "type": "qemu"}
+
+
+@pytest.mark.parametrize(
+    "stdout",
+    [
+        "garbage",
+        "",
+        json.dumps({"key": "result"}),  # object instead of list
+        json.dumps([{"value": "{}"}]),  # item without "key"
+        json.dumps([{"key": "result", "value": {"paths": "x"}}]),  # already a dict
+    ],
+)
+def test_fstrim_unreadable_output_is_empty_not_an_error(fake, stdout):
+    fake.result = (stdout, "", 0)
+    assert cli.fstrim(VM19) == []
+
+
+def test_fstrim_normalises_odd_path_entries(fake):
+    paths = [
+        {"path": "/", "trimmed": "12"},
+        {"error": "x"},
+        "junk",
+        {"path": "/var", "trimmed": 5},
+    ]
+    fake.result = (
+        json.dumps([{"key": "result", "value": json.dumps({"paths": paths})}]),
+        "",
+        0,
+    )
+    assert cli.fstrim(VM19) == [
+        {"path": "/", "trimmed": 0},  # non-int trimmed -> 0
+        {"error": "x", "path": "?"},  # missing path -> "?"; "junk" dropped
+        {"path": "/var", "trimmed": 5},  # well-formed: unchanged
+    ]
+
+
+def test_fstrim_gets_a_longer_timeout_than_other_calls(fake):
+    fake.result = (json.dumps([{"key": "result", "value": "{}"}]), "", 0)
+    cli.fstrim(VM19)
+    assert fake.calls[0][1]["timeout"] >= 1800  # a big disk can take longer than 330 s