Skip to content

Revert "fix: preserve original source lvolID through failback cutover" (#1276) - #1340

Merged
wmousa merged 1 commit into
R26.3from
revert-pr-1276
Sep 10, 2026
Merged

Revert "fix: preserve original source lvolID through failback cutover" (#1276)#1340
wmousa merged 1 commit into
R26.3from
revert-pr-1276

Conversation

@wmousa

@wmousa wmousa commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Reverts #1276 (789db7899) on R26.3.

Why

#1276 made every namespaced clone fail. sbctl snapshot clone died with:

Failed to add bdev to subsystem at requested nsid=1: nsid occupied and eviction
did not clear it [node 5b3af03f wanted nsid=1 max_namespaces=32
holds=[(1, '18d8dba1-9a11-452f-92c5-7fa6e8376b82')]]

Two hunks in add_lvol_on_node (simplyblock_core/controllers/lvol_controller.py) are responsible:

-        requested_nsid = primary_nsid
+        requested_nsid = primary_nsid or (lvol.ns_id or None)
...
-                if primary_nsid is not None:
+                if requested_nsid is not None:
                     return _fail_after_bdev(... "nsid occupied and eviction did not clear it")

LVol.ns_id has a model default of 1 — a legitimate nsid. snapshot_controller.clone() builds its record by hand and never resets it (the ordinary create path does, add_lvol_ha). Before #1276 only an explicit primary_nsid from a migration/fail-over caller could pin an nsid, so that default was inert. After #1276 the record's default became a request: every clone demanded nsid=1, which in a shared subsystem belongs to the subsystem's first lvol. The widened guard then refused to re-claim another subsystem, turning a recoverable placement into a hard failure.

--namespaced '' was no workaround: both CLI clone handlers pass args.namespaced as the 4th positional of clone(), which is pvc_name, so namespaced kept its default True (fixed separately).

Verification

  • ruff check — clean
  • mypy simplyblock_web simplyblock_cli simplyblock_core — only the 2 pre-existing tasks_runner_node_add.py annotation errors (that file is not in fix: preserve original source lvolID through failback cutover by swap… #1276, so they predate this revert)
  • tests/unit — 1874 passed, 7 skipped, 23 subtests passed
  • Revert applied with no conflicts (78 files, 848 insertions, 10559 deletions)

What this re-opens

This is a full revert, so the other fixes #1276 carried go with it. Before merging, please confirm these are acceptable to lose for now, or plan a narrower re-land:

  • _evict_stale_namespace returns to indexing subsystem_get(...) (a dict) with [0] — the eviction that silently never ran
  • fail-over's existing-copy guard returns to nqn-only matching — namespaced siblings can be silently skipped on a DR fail-over
  • namespaced siblings can again scatter across different target primaries
  • jc_set_dual_node is removed — a graceful sn shutdown on a 2-node cluster can abort the surviving node
  • soak cases 7-9 and their test infrastructure are removed

Follow-up

The clone failure also has a one-line fix that is independent of this revert (snapshot_controller.py is not touched by #1276): reset lvol.ns_id = 0 in clone(), matching add_lvol_ha. Worth landing regardless so the model default can never be requested again.

🤖 Generated with Claude Code

@wmousa
wmousa merged commit cb169ef into R26.3 Sep 10, 2026
7 checks passed
@wmousa
wmousa deleted the revert-pr-1276 branch September 10, 2026 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants