feat(prepare): exclude loop and virtual devices from host LVM scanning#49
Conversation
Set an LVM global_filter in /etc/lvm/lvm.conf in the prepare playbooks so the host LVM does not scan or activate volume groups backed by LINSTOR/DRBD volumes — or located inside loop-mounted images. Filters /dev/drbd*, /dev/dm-*, /dev/zd* and /dev/loop*, mirroring the Talos machine config. Applied to the ubuntu, rhel and suse prepare playbooks via lineinfile so the distro's default lvm.conf is preserved. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughFour files are updated to add LVM global_filter configuration: changelog documentation and three distribution-specific Ansible playbooks (RHEL, SUSE, Ubuntu) now configure ChangesLVM Global Filter Configuration for Host LVM Isolation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds Ansible tasks to exclude virtual and loop devices (DRBD, device-mapper, zvol, and loop devices) from host LVM scanning across RHEL, SUSE, and Ubuntu playbooks. The review feedback highlights two critical issues: first, the insertafter regular expression is too strict and could fail to match if there is leading whitespace, causing the configuration to be appended incorrectly; second, unconditionally excluding /dev/dm-.* will break systems using LVM on top of LUKS or Multipath. It is recommended to use a more robust regex and expose the filter list as a customizable variable with a default fallback.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| - name: Exclude virtual and loop devices from host LVM scanning | ||
| ansible.builtin.lineinfile: | ||
| path: /etc/lvm/lvm.conf | ||
| regexp: '^\s*#?\s*global_filter\s*=' | ||
| insertafter: '^devices {' | ||
| line: ' global_filter = [ "r|^/dev/drbd.*|", "r|^/dev/dm-.*|", "r|^/dev/zd.*|", "r|^/dev/loop.*|" ]' |
There was a problem hiding this comment.
This task introduces two issues that should be addressed:
-
Robustness of
insertafter: The regex^devices {is very strict and requires the line to start exactly withdevices {(no leading whitespace, exactly one space before the brace). If the target system'slvm.confhas any leading indentation or different spacing (e.g.,devices{), the regex will fail to match. Wheninsertafterfails to match,lineinfileappends the line to the end of the file. In LVM configuration, settings outside their respective blocks (likedevices { ... }) are invalid or ignored, which would break the configuration or prevent the filter from working. Using^\s*devices\s*{is much more robust. -
LUKS and Multipath Compatibility: Rejecting
/dev/dm-.*in theglobal_filterprevents the host LVM from scanning any device-mapper devices. This will completely break hosts that use LVM on top of LUKS (encrypted partitions) or LVM on top of Multipath (MPIO), as their physical volumes (PVs) reside on/dev/dm-Xdevices. To make this safe and customizable, we should expose the filter list as a variable (e.g.,cozystack_lvm_global_filter) using thedefaultfilter. This allows users with LUKS or Multipath to override the filter in their inventory (e.g., by removing"r|^/dev/dm-.*|"), while keeping the current default behavior for dedicated storage nodes.
- name: Exclude virtual and loop devices from host LVM scanning
ansible.builtin.lineinfile:
path: /etc/lvm/lvm.conf
regexp: '^\s*#?\s*global_filter\s*='
insertafter: '^\s*devices\s*{'
line: ' global_filter = {{ cozystack_lvm_global_filter | default(["r|^/dev/drbd.*|", "r|^/dev/dm-.*|", "r|^/dev/zd.*|", "r|^/dev/loop.*|"]) | to_json }}'There was a problem hiding this comment.
Both points addressed in 0657c42.
insertafteris now^\s*devices\s*{, so the setting still lands inside thedevices{}block whenlvm.confuses leading whitespace or compact bracing, instead of being appended at EOF where LVM ignores it.- The filter list is now exposed as
cozystack_lvm_global_filter(same default), rendered withto_json. Hosts whose own PVs live on device-mapper devices (LVM-on-LUKS, multipath) can override it from inventory — e.g. drop ther|^/dev/dm-.*|entry.
| - name: Exclude virtual and loop devices from host LVM scanning | ||
| ansible.builtin.lineinfile: | ||
| path: /etc/lvm/lvm.conf | ||
| regexp: '^\s*#?\s*global_filter\s*=' | ||
| insertafter: '^devices {' | ||
| line: ' global_filter = [ "r|^/dev/drbd.*|", "r|^/dev/dm-.*|", "r|^/dev/zd.*|", "r|^/dev/loop.*|" ]' |
There was a problem hiding this comment.
This task introduces two issues that should be addressed:
-
Robustness of
insertafter: The regex^devices {is very strict and requires the line to start exactly withdevices {(no leading whitespace, exactly one space before the brace). If the target system'slvm.confhas any leading indentation or different spacing (e.g.,devices{), the regex will fail to match. Wheninsertafterfails to match,lineinfileappends the line to the end of the file. In LVM configuration, settings outside their respective blocks (likedevices { ... }) are invalid or ignored, which would break the configuration or prevent the filter from working. Using^\s*devices\s*{is much more robust. -
LUKS and Multipath Compatibility: Rejecting
/dev/dm-.*in theglobal_filterprevents the host LVM from scanning any device-mapper devices. This will completely break hosts that use LVM on top of LUKS (encrypted partitions) or LVM on top of Multipath (MPIO), as their physical volumes (PVs) reside on/dev/dm-Xdevices. To make this safe and customizable, we should expose the filter list as a variable (e.g.,cozystack_lvm_global_filter) using thedefaultfilter. This allows users with LUKS or Multipath to override the filter in their inventory (e.g., by removing"r|^/dev/dm-.*|"), while keeping the current default behavior for dedicated storage nodes.
- name: Exclude virtual and loop devices from host LVM scanning
ansible.builtin.lineinfile:
path: /etc/lvm/lvm.conf
regexp: '^\s*#?\s*global_filter\s*='
insertafter: '^\s*devices\s*{'
line: ' global_filter = {{ cozystack_lvm_global_filter | default(["r|^/dev/drbd.*|", "r|^/dev/dm-.*|", "r|^/dev/zd.*|", "r|^/dev/loop.*|"]) | to_json }}'There was a problem hiding this comment.
Both points addressed in 0657c42.
insertafteris now^\s*devices\s*{, so the setting still lands inside thedevices{}block whenlvm.confuses leading whitespace or compact bracing, instead of being appended at EOF where LVM ignores it.- The filter list is now exposed as
cozystack_lvm_global_filter(same default), rendered withto_json. Hosts whose own PVs live on device-mapper devices (LVM-on-LUKS, multipath) can override it from inventory — e.g. drop ther|^/dev/dm-.*|entry.
| - name: Exclude virtual and loop devices from host LVM scanning | ||
| ansible.builtin.lineinfile: | ||
| path: /etc/lvm/lvm.conf | ||
| regexp: '^\s*#?\s*global_filter\s*=' | ||
| insertafter: '^devices {' | ||
| line: ' global_filter = [ "r|^/dev/drbd.*|", "r|^/dev/dm-.*|", "r|^/dev/zd.*|", "r|^/dev/loop.*|" ]' |
There was a problem hiding this comment.
This task introduces two issues that should be addressed:
-
Robustness of
insertafter: The regex^devices {is very strict and requires the line to start exactly withdevices {(no leading whitespace, exactly one space before the brace). If the target system'slvm.confhas any leading indentation or different spacing (e.g.,devices{), the regex will fail to match. Wheninsertafterfails to match,lineinfileappends the line to the end of the file. In LVM configuration, settings outside their respective blocks (likedevices { ... }) are invalid or ignored, which would break the configuration or prevent the filter from working. Using^\s*devices\s*{is much more robust. -
LUKS and Multipath Compatibility: Rejecting
/dev/dm-.*in theglobal_filterprevents the host LVM from scanning any device-mapper devices. This will completely break hosts that use LVM on top of LUKS (encrypted partitions) or LVM on top of Multipath (MPIO), as their physical volumes (PVs) reside on/dev/dm-Xdevices. To make this safe and customizable, we should expose the filter list as a variable (e.g.,cozystack_lvm_global_filter) using thedefaultfilter. This allows users with LUKS or Multipath to override the filter in their inventory (e.g., by removing"r|^/dev/dm-.*|"), while keeping the current default behavior for dedicated storage nodes.
- name: Exclude virtual and loop devices from host LVM scanning
ansible.builtin.lineinfile:
path: /etc/lvm/lvm.conf
regexp: '^\s*#?\s*global_filter\s*='
insertafter: '^\s*devices\s*{'
line: ' global_filter = {{ cozystack_lvm_global_filter | default(["r|^/dev/drbd.*|", "r|^/dev/dm-.*|", "r|^/dev/zd.*|", "r|^/dev/loop.*|"]) | to_json }}'There was a problem hiding this comment.
Both points addressed in 0657c42.
insertafteris now^\s*devices\s*{, so the setting still lands inside thedevices{}block whenlvm.confuses leading whitespace or compact bracing, instead of being appended at EOF where LVM ignores it.- The filter list is now exposed as
cozystack_lvm_global_filter(same default), rendered withto_json. Hosts whose own PVs live on device-mapper devices (LVM-on-LUKS, multipath) can override it from inventory — e.g. drop ther|^/dev/dm-.*|entry.
Andrei Kvapil (kvaps)
requested a review
from Aleksei Sviridkin (lexfrei)
as a code owner
Expose the host LVM global_filter as cozystack_lvm_global_filter so
operators whose own PVs live on device-mapper devices (LVM-on-LUKS,
multipath) can override the default instead of having those PVs filtered
out. The default list is unchanged.
Relax the insertafter anchor to '^\s*devices\s*{' so the setting still
lands inside the devices{} block when lvm.conf uses leading whitespace
or compact bracing; with the stricter '^devices {' a non-match made
lineinfile append global_filter at EOF, where LVM ignores it.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Add a 'Required: Host LVM global_filter' section next to the multipath DRBD blacklist describing the silent-failure trap it prevents, a row for cozystack_lvm_global_filter in the example-playbook variables table, and a CHANGELOG note that the filter is overridable from inventory. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
The Unreleased entries had drifted into two headers — one without an RST underline (rendering as body text) and a second underlined block with a Bugfixes subsection. Merge them into a single underlined Unreleased section with flat bullets matching the released-version style, so the release workflow's Unreleased-to-version rename targets one unambiguous section. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Clarify that the lineinfile task overwrites any existing global_filter (commented or active) in lvm.conf, so operators with custom filter rules must set cozystack_lvm_global_filter to the full desired list rather than only the entries they want to add. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Add tests/test-lvm-global-filter.yml asserting the templated default renders to valid LVM syntax, the lineinfile replaces a commented global_filter in place inside the devices block, is idempotent, falls back to the insertafter anchor (including an indented devices header) when no filter line exists, and honours a cozystack_lvm_global_filter override. Wire it into the Test workflow as its own job. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Extend the global_filter test with a drift guard that loads the rhel, suse and ubuntu prepare playbooks and asserts each one's regexp, insertafter and raw line template match the canonical values exercised here. Editing the task in one playbook without the others — or without this test — now fails CI. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
After writing global_filter, query LVM (lvmconfig devices/global_filter)
and fail the play if the setting is not reported as active — for example
when lvm.conf has no devices {} section and lineinfile appended the line
at EOF, outside every block, where LVM silently ignores it. Turns that
silent no-op into an actionable failure at prep time.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Drive each lvm.conf shape (commented-in-block, no-filter, indented header, override, and no-devices-block) through the prepare playbooks' lineinfile, then ask LVM via lvmconfig + LVM_SYSTEM_DIR whether the filter is effective — asserting it is for in-block cases and is NOT for the no-devices-block case (the silent failure the post-write check catches). Install lvm2 in the Test workflow for lvmconfig. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Note in the README and CHANGELOG that the prepare playbooks verify the global_filter with lvmconfig after writing it and fail loudly when it did not take effect (e.g. an lvm.conf with no devices section). Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
…cess
Compare the list lvmconfig reports for devices/global_filter against the
configured cozystack_lvm_global_filter, not only its exit status. When
lvm.conf holds more than one global_filter line, lineinfile replaces the
last match — which may sit outside devices {} — while a stale in-block
filter keeps winning; lvmconfig then returns 0 with the old value. The
value comparison rejects that. The default list is hoisted to a single
_cozystack_lvm_default_filter fact shared by the write and the check.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Add a scenario where a stale global_filter inside devices {} plus a
second match after it make lineinfile leave the stale value effective;
assert lvmconfig reports a list that does NOT equal the configured one —
which is what the playbooks' value comparison rejects. Mirror the
single-source _cozystack_lvm_default_filter the playbooks now use.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
LGTM. The host LVM global_filter is the right fix for the LINSTOR/DRBD/zvol/loop auto-activation trap and mirrors the Talos machine-config filter. It is configurable via cozystack_lvm_global_filter, the insertafter anchor is whitespace-tolerant, and a post-write lvmconfig check fails loudly — comparing the effective value, not just exit status — if the filter did not land inside the devices {} block. Verified end-to-end against real lvmconfig across in-block, no-filter, indented, override, no-block and stale-duplicate lvm.conf shapes; the new CI job is green.
2 participants
Summary
Add an LVM
global_filterto/etc/lvm/lvm.confin the prepare playbooks so the host LVM does not scan or activate volume groups backed by LINSTOR/DRBD volumes — or located inside loop-mounted images. Mirrors theglobal_filteralready shipped in the Talos machine config and complements the existing multipathd DRBD blacklist.Note: unlike Talos (minimal, non-LVM root), the filter is applied with
lineinfileso the distro's defaultlvm.confis preserved. The filter does hide/dev/dm-*and/dev/zd*from the host LVM tooling — intended for dedicated Cozystack storage nodes.Changes
global_filter = [ "r|^/dev/drbd.*|", "r|^/dev/dm-.*|", "r|^/dev/zd.*|", "r|^/dev/loop.*|" ]in/etc/lvm/lvm.conf(vialineinfile) in theubuntu,rhelandsuseprepare playbooks, right after the multipathd DRBD blacklist.Test plan
ansible-lintpassesansible-test sanitypassesSummary by CodeRabbit
New Features
Documentation