Skip to content

Conversation

@bmr-cymru
Copy link
Contributor

@bmr-cymru bmr-cymru commented Dec 24, 2025

Resolves: #827
Resolves: #825
Resolves: #829
Resolved: #828

Summary by CodeRabbit

  • Documentation
    • Documented a new Difference engine with multiple snapshot comparison output formats (tree, diff, summary, paths).
    • Added Screenshots section illustrating tree output and diff reports.
    • Expanded Quick Start with an "Understanding Changes" tutorial and practical diff command examples.
    • Added comprehensive Mount Manager docs, usage examples and lifecycle notes, plus user-facing commands: mount, umount, exec and shell.
    • Updated snapshot listings and JSON examples to include mounted, origin_mounted and mount_root fields.
    • Added logo attribution.

✏️ Tip: You can customize this high-level summary in your review settings.

Resolves: #825

Signed-off-by: Bryn M. Reeves <bmr@redhat.com>
Resolves: #829

Signed-off-by: Bryn M. Reeves <bmr@redhat.com>
@bmr-cymru bmr-cymru added enhancement New feature or request DifferenceEngine labels Dec 24, 2025
@coderabbitai
Copy link

coderabbitai bot commented Dec 24, 2025

Walkthrough

Documentation updates add a Difference Engine section and multiple diff output formats to README.md, and a Mount Manager section plus new public commands and snapshot fields to doc/user_guide.rst. No code or API changes.

Changes

Cohort / File(s) Summary
Difference engine / README
README.md
Added "Difference engine" description, multiple diff output formats (tree, diff, summary, paths), screenshots (tree output and diff report), "Understanding Changes" tutorial with example commands and a sample tabular diff report, expanded Quick Start examples, and logo attribution.
Mount Manager / User guide
doc/user_guide.rst
Added Mount Manager section: purpose and mounting workflow, automatic mount management semantics, interactive access and direct mount-point access, usage examples, and new public command docs for snapset mount, snapset umount, snapset exec, and snapset shell. Added snapshot set fields (mounted, origin_mounted, mount_root) and updated show/list and JSON sample outputs and headers.

Sequence Diagram(s)

(omitted — changes are documentation additions and do not modify runtime control flow)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

Suggested labels

documentation, UI/UX

Poem

🐰
I hopped through README with a smile so wide,
Found diffs and mounts tucked safely inside,
Screenshots and commands to lead the way,
Tux tips his hat as snapshots play,
A rabbit's small clap for docs that guide 🥕

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: documentation updates to README.md and user_guide.rst covering both the mount manager and difference engine features.
Linked Issues check ✅ Passed All linked issues are addressed: #827 update of both files, #825 difference engine in README.md, #829 Larry Ewing attribution, and #828 mount manager documentation in user_guide.rst.
Out of Scope Changes check ✅ Passed All changes are directly related to documentation objectives; README.md and user_guide.rst updates address mount manager and difference engine documentation as required.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bmr-docs-update-diff-mount

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e71b626 and 65960fe.

📒 Files selected for processing (1)
  • doc/user_guide.rst
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 0
File: :0-0
Timestamp: 2025-08-25T17:19:19.563Z
Learning: The man page changes mentioned in the AI summary for PR #383 were rebasing artifacts and should be ignored when reviewing this branch - there are no actual man page changes on the bmr-vm-tests branch.
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 541
File: snapm/manager/_mounts.py:174-197
Timestamp: 2025-10-22T20:06:54.993Z
Learning: For snapshotmanager/snapm follow-up requests from bmr-cymru, create a GitHub issue with label "enhancement", set/ensure a "type: Task" label, and assign it to bmr-cymru. Include backlinks to the PR and the discussion comment in the issue body.
📚 Learning: 2025-10-21T14:48:15.958Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 541
File: snapm/manager/_mounts.py:205-236
Timestamp: 2025-10-21T14:48:15.958Z
Learning: In snapm/manager/_mounts.py (and generally for mount operations): since util-linux 2.23 and Linux kernel 2.6.15+, mount propagation flags like --make-rprivate and --make-unbindable can be combined with the initial mount command in a single invocation. The mount utility automatically applies these flags via additional mount(2) system calls after the mount succeeds. It is not necessary to split these into separate remount commands.
<!--

Applied to files:

  • doc/user_guide.rst
📚 Learning: 2025-10-20T21:12:52.344Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 541
File: snapm/manager/_mounts.py:372-377
Timestamp: 2025-10-20T21:12:52.344Z
Learning: In snapm/manager/_mounts.py mount operations: do not create missing mount point directories on snapshot filesystems; instead, check if the directory exists with isdir(), log a warning if missing, and skip that mount to avoid modifying the snapshot filesystem unnecessarily.

Applied to files:

  • doc/user_guide.rst
📚 Learning: 2025-10-22T20:06:54.993Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 541
File: snapm/manager/_mounts.py:174-197
Timestamp: 2025-10-22T20:06:54.993Z
Learning: For snapshotmanager/snapm follow-up requests from bmr-cymru, create a GitHub issue with label "enhancement", set/ensure a "type: Task" label, and assign it to bmr-cymru. Include backlinks to the PR and the discussion comment in the issue body.

Applied to files:

  • doc/user_guide.rst
📚 Learning: 2025-08-25T17:19:19.563Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 0
File: :0-0
Timestamp: 2025-08-25T17:19:19.563Z
Learning: The man page changes mentioned in the AI summary for PR #383 were rebasing artifacts and should be ignored when reviewing this branch - there are no actual man page changes on the bmr-vm-tests branch.

Applied to files:

  • doc/user_guide.rst
📚 Learning: 2025-09-14T02:14:30.316Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 0
File: :0-0
Timestamp: 2025-09-14T02:14:30.316Z
Learning: PR #478 in snapshotmanager/snapm fixes a regression in the LVM2 plugin where `can_snapshot()` method throws `SnapmCalloutError` when probing non-device-mapper devices. The fix adds device major number validation to prevent calling dmsetup on non-dm devices, allowing proper "no provider found" error messages instead of confusing dmsetup errors.

Applied to files:

  • doc/user_guide.rst
📚 Learning: 2025-12-07T14:28:10.096Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 0
File: :0-0
Timestamp: 2025-12-07T14:28:10.096Z
Learning: In the snapshotmanager/snapm codebase, the `MountBase.exec()` method in `snapm/manager/_mounts.py` intentionally supports long-running commands (e.g., interactive shells) and should not have timeouts added, as they would break legitimate use cases where commands need to run indefinitely or for extended periods.

Applied to files:

  • doc/user_guide.rst
📚 Learning: 2025-12-13T00:14:20.405Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 0
File: :0-0
Timestamp: 2025-12-13T00:14:20.405Z
Learning: In the snapm codebase, snapshot set names are unique and two mounts cannot exist for the same `Selection(name=...)`. Therefore, `manager.mounts.find_mounts(Selection(name=...))` cannot return multiple mounts.

Applied to files:

  • doc/user_guide.rst
📚 Learning: 2025-09-09T19:26:01.116Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 454
File: man/man8/snapm.8:1434-1462
Timestamp: 2025-09-09T19:26:01.116Z
Learning: The `list` command JSON format in snapm uses a wrapper object structure with a type-specific key (e.g., "Snapsets", "Snapshots") that maps to an array of objects, rather than a direct JSON array. This design provides flexibility for future mixed-object-type reports and follows patterns similar to lvm2's JSON reports. For example: `{"Snapsets": [{"name": "backup", ...}, {"name": "before-upgrade", ...}]}`.

Applied to files:

  • doc/user_guide.rst
📚 Learning: 2025-09-09T19:56:30.429Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 454
File: man/man8/snapm.8:1433-1461
Timestamp: 2025-09-09T19:56:30.429Z
Learning: In snapm JSON output, field names always use full prefixed names (e.g., "snapset_name", "snapset_status") to avoid collisions between different object types, even though prefixes may be optional in other contexts when unambiguous. This is an intentional architectural decision for consistency and conflict prevention in JSON reports.

Applied to files:

  • doc/user_guide.rst
📚 Learning: 2025-09-09T16:06:19.848Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 0
File: :0-0
Timestamp: 2025-09-09T16:06:19.848Z
Learning: In snapm CLI, `show` commands produce JSON with individual objects using fixed field names that match regular show command output, while `list` commands produce JSON arrays with keys derived from report field names that can be controlled by the caller (fields, ordering, sorting). This architectural difference is intentional - show format is fixed for predictability, list format is flexible for customizable reporting.

