Skip to content

Commit 72b32d5

Browse files
kvapsclaude
andcommitted
fix(lvm): in-line --config filter on every LVM invocation (upstream parity)
Adds a defensive `--config "devices { filter=['r|^/dev/drbd|','r|^/dev/zd|'] }"` to every shell-out to `lvs` / `pvs` / `vgs` / `lvcreate` / `lvextend` / `lvremove` in `pkg/storage/lvm/` and the discovery loop's `pkg/satellite/signatures.go.HasLVMSignature`. Mirrors upstream LINSTOR's LinstorVlmLayer.java defensive filter — rejects: - `/dev/drbdN` paths so LVM doesn't loop on its own LVs that are exposed back to the host via DRBD (would let LVM see the same VG twice and surface duplicate-VG warnings at best, corrupt metadata at worst); - ZFS zvol paths so a mixed-pool host doesn't accidentally let LVM scan ZFS-managed block devices. The new `lvm.ConfigFilter` constant + `lvm.Args(extra...)` helper centralise the filter — every LVM invocation now goes through `lvm.Args(...)`. The PLAN's Phase 8.2 entry is updated with a "new LVM call sites MUST use lvm.Args" rule so future code keeps the contract. Pinned indirectly via the existing storage test suite — every test that asserts an exact `lvX ...` command line was updated to include the filter prefix; wrap-substring assertions left alone since the wrap text doesn't repeat the filter. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
1 parent 9fcd368 commit 72b32d5

13 files changed

Lines changed: 199 additions & 166 deletions

PLAN.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,7 @@ The phases above closed the MVP slice and the csi-sanity REST contract. A deep a
407407
- [x] **Backing-device failure under DRBD** (2026-05-09). The events2 observer now watches for `disk:Failed` on the local replica and runs `drbdadm detach --force <rd>` so the lower disk stops getting hammered. Peers stay UpToDate, the consumer keeps doing I/O via DRBD's network path. The detach is best-effort (logged, not retried) — the next reconcile will redrive state if the storage layer comes back. The Failed observation still ships to the controller via ReportObserved, so a Status condition reflecting the diskless state is one ResourceObservation handler away. **End-to-end "pull the LV out from under DRBD"** sits on the 8.8 e2e checklist; satellite-side hook is in place.
408408
- [x] **DRBD options hierarchy** controller → resource-group → resource-definition → resource (2026-05-09). `pkg/drbd.ResolveOptions` walks the four scopes, lower wins. The resource controller reads ControllerProps via the KVEntry CRD, the parent RG via `client.Get`, the RD via the existing lookup, then folds in the resource's own props. The merged map flows through `dispatcher.ApplyOptions.EffectiveProps`; `buildDesired` splits it: DrbdOptions/* land on the satellite's drbd_options bag (the .res renderer drops them in the right `net`/`disk`/`peer-device`/`handlers` block via `pkg/drbd.SectionFor`), non-DRBD props stay on the wire-side Props map. Tests: resolver unit tests for override / partial inheritance / non-DRBD-prop pass-through; `TestApplyDRBDOptionsFromEffectiveProps` for the dispatcher wiring.
409409
- [x] **`allow-two-primaries` plumbing** (2026-05-09): the DRBD option-hierarchy now flows arbitrary `DrbdOptions/Net/...` keys (including `allow-two-primaries yes`) through the satellite into the rendered .res file's `net { }` block. Operators set the knob via `linstor c sp DrbdOptions/Net/allow-two-primaries yes` (or RG/RD/Resource scope). The first-activation auto-primary seed still picks one replica deterministically (lowest stable node-id) — that's correct: dual-primary is for the consumer's promotion (Ganesha promoter, KubeVirt live-migration controller), not for initial sync. `splitDRBDOptions` strips the `DrbdOptions/<Section>/` prefix so the .res renderer emits `allow-two-primaries yes;` verbatim. **Live-migration coordination on the controller side** (orchestrating `drbdadm primary` on the destination, then `drbdadm secondary` on the source) lives outside this scope — that's what drbd-reactor + the consumer (KubeVirt VirtualMachineInstanceMigration / Ganesha promoter) own.
410+
- [x] **LVM in-line config filter** (2026-05-10). Every shell-out to `lvs` / `pvs` / `vgs` / `lvcreate` / `lvextend` / `lvremove` (in `pkg/storage/lvm/` + `pkg/satellite/signatures.go`) now goes through `lvm.Args(...)` which prepends `--config "devices { filter=['r|^/dev/drbd|','r|^/dev/zd|'] }"`. Mirrors upstream LINSTOR's `LinstorVlmLayer.java` defensive filter — rejects DRBD device paths (so LVM doesn't loop on its own LVs exposed via /dev/drbdN) and ZFS zvols (so a mixed-pool host doesn't accidentally let LVM scan ZFS-managed devices). Centralising the filter in one helper keeps every LVM invocation in lock-step; new code that adds an LVM call MUST go through `lvm.Args(...)`.
410411

411412
### 8.3 Replica lifecycle
412413

pkg/satellite/grpc_server_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import (
3535
func TestGRPCServerApplyResources(t *testing.T) {
3636
dir := t.TempDir()
3737
fx := storage.NewFakeExec()
38-
fx.Expect("lvs --noheadings -o lv_name vg/pvc-1_00000",
38+
fx.Expect("lvs --config devices { filter=['r|^/dev/drbd|','r|^/dev/zd|'] } --noheadings -o lv_name vg/pvc-1_00000",
3939
storage.FakeResponse{Stdout: []byte("")})
4040

4141
thin := lvm.NewThin(lvm.ThinConfig{VolumeGroup: "vg", ThinPool: "tp"}, fx)
@@ -310,12 +310,12 @@ func TestGRPCServerDeleteResourceProviderError(t *testing.T) {
310310
fx := storage.NewFakeExec()
311311

312312
// First Apply succeeds (registers resource→pool map + creates LV).
313-
fx.Expect("lvs --noheadings -o lv_name vg/pvc-del-busy_00000",
313+
fx.Expect("lvs --config devices { filter=['r|^/dev/drbd|','r|^/dev/zd|'] } --noheadings -o lv_name vg/pvc-del-busy_00000",
314314
storage.FakeResponse{Stdout: []byte("")})
315315
// During DeleteResource: lvExists → "yes", lvremove fails (busy).
316-
fx.Expect("lvs --noheadings -o lv_name vg/pvc-del-busy_00000",
316+
fx.Expect("lvs --config devices { filter=['r|^/dev/drbd|','r|^/dev/zd|'] } --noheadings -o lv_name vg/pvc-del-busy_00000",
317317
storage.FakeResponse{Stdout: []byte("pvc-del-busy_00000\n")})
318-
fx.Expect("lvremove --force vg/pvc-del-busy_00000",
318+
fx.Expect("lvremove --config devices { filter=['r|^/dev/drbd|','r|^/dev/zd|'] } --force vg/pvc-del-busy_00000",
319319
storage.FakeResponse{Err: errLVRemoveEBUSY})
320320

321321
thin := lvm.NewThin(lvm.ThinConfig{VolumeGroup: "vg", ThinPool: "tp"}, fx)

0 commit comments

Comments
 (0)