Applied to files:

  • doc/user_guide.rst
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (35)
  • GitHub Check: rpm-build:fedora-rawhide-x86_64
  • GitHub Check: rpm-build:fedora-42-x86_64
  • GitHub Check: rpm-build:epel-9-x86_64
  • GitHub Check: rpm-build:fedora-43-x86_64
  • GitHub Check: rpm-build:centos-stream-9-x86_64
  • GitHub Check: rpm-build:centos-stream-10-x86_64
  • GitHub Check: rpm-build:fedora-43-x86_64
  • GitHub Check: rpm-build:epel-9-x86_64
  • GitHub Check: rpm-build:centos-stream-10-x86_64
  • GitHub Check: rpm-build:centos-stream-9-x86_64
  • GitHub Check: rpm-build:fedora-rawhide-x86_64
  • GitHub Check: rpm-build:fedora-42-x86_64
  • GitHub Check: rpm-build:fedora-43-x86_64
  • GitHub Check: rpm-build:centos-stream-9-x86_64
  • GitHub Check: rpm-build:epel-9-x86_64
  • GitHub Check: rpm-build:fedora-42-x86_64
  • GitHub Check: rpm-build:fedora-rawhide-x86_64
  • GitHub Check: rpm-build:centos-stream-10-x86_64
  • GitHub Check: virt_tests (bios, lvm, centos-stream10)
  • GitHub Check: virt_tests (bios, lvm, fedora41)
  • GitHub Check: virt_tests (bios, lvm-thin, fedora41)
  • GitHub Check: virt_tests (bios, lvm, fedora42)
  • GitHub Check: virt_tests (uefi, lvm-thin, fedora42)
  • GitHub Check: virt_tests (uefi, lvm, centos-stream10)
  • GitHub Check: virt_tests (uefi, lvm, fedora41)
  • GitHub Check: virt_tests (uefi, lvm, fedora42)
  • GitHub Check: virt_tests (uefi, lvm, centos-stream9)
  • GitHub Check: virt_tests (bios, lvm, centos-stream9)
  • GitHub Check: virt_tests (uefi, lvm-thin, centos-stream10)
  • GitHub Check: virt_tests (bios, lvm-thin, centos-stream9)
  • GitHub Check: virt_tests (bios, lvm-thin, centos-stream10)
  • GitHub Check: virt_tests (uefi, lvm-thin, centos-stream9)
  • GitHub Check: virt_tests (bios, lvm-thin, fedora42)
  • GitHub Check: virt_tests (uefi, lvm-thin, fedora41)
  • GitHub Check: python-basic-tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@packit-as-a-service
Copy link

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo dnf install -y 'dnf*-command(copr)'
  • dnf copr enable packit/snapshotmanager-snapm-830
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2e0a14c and f0f4157.

📒 Files selected for processing (1)
  • README.md
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 0
File: :0-0
Timestamp: 2025-08-25T17:19:19.563Z
Learning: The man page changes mentioned in the AI summary for PR #383 were rebasing artifacts and should be ignored when reviewing this branch - there are no actual man page changes on the bmr-vm-tests branch.
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 541
File: snapm/manager/_mounts.py:174-197
Timestamp: 2025-10-22T20:06:54.993Z
Learning: For snapshotmanager/snapm follow-up requests from bmr-cymru, create a GitHub issue with label "enhancement", set/ensure a "type: Task" label, and assign it to bmr-cymru. Include backlinks to the PR and the discussion comment in the issue body.
📚 Learning: 2025-09-10T23:42:35.992Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 357
File: snapm.spec:132-134
Timestamp: 2025-09-10T23:42:35.992Z
Learning: In the snapm project, changelog entries in snapm.spec are auto-generated from git log using `git log --oneline v0.4.0..v0.5.0 | sed  -e 's/^[a-f0-9]* /- /' -e 's/%//'` and should not be manually edited to avoid rewriting git history.

Applied to files:

  • README.md
📚 Learning: 2025-09-09T16:06:19.848Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 0
File: :0-0
Timestamp: 2025-09-09T16:06:19.848Z
Learning: In snapm CLI, `show` commands produce JSON with individual objects using fixed field names that match regular show command output, while `list` commands produce JSON arrays with keys derived from report field names that can be controlled by the caller (fields, ordering, sorting). This architectural difference is intentional - show format is fixed for predictability, list format is flexible for customizable reporting.

Applied to files:

  • README.md
📚 Learning: 2025-12-22T10:31:19.936Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 773
File: snapm.spec:148-149
Timestamp: 2025-12-22T10:31:19.936Z
Learning: In the snapm project's snapm.spec file, do not flag typos, formatting issues, or other problems in the %changelog section, as these entries are auto-generated from git commit messages and represent the historical record. Modifying them would require rewriting git history, which the project avoids.

Applied to files:

  • README.md
📚 Learning: 2025-08-22T19:34:08.396Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 0
File: :0-0
Timestamp: 2025-08-22T19:34:08.396Z
Learning: The snapshotmanager organization (including snapm and boom projects) uses "SHORTMONTH DAY YEAR" format for man page headers. This format has been used by boom since 2017. Organizational consistency within snapshotmanager is prioritized over cross-project consistency when it comes to formatting standards.

Applied to files:

  • README.md
📚 Learning: 2025-08-30T13:46:15.300Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 0
File: :0-0
Timestamp: 2025-08-30T13:46:15.300Z
Learning: In snapm, boom is a hard requirement because snapm.manager._boot imports boom modules directly, and the Manager class imports and uses functionality from _boot including BootCache, create_snapset_boot_entry, and delete_snapset_boot_entry. The Manager class cannot be instantiated without boom being available.

Applied to files:

  • README.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (34)
  • GitHub Check: rpm-build:fedora-rawhide-x86_64
  • GitHub Check: rpm-build:fedora-42-x86_64
  • GitHub Check: rpm-build:epel-9-x86_64
  • GitHub Check: rpm-build:fedora-43-x86_64
  • GitHub Check: rpm-build:centos-stream-9-x86_64
  • GitHub Check: rpm-build:centos-stream-10-x86_64
  • GitHub Check: rpm-build:fedora-43-x86_64
  • GitHub Check: rpm-build:fedora-rawhide-x86_64
  • GitHub Check: rpm-build:centos-stream-10-x86_64
  • GitHub Check: rpm-build:fedora-42-x86_64
  • GitHub Check: rpm-build:centos-stream-9-x86_64
  • GitHub Check: rpm-build:epel-9-x86_64
  • GitHub Check: rpm-build:centos-stream-10-x86_64
  • GitHub Check: rpm-build:centos-stream-9-x86_64
  • GitHub Check: rpm-build:fedora-43-x86_64
  • GitHub Check: rpm-build:fedora-rawhide-x86_64
  • GitHub Check: rpm-build:fedora-42-x86_64
  • GitHub Check: rpm-build:epel-9-x86_64
  • GitHub Check: virt_tests (bios, lvm, fedora42)
  • GitHub Check: virt_tests (bios, lvm-thin, centos-stream9)
  • GitHub Check: virt_tests (bios, lvm-thin, fedora41)
  • GitHub Check: virt_tests (uefi, lvm, centos-stream10)
  • GitHub Check: virt_tests (bios, lvm, centos-stream9)
  • GitHub Check: virt_tests (uefi, lvm, fedora41)
  • GitHub Check: virt_tests (uefi, lvm-thin, centos-stream9)
  • GitHub Check: virt_tests (uefi, lvm-thin, fedora42)
  • GitHub Check: virt_tests (bios, lvm-thin, fedora42)
  • GitHub Check: virt_tests (bios, lvm-thin, centos-stream10)
  • GitHub Check: virt_tests (uefi, lvm, fedora42)
  • GitHub Check: virt_tests (uefi, lvm, centos-stream9)
  • GitHub Check: virt_tests (bios, lvm, centos-stream10)
  • GitHub Check: virt_tests (uefi, lvm-thin, centos-stream10)
  • GitHub Check: virt_tests (bios, lvm, fedora41)
  • GitHub Check: python-basic-tests
🔇 Additional comments (3)
README.md (3)

23-24: Feature description is clear and well‑placed.

The difference engine feature entry fits naturally into the existing list and accurately describes the functionality. This addresses issue #825.


198-234: Excellent tutorial section with comprehensive command examples.

The "Understanding Changes" section effectively demonstrates the difference engine functionality with clear, progressive examples covering all major output formats. The examples are practical and consistent with the command syntax shown elsewhere in the documentation.


28-40: Images are accessible and properly linked.

Both screenshot images referenced in the Screenshots section are accessible and return HTTP 200 status. The section is well‑structured with clear descriptions and functional external references.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f0f4157 and 03ed7e2.

📒 Files selected for processing (1)
  • doc/user_guide.rst
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 0
File: :0-0
Timestamp: 2025-08-25T17:19:19.563Z
Learning: The man page changes mentioned in the AI summary for PR #383 were rebasing artifacts and should be ignored when reviewing this branch - there are no actual man page changes on the bmr-vm-tests branch.
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 541
File: snapm/manager/_mounts.py:174-197
Timestamp: 2025-10-22T20:06:54.993Z
Learning: For snapshotmanager/snapm follow-up requests from bmr-cymru, create a GitHub issue with label "enhancement", set/ensure a "type: Task" label, and assign it to bmr-cymru. Include backlinks to the PR and the discussion comment in the issue body.
📚 Learning: 2025-10-21T14:48:15.958Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 541
File: snapm/manager/_mounts.py:205-236
Timestamp: 2025-10-21T14:48:15.958Z
Learning: In snapm/manager/_mounts.py (and generally for mount operations): since util-linux 2.23 and Linux kernel 2.6.15+, mount propagation flags like --make-rprivate and --make-unbindable can be combined with the initial mount command in a single invocation. The mount utility automatically applies these flags via additional mount(2) system calls after the mount succeeds. It is not necessary to split these into separate remount commands.
<!--

Applied to files:

  • doc/user_guide.rst
📚 Learning: 2025-10-20T21:12:52.344Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 541
File: snapm/manager/_mounts.py:372-377
Timestamp: 2025-10-20T21:12:52.344Z
Learning: In snapm/manager/_mounts.py mount operations: do not create missing mount point directories on snapshot filesystems; instead, check if the directory exists with isdir(), log a warning if missing, and skip that mount to avoid modifying the snapshot filesystem unnecessarily.

Applied to files:

  • doc/user_guide.rst
📚 Learning: 2025-12-07T14:28:10.096Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 0
File: :0-0
Timestamp: 2025-12-07T14:28:10.096Z
Learning: In the snapshotmanager/snapm codebase, the `MountBase.exec()` method in `snapm/manager/_mounts.py` intentionally supports long-running commands (e.g., interactive shells) and should not have timeouts added, as they would break legitimate use cases where commands need to run indefinitely or for extended periods.

Applied to files:

  • doc/user_guide.rst
📚 Learning: 2025-12-13T00:14:20.405Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 0
File: :0-0
Timestamp: 2025-12-13T00:14:20.405Z
Learning: In the snapm codebase, snapshot set names are unique and two mounts cannot exist for the same `Selection(name=...)`. Therefore, `manager.mounts.find_mounts(Selection(name=...))` cannot return multiple mounts.

Applied to files:

  • doc/user_guide.rst
📚 Learning: 2025-09-09T19:26:01.116Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 454
File: man/man8/snapm.8:1434-1462
Timestamp: 2025-09-09T19:26:01.116Z
Learning: The `list` command JSON format in snapm uses a wrapper object structure with a type-specific key (e.g., "Snapsets", "Snapshots") that maps to an array of objects, rather than a direct JSON array. This design provides flexibility for future mixed-object-type reports and follows patterns similar to lvm2's JSON reports. For example: `{"Snapsets": [{"name": "backup", ...}, {"name": "before-upgrade", ...}]}`.

Applied to files:

  • doc/user_guide.rst
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (35)
  • GitHub Check: rpm-build:fedora-43-x86_64
  • GitHub Check: rpm-build:fedora-rawhide-x86_64
  • GitHub Check: rpm-build:centos-stream-10-x86_64
  • GitHub Check: rpm-build:fedora-42-x86_64
  • GitHub Check: rpm-build:centos-stream-9-x86_64
  • GitHub Check: rpm-build:epel-9-x86_64
  • GitHub Check: rpm-build:fedora-43-x86_64
  • GitHub Check: rpm-build:fedora-rawhide-x86_64
  • GitHub Check: rpm-build:centos-stream-10-x86_64
  • GitHub Check: rpm-build:fedora-42-x86_64
  • GitHub Check: rpm-build:centos-stream-9-x86_64
  • GitHub Check: rpm-build:epel-9-x86_64
  • GitHub Check: rpm-build:fedora-42-x86_64
  • GitHub Check: rpm-build:centos-stream-9-x86_64
  • GitHub Check: rpm-build:fedora-rawhide-x86_64
  • GitHub Check: rpm-build:fedora-43-x86_64
  • GitHub Check: rpm-build:centos-stream-10-x86_64
  • GitHub Check: rpm-build:epel-9-x86_64
  • GitHub Check: virt_tests (bios, lvm, fedora42)
  • GitHub Check: virt_tests (bios, lvm, centos-stream10)
  • GitHub Check: virt_tests (bios, lvm-thin, centos-stream9)
  • GitHub Check: virt_tests (uefi, lvm-thin, fedora42)
  • GitHub Check: virt_tests (bios, lvm, fedora41)
  • GitHub Check: virt_tests (uefi, lvm, centos-stream9)
  • GitHub Check: virt_tests (uefi, lvm-thin, centos-stream9)
  • GitHub Check: virt_tests (uefi, lvm, fedora42)
  • GitHub Check: virt_tests (bios, lvm, centos-stream9)
  • GitHub Check: virt_tests (bios, lvm-thin, fedora41)
  • GitHub Check: virt_tests (bios, lvm-thin, centos-stream10)
  • GitHub Check: virt_tests (uefi, lvm-thin, fedora41)
  • GitHub Check: virt_tests (uefi, lvm, fedora41)
  • GitHub Check: virt_tests (uefi, lvm-thin, centos-stream10)
  • GitHub Check: virt_tests (bios, lvm-thin, fedora42)
  • GitHub Check: virt_tests (uefi, lvm, centos-stream10)
  • GitHub Check: python-basic-tests
🔇 Additional comments (4)
doc/user_guide.rst (4)

62-89: Mount Manager introduction and overview look solid.

The conceptual "How it Works" section clearly explains the mount process and the layering of snapshots with host bind-mounts. This provides good context for users new to the feature.


872-895: snapset mount command documentation is clear and functional.

The documentation provides the essential information: what the command does, where mount points are created, and a practical example. The reference to "appropriate paths within this hierarchy" is consistent with the conceptual "How it Works" section.


897-970: snapset umount, exec, and shell commands are well-documented.

All three command sections provide clear syntax, behaviour explanation, and practical examples. The automatic mount/unmount semantics are consistently explained across all three commands, and the emphasis on command argument handling and shell environment behaviour is helpful.


199-295: Difference Engine documentation is comprehensive and well-structured.

The section effectively covers the multi-stage analysis approach, provides detailed output format descriptions with use cases, documents the reportable fields for diffreport, explains comparison modes, and covers performance and caching. The cache expiry timing (15 minutes / 900 seconds) is internally consistent, and the warnings about cache behaviour with the running system (.) are helpful.

@bmr-cymru bmr-cymru force-pushed the bmr-docs-update-diff-mount branch from 03ed7e2 to e71b626 Compare December 24, 2025 22:57
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 03ed7e2 and e71b626.

📒 Files selected for processing (1)
  • doc/user_guide.rst
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 0
File: :0-0
Timestamp: 2025-08-25T17:19:19.563Z
Learning: The man page changes mentioned in the AI summary for PR #383 were rebasing artifacts and should be ignored when reviewing this branch - there are no actual man page changes on the bmr-vm-tests branch.
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 541
File: snapm/manager/_mounts.py:174-197
Timestamp: 2025-10-22T20:06:54.993Z
Learning: For snapshotmanager/snapm follow-up requests from bmr-cymru, create a GitHub issue with label "enhancement", set/ensure a "type: Task" label, and assign it to bmr-cymru. Include backlinks to the PR and the discussion comment in the issue body.
📚 Learning: 2025-10-21T14:48:15.958Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 541
File: snapm/manager/_mounts.py:205-236
Timestamp: 2025-10-21T14:48:15.958Z
Learning: In snapm/manager/_mounts.py (and generally for mount operations): since util-linux 2.23 and Linux kernel 2.6.15+, mount propagation flags like --make-rprivate and --make-unbindable can be combined with the initial mount command in a single invocation. The mount utility automatically applies these flags via additional mount(2) system calls after the mount succeeds. It is not necessary to split these into separate remount commands.
<!--

Applied to files:

  • doc/user_guide.rst
📚 Learning: 2025-10-20T21:12:52.344Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 541
File: snapm/manager/_mounts.py:372-377
Timestamp: 2025-10-20T21:12:52.344Z
Learning: In snapm/manager/_mounts.py mount operations: do not create missing mount point directories on snapshot filesystems; instead, check if the directory exists with isdir(), log a warning if missing, and skip that mount to avoid modifying the snapshot filesystem unnecessarily.

Applied to files:

  • doc/user_guide.rst
📚 Learning: 2025-12-07T14:28:10.096Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 0
File: :0-0
Timestamp: 2025-12-07T14:28:10.096Z
Learning: In the snapshotmanager/snapm codebase, the `MountBase.exec()` method in `snapm/manager/_mounts.py` intentionally supports long-running commands (e.g., interactive shells) and should not have timeouts added, as they would break legitimate use cases where commands need to run indefinitely or for extended periods.

Applied to files:

  • doc/user_guide.rst
📚 Learning: 2025-12-13T00:14:20.405Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 0
File: :0-0
Timestamp: 2025-12-13T00:14:20.405Z
Learning: In the snapm codebase, snapshot set names are unique and two mounts cannot exist for the same `Selection(name=...)`. Therefore, `manager.mounts.find_mounts(Selection(name=...))` cannot return multiple mounts.

Applied to files:

  • doc/user_guide.rst
📚 Learning: 2025-09-09T19:26:01.116Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 454
File: man/man8/snapm.8:1434-1462
Timestamp: 2025-09-09T19:26:01.116Z
Learning: The `list` command JSON format in snapm uses a wrapper object structure with a type-specific key (e.g., "Snapsets", "Snapshots") that maps to an array of objects, rather than a direct JSON array. This design provides flexibility for future mixed-object-type reports and follows patterns similar to lvm2's JSON reports. For example: `{"Snapsets": [{"name": "backup", ...}, {"name": "before-upgrade", ...}]}`.

Applied to files:

  • doc/user_guide.rst
📚 Learning: 2025-09-09T19:56:30.429Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 454
File: man/man8/snapm.8:1433-1461
Timestamp: 2025-09-09T19:56:30.429Z
Learning: In snapm JSON output, field names always use full prefixed names (e.g., "snapset_name", "snapset_status") to avoid collisions between different object types, even though prefixes may be optional in other contexts when unambiguous. This is an intentional architectural decision for consistency and conflict prevention in JSON reports.

Applied to files:

  • doc/user_guide.rst
📚 Learning: 2025-09-09T16:06:19.848Z
Learnt from: bmr-cymru
Repo: snapshotmanager/snapm PR: 0
File: :0-0
Timestamp: 2025-09-09T16:06:19.848Z
Learning: In snapm CLI, `show` commands produce JSON with individual objects using fixed field names that match regular show command output, while `list` commands produce JSON arrays with keys derived from report field names that can be controlled by the caller (fields, ordering, sorting). This architectural difference is intentional - show format is fixed for predictability, list format is flexible for customizable reporting.

Applied to files:

  • doc/user_guide.rst
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (34)
  • GitHub Check: rpm-build:fedora-43-x86_64
  • GitHub Check: rpm-build:epel-9-x86_64
  • GitHub Check: rpm-build:centos-stream-10-x86_64
  • GitHub Check: rpm-build:centos-stream-9-x86_64
  • GitHub Check: rpm-build:fedora-rawhide-x86_64
  • GitHub Check: rpm-build:fedora-42-x86_64
  • GitHub Check: rpm-build:fedora-43-x86_64
  • GitHub Check: rpm-build:fedora-rawhide-x86_64
  • GitHub Check: rpm-build:centos-stream-10-x86_64
  • GitHub Check: rpm-build:fedora-42-x86_64
  • GitHub Check: rpm-build:centos-stream-9-x86_64
  • GitHub Check: rpm-build:epel-9-x86_64
  • GitHub Check: rpm-build:fedora-43-x86_64
  • GitHub Check: rpm-build:centos-stream-9-x86_64
  • GitHub Check: rpm-build:epel-9-x86_64
  • GitHub Check: rpm-build:fedora-42-x86_64
  • GitHub Check: rpm-build:fedora-rawhide-x86_64
  • GitHub Check: rpm-build:centos-stream-10-x86_64
  • GitHub Check: virt_tests (bios, lvm-thin, fedora41)
  • GitHub Check: virt_tests (uefi, lvm, centos-stream9)
  • GitHub Check: virt_tests (uefi, lvm, fedora41)
  • GitHub Check: virt_tests (uefi, lvm-thin, fedora41)
  • GitHub Check: virt_tests (uefi, lvm-thin, fedora42)
  • GitHub Check: virt_tests (uefi, lvm, centos-stream10)
  • GitHub Check: virt_tests (uefi, lvm-thin, centos-stream10)
  • GitHub Check: virt_tests (uefi, lvm-thin, centos-stream9)
  • GitHub Check: virt_tests (bios, lvm-thin, centos-stream10)
  • GitHub Check: virt_tests (bios, lvm, fedora42)
  • GitHub Check: virt_tests (bios, lvm-thin, fedora42)
  • GitHub Check: virt_tests (bios, lvm-thin, centos-stream9)
  • GitHub Check: virt_tests (bios, lvm, centos-stream10)
  • GitHub Check: virt_tests (bios, lvm, fedora41)
  • GitHub Check: virt_tests (bios, lvm, centos-stream9)
  • GitHub Check: python-basic-tests
🔇 Additional comments (5)
doc/user_guide.rst (5)

62-198: Mount Manager section is comprehensive and well-structured.

The new Mount Manager documentation clearly explains the feature's purpose, how it works, automatic mount management, and provides practical examples for interactive access, command execution, and direct mount point access. The section integrates well with the overall documentation and makes the feature accessible to users.


199-295: Difference Engine section is thorough and appropriately positioned.

The Difference Engine section provides a solid foundation for understanding the feature, covering operation mechanics, supported output formats, comparison modes, and performance considerations. The section clearly references the diff and diffreport commands documented below, creating good cross-referencing.


691-708: Field reference lists correctly document new snapshot set fields.

Both field reference sections (lines 691–708 and 1212–1229) have been updated to include the three new fields:

  • mounted – SnapshotSet mount status
  • origin_mounted – SnapshotSet origin mount status
  • mount_root – SnapshotSet mount root directory path

These align with the documented Mount Manager functionality and are properly integrated into the field descriptions.

Also applies to: 1212-1229


381-382: JSON and example output consistently reflect new fields.

The updated examples throughout the documentation correctly show:

  • "Mounted" and "OriginMounted" fields in snapset show human-readable output (lines 381–382, 404–405, 792–793, 812, 875–876)
  • "snapset_mounted" field in snapset list JSON output (lines 747, 758, 1252, 1263)
  • "Mounted" and "OriginMounted" in snapset show JSON output (lines 875–876, 1293–1294)
  • "Mounted" in tabular list output (line 414–419)

This consistency across all output formats demonstrates proper integration of the new fields.

Also applies to: 404-405, 414-419, 747-747, 758-758, 875-876, 1252-1252, 1263-1263, 1293-1294


889-986: Mount-related command documentation is clear and examples-driven.

The new snapset mount, snapset umount, snapset exec, and snapset shell commands are documented with clear descriptions and practical examples showing typical usage patterns. The examples demonstrate both automated mounting behaviour and explicit mount/unmount workflows, helping users understand when to use which approach.

Resolves: #828

Signed-off-by: Bryn M. Reeves <bmr@redhat.com>
@bmr-cymru bmr-cymru force-pushed the bmr-docs-update-diff-mount branch from e71b626 to 65960fe Compare December 24, 2025 23:11
@bmr-cymru bmr-cymru merged commit 65960fe into main Dec 24, 2025
26 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DifferenceEngine enhancement New feature or request

Projects

None yet

2 participants