From 84156297c7de7820d6b3aa505a8dc60ce2ba33c6 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Wed, 24 Jun 2026 09:40:29 +0330 Subject: [PATCH 01/49] enabling the hublvol detch on cleanup source --- .../services/tasks_runner_lvol_migration.py | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index 55af8b98f..866e12346 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -1826,15 +1826,17 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): # bdev_nvme_detach_controller can still reach the hub bdev. # Step 8: delete source primary NVMe-oF subsystem. # Then: delete source lvol bdev. - # TODO: reenable hub detach after migration — currently disabled to keep hub controller attached for debugging. - # TEMP: keep hub controller attached after migration - # hub_ctrl_name = ctx.get('hub_ctrl_name') - # if hub_ctrl_name: - # try: - # src_rpc.bdev_nvme_detach_controller(hub_ctrl_name) - # logger.info(f"Step 7: deferred hub controller detach: {hub_ctrl_name}") - # except Exception as e: - # logger.warning(f"Deferred hub detach {hub_ctrl_name} (non-fatal): {e}") + hub_ctrl_name = ctx.get('hub_ctrl_name') + if hub_ctrl_name: + try: + src_rpc.bdev_nvme_detach_controller(hub_ctrl_name) + logger.info(f"Step 7: deferred hub controller detach: {hub_ctrl_name}") + # Give SPDK time to complete the NVMe disconnect handshake before the + # next migration attempts to reattach. Without this the controller + # stays in 'deleting' state and the attach fails. + time.sleep(10) + except Exception as e: + logger.warning(f"Deferred hub detach {hub_ctrl_name} (non-fatal): {e}") lvol = None try: lvol = db.get_lvol_by_id(migration.lvol_id) From 5d53b477e951eeea7f55a14c3da84cbcec4a1419 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Wed, 24 Jun 2026 13:01:48 +0330 Subject: [PATCH 02/49] adding a hublvol controller to control detch behaviour --- .../services/hub_controller_manager.py | 337 ++++++++++++++++++ .../services/tasks_runner_lvol_migration.py | 161 ++------- 2 files changed, 369 insertions(+), 129 deletions(-) create mode 100644 simplyblock_core/services/hub_controller_manager.py diff --git a/simplyblock_core/services/hub_controller_manager.py b/simplyblock_core/services/hub_controller_manager.py new file mode 100644 index 000000000..4459903ac --- /dev/null +++ b/simplyblock_core/services/hub_controller_manager.py @@ -0,0 +1,337 @@ +# coding=utf-8 +""" +hub_controller_manager.py — lifecycle manager for migration hub NVMe-oF controllers. + +Goal +---- +A migration hub controller (mighub*n1 bdev on the source SPDK) connects source to +target's dedicated transfer-hub subsystem. Two problems arise when controllers are +detached naively: + + 1. Concurrent migrations: migration A finishes and detaches the controller while + migration B (to the same target) is still running and still needs it. + + 2. Deleting-state race: bdev_nvme_detach_controller triggers an async NVMe + disconnect handshake. If a new attach arrives before the handshake completes + the controller is in 'deleting' state and the attach fails. + +This module solves both: + + • Reference counting — controller is only detached when ALL migrations using it + have finished (refcount == 0) AND it has been idle for IDLE_TIMEOUT seconds. + + • Cooldown enforcement — a minimum DETACH_COOLDOWN gap is maintained between + the moment a detach RPC is issued and any subsequent re-attach attempt. If + acquire() is called during the cooldown window it returns a transient error so + the task runner retries on the next loop iteration (~3 s) until the gap passes. + +Public API (called by tasks_runner_lvol_migration) +-------------------------------------------------- + hub_manager.acquire(src_node_id, src_rpc, tgt_node, trtype) + → attach (or reuse) the controller, increment refcount + → returns (ctrl_name, hub_bdev, error) + + hub_manager.release(src_node_id, tgt_node_id) + → decrement refcount, start idle timer — does NOT detach + + hub_manager.detach_now(src_node_id, tgt_node_id, src_rpc=None) + → immediate detach (failure / cancel path), records detach timestamp +""" + +import threading +import time + +from simplyblock_core import utils + +logger = utils.get_logger(__name__) + + +class _HubEntry: + __slots__ = ('ctrl_name', 'hub_bdev', 'src_rpc', 'tgt_node', + 'refcount', 'last_release') + + def __init__(self, ctrl_name: str, hub_bdev: str, src_rpc, tgt_node): + self.ctrl_name = ctrl_name + self.hub_bdev = hub_bdev # e.g. "mighubXXXXXXXXn1" + self.src_rpc = src_rpc # refreshed on every acquire — used by GC + self.tgt_node = tgt_node + self.refcount = 1 + self.last_release = 0.0 # monotonic ts when refcount last hit 0 + + +class HubControllerManager: + """ + Thread-safe lifecycle manager for migration hub NVMe-oF controllers. + + One instance (``hub_manager``) is created at module level and shared by + the entire migration task runner process. + """ + + # Minimum seconds between issuing a detach RPC and allowing any re-attach. + # Must be long enough for the NVMe TCP disconnect handshake to complete. + DETACH_COOLDOWN = 10 + + # Seconds with refcount == 0 before the GC triggers a detach. + # Setting this well above DETACH_COOLDOWN means back-to-back migrations + # typically reuse the existing controller without going through detach+reattach. + IDLE_TIMEOUT = 60 + + # GC sweep period. + GC_INTERVAL = 15 + + def __init__(self): + self._lock = threading.Lock() + self._entries: dict = {} # (src_node_id, tgt_node_id) → _HubEntry + self._detach_ts: dict = {} # (src_node_id, tgt_node_id) → monotonic ts of last detach + self._gc_thread: threading.Thread | None = None + + # ------------------------------------------------------------------ + # Public API + # ------------------------------------------------------------------ + + def acquire(self, src_node_id: str, src_rpc, tgt_node, trtype: str): + """ + Ensure the hub controller from *src_node_id* to *tgt_node* is attached. + + Returns (ctrl_name, hub_bdev, error). On transient cooldown the error + string starts with "HUB_COOLDOWN:" so callers can log it distinctly. + """ + key = (src_node_id, tgt_node.get_id()) + + with self._lock: + entry = self._entries.get(key) + if entry is not None: + try: + if src_rpc.get_bdevs(entry.hub_bdev): + entry.refcount += 1 + entry.src_rpc = src_rpc + logger.info( + f"[HubMgr] reusing {entry.ctrl_name} " + f"src={src_node_id[:8]} tgt={tgt_node.get_id()[:8]} " + f"refcount={entry.refcount}" + ) + return entry.ctrl_name, entry.hub_bdev, None + except Exception: + pass + logger.info(f"[HubMgr] stale entry for {entry.ctrl_name}; re-attaching") + del self._entries[key] + + # Enforce cooldown before any fresh attach + cooldown_remaining = self._cooldown_remaining(key) + if cooldown_remaining > 0: + msg = ( + f"HUB_COOLDOWN: {cooldown_remaining:.1f}s remaining after last detach " + f"(src={src_node_id[:8]} tgt={tgt_node.get_id()[:8]})" + ) + logger.info(f"[HubMgr] {msg}") + return None, None, msg + + # Attach outside the lock so we don't block the GC thread + ctrl_name, hub_bdev, err = self._attach(src_rpc, tgt_node, trtype) + if err: + return None, None, err + + with self._lock: + # Another thread may have attached concurrently — prefer the winner + existing = self._entries.get(key) + if existing is not None: + try: + if src_rpc.get_bdevs(existing.hub_bdev): + existing.refcount += 1 + existing.src_rpc = src_rpc + try: + src_rpc.bdev_nvme_detach_controller(ctrl_name) + except Exception: + pass + return existing.ctrl_name, existing.hub_bdev, None + except Exception: + pass + + entry = _HubEntry(ctrl_name, hub_bdev, src_rpc, tgt_node) + self._entries[key] = entry + self._ensure_gc_running() + + logger.info( + f"[HubMgr] attached {ctrl_name} " + f"src={src_node_id[:8]} tgt={tgt_node.get_id()[:8]}" + ) + return ctrl_name, hub_bdev, None + + def release(self, src_node_id: str, tgt_node_id: str): + """ + Signal that one migration using this hub has finished. + + Decrements refcount and starts the idle timer. Does NOT detach — + the GC thread handles detach after IDLE_TIMEOUT seconds. + """ + key = (src_node_id, tgt_node_id) + with self._lock: + entry = self._entries.get(key) + if entry is None: + return + entry.refcount = max(0, entry.refcount - 1) + if entry.refcount == 0: + entry.last_release = time.monotonic() + logger.info( + f"[HubMgr] released {entry.ctrl_name} " + f"src={src_node_id[:8]} tgt={tgt_node_id[:8]} — " + f"idle timer started (detach in ~{self.IDLE_TIMEOUT}s if not reused)" + ) + + def detach_now(self, src_node_id: str, tgt_node_id: str, src_rpc=None): + """ + Immediately detach the hub controller (failure / cancel path). + + Records the detach timestamp so subsequent acquire() calls observe + the DETACH_COOLDOWN gap. + """ + key = (src_node_id, tgt_node_id) + with self._lock: + entry = self._entries.pop(key, None) + if entry is None: + return + rpc = src_rpc or entry.src_rpc + self._do_detach(key, entry, rpc, reason="detach_now") + + # ------------------------------------------------------------------ + # Internal helpers + # ------------------------------------------------------------------ + + def _cooldown_remaining(self, key) -> float: + """Return seconds remaining in the post-detach cooldown (0 if none).""" + ts = self._detach_ts.get(key, 0) + if ts == 0: + return 0.0 + elapsed = time.monotonic() - ts + return max(0.0, self.DETACH_COOLDOWN - elapsed) + + def _do_detach(self, key, entry: _HubEntry, rpc, reason: str): + """Issue the detach RPC and record the detach timestamp.""" + try: + if rpc.get_bdevs(entry.hub_bdev): + rpc.bdev_nvme_detach_controller(entry.ctrl_name) + logger.info(f"[HubMgr] detached {entry.ctrl_name} ({reason})") + else: + logger.info( + f"[HubMgr] {entry.ctrl_name} bdev already gone ({reason}); skipping detach" + ) + except Exception as e: + logger.warning(f"[HubMgr] detach {entry.ctrl_name} ({reason}) non-fatal: {e}") + finally: + # Record timestamp regardless of whether the RPC succeeded — the + # controller state is unknown so enforce the cooldown either way. + with self._lock: + self._detach_ts[key] = time.monotonic() + + @staticmethod + def _attach(src_rpc, tgt_node, trtype: str): + """ + Attach a hub controller on *src_rpc* to *tgt_node*'s migration hub subsystem. + + Returns (ctrl_name, hub_bdev, error). + """ + if tgt_node.transfer_hublvol is None or not tgt_node.transfer_hublvol.bdev_name: + tgt_node.create_transfer_hublvol() + + # Already attached (crash recovery or concurrent acquire) + if src_rpc.get_bdevs(tgt_node.transfer_hublvol.get_remote_bdev_name()): + return ( + tgt_node.transfer_hublvol.bdev_name, + tgt_node.transfer_hublvol.get_remote_bdev_name(), + None, + ) + + attached = False + for iface in tgt_node.data_nics: + ip = iface.ip4_address + if tgt_node.active_rdma: + if iface.trtype != "RDMA": + continue + nic_trtype = "RDMA" + else: + if iface.trtype != "TCP": + continue + nic_trtype = "TCP" + + ret = src_rpc.bdev_nvme_attach_controller( + tgt_node.transfer_hublvol.bdev_name, + tgt_node.transfer_hublvol.nqn, + ip, + tgt_node.transfer_hublvol.nvmf_port, + nic_trtype, + ) + if not ret: + if src_rpc.bdev_nvme_controller_list(tgt_node.transfer_hublvol.bdev_name): + logger.info("[HubMgr] zombie controller found; detaching and reattaching") + src_rpc.bdev_nvme_detach_controller(tgt_node.transfer_hublvol.bdev_name) + try: + tgt_node.create_transfer_hublvol() + except Exception as e: + logger.warning(f"[HubMgr] hub subsystem re-create (non-fatal): {e}") + ret = src_rpc.bdev_nvme_attach_controller( + tgt_node.transfer_hublvol.bdev_name, + tgt_node.transfer_hublvol.nqn, + ip, + tgt_node.transfer_hublvol.nvmf_port, + nic_trtype, + ) + if not ret: + return None, None, ( + f"Failed to attach migration hub controller to {tgt_node.get_id()}" + ) + attached = True + break + + if not attached: + return None, None, f"No suitable NIC found on target node {tgt_node.get_id()}" + + return ( + tgt_node.transfer_hublvol.bdev_name, + tgt_node.transfer_hublvol.get_remote_bdev_name(), + None, + ) + + def _ensure_gc_running(self): + """Start the GC daemon thread if not already running. Must be called under self._lock.""" + if self._gc_thread is None or not self._gc_thread.is_alive(): + self._gc_thread = threading.Thread( + target=self._gc_loop, name="HubMgrGC", daemon=True + ) + self._gc_thread.start() + + def _gc_loop(self): + logger.info("[HubMgr] GC thread started") + while True: + time.sleep(self.GC_INTERVAL) + now = time.monotonic() + + with self._lock: + to_evict = [ + key + for key, e in self._entries.items() + if e.refcount == 0 and (now - e.last_release) >= self.IDLE_TIMEOUT + ] + evicted = [(key, self._entries.pop(key)) for key in to_evict] + + # Prune stale detach timestamps older than DETACH_COOLDOWN — no + # longer needed once the cooldown window has passed. + stale = [k for k, ts in self._detach_ts.items() + if (now - ts) > self.DETACH_COOLDOWN * 2] + for k in stale: + del self._detach_ts[k] + + if not self._entries: + logger.info("[HubMgr] GC thread exiting (no active entries)") + self._gc_thread = None + break # restarted on next acquire() + + for key, entry in evicted: + logger.info( + f"[HubMgr] GC evicting {entry.ctrl_name} " + f"(idle {now - entry.last_release:.0f}s >= {self.IDLE_TIMEOUT}s)" + ) + self._do_detach(key, entry, entry.src_rpc, "gc-idle") + + +# Module-level singleton — imported and used by tasks_runner_lvol_migration +hub_manager = HubControllerManager() diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index 866e12346..f9fead742 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -97,6 +97,7 @@ from simplyblock_core.models.storage_node import StorageNode from simplyblock_core.models.snapshot import SnapShot from simplyblock_core.rpc_client import RPCException, RPCClient +from simplyblock_core.services.hub_controller_manager import hub_manager logger = utils.get_logger(__name__) db = db_mod.DBController() @@ -134,74 +135,6 @@ def _get_migration_nic(node): return trtype, node.mgmt_ip -def _ensure_hub_attached(src_rpc, tgt_rpc, tgt_node, trtype, target_ip): - """ - Ensure a migration-specific hub NVMe-oF controller is attached on source. - - Creates a dedicated hublvol (named "mighub") on the target lvstore - and exposes it via its own NVMe-oF subsystem. Using a separate bdev and NQN - avoids two Case B (src-is-tgt-secondary) problems: - • TRID conflict: the migration NQN differs from the node's regular hub NQN, - so SPDK does not reject a second controller to the same (addr, NQN). - • Exclusive claim: the new hublvol is unclaimed at creation, so - bdev_lvol_transfer can open it without hitting the NVMe-oF Target's - exclusive_write claim on the permanent hublvol. - - The hub controller is shared across all snapshot transfers and the final - migration — created once during SNAP_COPY, reused by PHASE_LVOL_MIGRATE, - detached in CLEANUP_SOURCE. All names/NQNs are deterministic so crash - recovery works without additional FDB state. - - Returns (ctrl_name, hub_bdev, error_string|None). - """ - - if tgt_node.transfer_hublvol is None or not tgt_node.transfer_hublvol.bdev_name: - tgt_node.create_transfer_hublvol() - - # Already attached (prior iteration or crash recovery). - if src_rpc.get_bdevs(tgt_node.transfer_hublvol.get_remote_bdev_name()): - return tgt_node.transfer_hublvol.bdev_name, tgt_node.transfer_hublvol.get_remote_bdev_name(), None - - for iface in tgt_node.data_nics: - ip = iface.ip4_address - if tgt_node.active_rdma: - if iface.trtype != "RDMA": - logger.debug("Skipping non-RDMA iface %s (active_rdma=True)", ip) - continue - trtype = "RDMA" - else: - if iface.trtype != "TCP": - logger.debug("Skipping non-TCP iface %s (active_tcp=True)", ip) - continue - trtype = "TCP" - - # Attach NVMe controller on source to the migration subsystem. - ret = src_rpc.bdev_nvme_attach_controller( - tgt_node.transfer_hublvol.bdev_name, tgt_node.transfer_hublvol.nqn, - ip, tgt_node.transfer_hublvol.nvmf_port, trtype) - if not ret: - # Attach can fail with EEXIST if a prior crashed attempt attached the controller - # but the namespace wasn't in the subsystem yet, or if the target SPDK restarted - # between migrations dropping the subsystem while the source controller persisted. - # Detach the zombie, re-ensure the hub subsystem is healthy on the target (idempotent), - # then retry once so the controller reconnects to a populated subsystem. - if src_rpc.bdev_nvme_controller_list(tgt_node.transfer_hublvol.bdev_name): - logger.info( - "_ensure_hub_attached: zombie mighub controller found (no bdev); " - "detaching and reattaching" - ) - src_rpc.bdev_nvme_detach_controller(tgt_node.transfer_hublvol.bdev_name) - try: - tgt_node.create_transfer_hublvol() - except Exception as e: - logger.warning(f"_ensure_hub_attached: hub subsystem re-create (non-fatal): {e}") - ret = src_rpc.bdev_nvme_attach_controller( - tgt_node.transfer_hublvol.bdev_name, tgt_node.transfer_hublvol.nqn, - ip, tgt_node.transfer_hublvol.nvmf_port, trtype) - if not ret: - return None, None, f"Failed to attach migration hub controller to {tgt_node.get_id()}" - return tgt_node.transfer_hublvol.bdev_name, tgt_node.transfer_hublvol.get_remote_bdev_name(), None - _MIGRATION_BDEV_SUFFIX = constants.LVOL_MIG_BDEV_SUFFIX @@ -461,39 +394,22 @@ def _log_spdk_bdev_size(rpc, composite_name, label): return None -def _delete_bdev_blocking(bdev_name, primary_rpc, secondary_rpc=None, max_polls=120): +def _delete_bdev_blocking(bdev_name, primary_rpc, secondary_rpc=None): """ - Full 3-step async-delete sequence for use in synchronous error-recovery paths. - Mirrors the control-plane pattern in storage_node_ops.safe_delete_bdev(): + Delete a bdev cleanly in a single RPC call. - 1. delete_lvol(sync=False) on primary – start async background deletion - 2. poll bdev_lvol_get_lvol_delete_status until complete (0) or not-found (2) - 3. delete_lvol(sync=True) on primary – sync finalize / confirm removal - delete_lvol(sync=True) on secondary – sync finalize (best-effort) - - Blocks for up to max_polls × 0.25 s. Use only in error-recovery paths where - a bdev was just created and must be cleaned up before returning. + special_delete=True handles blob reference cleanup atomically — no + async-start/poll/finalize sequence needed. secondary_rpc deletion is + best-effort (non-fatal if the secondary is unreachable). """ - # Step 1: start async deletion - ret, _ = primary_rpc.delete_lvol(bdev_name) + ret, _ = primary_rpc.delete_lvol(bdev_name, special_delete=True) if not ret: - logger.warning(f"delete bdev {bdev_name}: async start failed (continuing)") - - # Step 2: poll - for _ in range(max_polls): - status = primary_rpc.bdev_lvol_get_lvol_delete_status(bdev_name) - if status in (0, 2): - break - if status == 1: - time.sleep(0.25) - else: - logger.warning(f"delete bdev {bdev_name}: unexpected status {status}") - break - - # Step 3: sync finalize - primary_rpc.delete_lvol(bdev_name, del_async=True) + logger.warning(f"delete bdev {bdev_name}: special_delete failed (continuing)") if secondary_rpc: - secondary_rpc.delete_lvol(bdev_name, del_async=True) + try: + secondary_rpc.delete_lvol(bdev_name, special_delete=True) + except Exception as e: + logger.warning(f"delete bdev {bdev_name} on secondary (non-fatal): {e}") # --------------------------------------------------------------------------- @@ -699,7 +615,7 @@ def _cleanup_final_migration(src_rpc, ctx, tgt_rpc=None, rollback_target=False): def _setup_snap_transfer(snap, snap_index, migration, src_node, tgt_node, - src_rpc, tgt_rpc, trtype, target_ip, + src_rpc, tgt_rpc, trtype, tgt_sec=None, sec_rpc=None, lvol_size_mib=None): """ Prepare a single snapshot for async transfer: @@ -778,10 +694,10 @@ def _cleanup(): return None, f"Could not get map_id for snap {snap_uuid} on target" # Step 5: ensure hub controller is attached on source (shared across all - # snapshot transfers; created once, reused by PHASE_LVOL_MIGRATE, detached - # in CLEANUP_SOURCE). - _, hub_bdev, hub_err = _ensure_hub_attached( - src_rpc, tgt_rpc, tgt_node, trtype, target_ip) + # snapshot transfers; created once, reused by PHASE_LVOL_MIGRATE, released + # in CLEANUP_SOURCE and detached lazily by HubControllerManager). + _, hub_bdev, hub_err = hub_manager.acquire( + src_node.get_id(), src_rpc, tgt_node, trtype) if hub_err: _cleanup() return None, hub_err @@ -913,7 +829,7 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): Returns (done: bool, suspend: bool, error: str|None). """ plan = migration.snap_migration_plan - trtype, target_ip = _get_migration_nic(tgt_node) + trtype, _ = _get_migration_nic(tgt_node) ctx = migration.transfer_context or {} # Snap bdevs on TGT must cover the full logical address range of the lvol, @@ -1001,7 +917,7 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): t, err = _setup_snap_transfer( snap, snap_index, migration, src_node, tgt_node, - src_rpc, tgt_rpc, trtype, target_ip, + src_rpc, tgt_rpc, trtype, tgt_sec=tgt_sec, sec_rpc=sec_rpc, lvol_size_mib=_snap_lvol_size_mib) if t is None: @@ -1187,7 +1103,7 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): t, err = _setup_snap_transfer( snap, snap_index, migration, src_node, tgt_node, - src_rpc, tgt_rpc, trtype, target_ip, + src_rpc, tgt_rpc, trtype, tgt_sec=tgt_sec, sec_rpc=sec_rpc, lvol_size_mib=_snap_lvol_size_mib) if t is None: @@ -1313,7 +1229,7 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): except KeyError as e: return False, True, str(e) - trtype, target_ip = _get_migration_nic(tgt_node) + trtype, _ = _get_migration_nic(tgt_node) src_lvol_composite = f"{src_node.lvstore}/{lvol.lvol_bdev}" tgt_lvol_bdev = lvol.lvol_bdev + _MIGRATION_BDEV_SUFFIX tgt_lvol_composite = f"{tgt_node.lvstore}/{tgt_lvol_bdev}" @@ -1397,8 +1313,8 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): # a clean primary-port subsystem (min_cntlid=2000). # Step 3: connect source to target migration hub lvol - ctrl_name, hub_bdev, hub_err = _ensure_hub_attached( - src_rpc, tgt_rpc, tgt_node, trtype, target_ip) + ctrl_name, hub_bdev, hub_err = hub_manager.acquire( + src_node.get_id(), src_rpc, tgt_node, trtype) if hub_err: # Do NOT delete the target bdev on hub error — it is unrelated to # the hub connection and deleting it forces a recreate on retry, @@ -1826,17 +1742,12 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): # bdev_nvme_detach_controller can still reach the hub bdev. # Step 8: delete source primary NVMe-oF subsystem. # Then: delete source lvol bdev. - hub_ctrl_name = ctx.get('hub_ctrl_name') - if hub_ctrl_name: - try: - src_rpc.bdev_nvme_detach_controller(hub_ctrl_name) - logger.info(f"Step 7: deferred hub controller detach: {hub_ctrl_name}") - # Give SPDK time to complete the NVMe disconnect handshake before the - # next migration attempts to reattach. Without this the controller - # stays in 'deleting' state and the attach fails. - time.sleep(10) - except Exception as e: - logger.warning(f"Deferred hub detach {hub_ctrl_name} (non-fatal): {e}") + # Step 7: release the hub controller back to the manager. + # HubControllerManager keeps it alive for IDLE_TIMEOUT seconds so that a + # back-to-back migration to the same target can reuse it without hitting + # the 'deleting' state race that an immediate detach would cause. + hub_manager.release(src_node.get_id(), tgt_node.get_id()) + logger.info(f"Step 7: hub controller released to idle manager (src={src_node.get_id()[:8]} tgt={tgt_node.get_id()[:8]})") lvol = None try: lvol = db.get_lvol_by_id(migration.lvol_id) @@ -1908,17 +1819,9 @@ def _handle_cleanup_target(migration, tgt_node, tgt_rpc, src_rpc=None): Returns (done: bool, suspend: bool, error: str|None). """ - # Detach the shared hub controller on the source (best-effort). - # The hub is attached once during SNAP_COPY and must be torn down on any - # failure/cancel path; if the source is unreachable this is non-fatal. - if src_rpc is not None: - hub_ctrl_name = tgt_node.transfer_hublvol.bdev_name - try: - if src_rpc.get_bdevs(f"{hub_ctrl_name}n1"): - src_rpc.bdev_nvme_detach_controller(hub_ctrl_name) - logger.info(f"cleanup_target: detached hub controller {hub_ctrl_name}") - except Exception as e: - logger.warning(f"cleanup_target: hub detach {hub_ctrl_name} (non-fatal): {e}") + # Immediately detach the hub controller on failure/cancel — don't leave it + # connected to a target whose snapshots we're about to roll back. + hub_manager.detach_now(migration.source_node_id, tgt_node.get_id(), src_rpc=src_rpc) ctx = migration.transfer_context or {} tgt_sec_rpc = _get_secondary_rpc(tgt_node) From 741d0561cbd48a4471624b52068881dabf44de45 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Thu, 25 Jun 2026 14:39:56 +0330 Subject: [PATCH 03/49] fixing any issue with back to back migrations --- .../services/hub_controller_manager.py | 82 ++++++++----------- .../services/tasks_runner_lvol_migration.py | 44 ++++------ 2 files changed, 54 insertions(+), 72 deletions(-) diff --git a/simplyblock_core/services/hub_controller_manager.py b/simplyblock_core/services/hub_controller_manager.py index 4459903ac..c0184dbcc 100644 --- a/simplyblock_core/services/hub_controller_manager.py +++ b/simplyblock_core/services/hub_controller_manager.py @@ -17,8 +17,9 @@ This module solves both: - • Reference counting — controller is only detached when ALL migrations using it - have finished (refcount == 0) AND it has been idle for IDLE_TIMEOUT seconds. + • Activity-based lifetime — the controller stays alive as long as any migration + calls acquire() within IDLE_TIMEOUT seconds. No reference counting needed; + each acquire() simply refreshes the last-used timestamp. • Cooldown enforcement — a minimum DETACH_COOLDOWN gap is maintained between the moment a detach RPC is issued and any subsequent re-attach attempt. If @@ -28,11 +29,11 @@ Public API (called by tasks_runner_lvol_migration) -------------------------------------------------- hub_manager.acquire(src_node_id, src_rpc, tgt_node, trtype) - → attach (or reuse) the controller, increment refcount + → attach (or reuse) the controller, refresh the idle timer → returns (ctrl_name, hub_bdev, error) hub_manager.release(src_node_id, tgt_node_id) - → decrement refcount, start idle timer — does NOT detach + → no-op kept for call-site compatibility; idle timer is driven by acquire() hub_manager.detach_now(src_node_id, tgt_node_id, src_rpc=None) → immediate detach (failure / cancel path), records detach timestamp @@ -47,16 +48,14 @@ class _HubEntry: - __slots__ = ('ctrl_name', 'hub_bdev', 'src_rpc', 'tgt_node', - 'refcount', 'last_release') + __slots__ = ('ctrl_name', 'hub_bdev', 'src_rpc', 'tgt_node', 'last_used') def __init__(self, ctrl_name: str, hub_bdev: str, src_rpc, tgt_node): - self.ctrl_name = ctrl_name - self.hub_bdev = hub_bdev # e.g. "mighubXXXXXXXXn1" - self.src_rpc = src_rpc # refreshed on every acquire — used by GC - self.tgt_node = tgt_node - self.refcount = 1 - self.last_release = 0.0 # monotonic ts when refcount last hit 0 + self.ctrl_name = ctrl_name + self.hub_bdev = hub_bdev # e.g. "mighubXXXXXXXXn1" + self.src_rpc = src_rpc # refreshed on every acquire — used by GC + self.tgt_node = tgt_node + self.last_used = time.monotonic() class HubControllerManager: @@ -71,13 +70,13 @@ class HubControllerManager: # Must be long enough for the NVMe TCP disconnect handshake to complete. DETACH_COOLDOWN = 10 - # Seconds with refcount == 0 before the GC triggers a detach. - # Setting this well above DETACH_COOLDOWN means back-to-back migrations - # typically reuse the existing controller without going through detach+reattach. - IDLE_TIMEOUT = 60 + # Seconds since the last acquire() before the GC triggers a detach. + # Refreshed on every acquire() so concurrent migrations naturally keep + # the controller alive without any reference counting. + IDLE_TIMEOUT = 300 # 5 minutes # GC sweep period. - GC_INTERVAL = 15 + GC_INTERVAL = 30 def __init__(self): self._lock = threading.Lock() @@ -93,6 +92,9 @@ def acquire(self, src_node_id: str, src_rpc, tgt_node, trtype: str): """ Ensure the hub controller from *src_node_id* to *tgt_node* is attached. + Refreshes the idle timer so the controller stays alive as long as + migrations are actively using it. + Returns (ctrl_name, hub_bdev, error). On transient cooldown the error string starts with "HUB_COOLDOWN:" so callers can log it distinctly. """ @@ -103,12 +105,11 @@ def acquire(self, src_node_id: str, src_rpc, tgt_node, trtype: str): if entry is not None: try: if src_rpc.get_bdevs(entry.hub_bdev): - entry.refcount += 1 + entry.last_used = time.monotonic() entry.src_rpc = src_rpc logger.info( f"[HubMgr] reusing {entry.ctrl_name} " - f"src={src_node_id[:8]} tgt={tgt_node.get_id()[:8]} " - f"refcount={entry.refcount}" + f"src={src_node_id[:8]} tgt={tgt_node.get_id()[:8]}" ) return entry.ctrl_name, entry.hub_bdev, None except Exception: @@ -137,7 +138,7 @@ def acquire(self, src_node_id: str, src_rpc, tgt_node, trtype: str): if existing is not None: try: if src_rpc.get_bdevs(existing.hub_bdev): - existing.refcount += 1 + existing.last_used = time.monotonic() existing.src_rpc = src_rpc try: src_rpc.bdev_nvme_detach_controller(ctrl_name) @@ -157,26 +158,13 @@ def acquire(self, src_node_id: str, src_rpc, tgt_node, trtype: str): ) return ctrl_name, hub_bdev, None - def release(self, src_node_id: str, tgt_node_id: str): + def release(self, src_node_id: str, tgt_node_id: str): # noqa: ARG002 """ - Signal that one migration using this hub has finished. + No-op — kept for call-site compatibility. - Decrements refcount and starts the idle timer. Does NOT detach — - the GC thread handles detach after IDLE_TIMEOUT seconds. + The idle timer is driven entirely by acquire(); explicit release calls + are not needed and were a source of refcount imbalance bugs. """ - key = (src_node_id, tgt_node_id) - with self._lock: - entry = self._entries.get(key) - if entry is None: - return - entry.refcount = max(0, entry.refcount - 1) - if entry.refcount == 0: - entry.last_release = time.monotonic() - logger.info( - f"[HubMgr] released {entry.ctrl_name} " - f"src={src_node_id[:8]} tgt={tgt_node_id[:8]} — " - f"idle timer started (detach in ~{self.IDLE_TIMEOUT}s if not reused)" - ) def detach_now(self, src_node_id: str, tgt_node_id: str, src_rpc=None): """ @@ -309,29 +297,31 @@ def _gc_loop(self): to_evict = [ key for key, e in self._entries.items() - if e.refcount == 0 and (now - e.last_release) >= self.IDLE_TIMEOUT + if (now - e.last_used) >= self.IDLE_TIMEOUT ] evicted = [(key, self._entries.pop(key)) for key in to_evict] - # Prune stale detach timestamps older than DETACH_COOLDOWN — no - # longer needed once the cooldown window has passed. + # Prune stale detach timestamps older than DETACH_COOLDOWN. stale = [k for k, ts in self._detach_ts.items() if (now - ts) > self.DETACH_COOLDOWN * 2] for k in stale: del self._detach_ts[k] - if not self._entries: - logger.info("[HubMgr] GC thread exiting (no active entries)") - self._gc_thread = None - break # restarted on next acquire() + should_exit = not self._entries for key, entry in evicted: logger.info( f"[HubMgr] GC evicting {entry.ctrl_name} " - f"(idle {now - entry.last_release:.0f}s >= {self.IDLE_TIMEOUT}s)" + f"(idle {now - entry.last_used:.0f}s >= {self.IDLE_TIMEOUT}s)" ) self._do_detach(key, entry, entry.src_rpc, "gc-idle") + if should_exit: + with self._lock: + self._gc_thread = None + logger.info("[HubMgr] GC thread exiting (no active entries)") + break # restarted on next acquire() + # Module-level singleton — imported and used by tasks_runner_lvol_migration hub_manager = HubControllerManager() diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index f9fead742..d86bf72db 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -242,16 +242,18 @@ def _apply_migration_to_db(migration, tgt_lvol_uuid=None, tgt_lvol_bdev=None): logger.warning(f"_apply_migration_to_db: snapshot not found: {snap_uuid}") continue - # snap_bdev: update lvstore prefix and add migration suffix. - # Guard is idempotent: if called twice after crash+retry, src_short - # already carries the suffix — do not append it again. + # snap_bdev: update lvstore prefix and normalise the migration suffix. + # Use the lvstore to detect a retry (snap already updated) rather than + # endswith(suffix), which gives a false-positive on back-to-back migrations + # where the source bdev legitimately ends with the suffix from the prior run. tgt_short = None if snap.snap_bdev and '/' in snap.snap_bdev: - src_short = snap.snap_bdev.split('/', 1)[1] - if src_short.endswith(_MIGRATION_BDEV_SUFFIX): - tgt_short = src_short + src_lvstore, src_short = snap.snap_bdev.split('/', 1) + if src_lvstore == tgt_node.lvstore: + tgt_short = src_short # already updated by a previous call — idempotent else: - tgt_short = src_short + _MIGRATION_BDEV_SUFFIX + base = src_short[:-len(_MIGRATION_BDEV_SUFFIX)] if src_short.endswith(_MIGRATION_BDEV_SUFFIX) else src_short + tgt_short = base + _MIGRATION_BDEV_SUFFIX snap.snap_bdev = f"{tgt_node.lvstore}/{tgt_short}" if tgt_short and tgt_short in spdk_info: @@ -331,15 +333,13 @@ def _snap_short_name(snap): def _snap_tgt_short_name(snap): """Return the migration-target bdev short name for a snapshot. - Normally the target bdev is named + _MIGRATION_BDEV_SUFFIX. - However, when apply_migration_to_db() is called early (at cutover time), - snap.snap_bdev is already updated to the target name which already carries the - suffix. In that case return it as-is to avoid producing a double suffix (e.g. - 'SNAP_16745mm' instead of 'SNAP_16745m'). + Always strips any existing migration suffix before adding one so that + back-to-back migrations (where the source bdev already carries the suffix + from the previous migration) do not produce a double suffix like 'SNAP_16745mm'. """ short = _snap_short_name(snap) if short.endswith(_MIGRATION_BDEV_SUFFIX): - return short # apply_migration_to_db already updated this snap + short = short[:-len(_MIGRATION_BDEV_SUFFIX)] return short + _MIGRATION_BDEV_SUFFIX @@ -630,7 +630,7 @@ def _setup_snap_transfer(snap, snap_index, migration, src_node, tgt_node, Callers are responsible for rolling back any previously launched transfers. """ snap_uuid = snap.uuid - snap_short = _snap_short_name(snap) + _MIGRATION_BDEV_SUFFIX + snap_short = _snap_tgt_short_name(snap) src_composite = _snap_composite(src_node.lvstore, snap) tgt_composite = f"{tgt_node.lvstore}/{snap_short}" @@ -748,7 +748,7 @@ def _post_process_snap(snap: SnapShot, tgt_node: StorageNode, tgt_rpc: RPCClient # Predecessor was created on target with the migration suffix — build # composite from the source short name + suffix, not from snap_bdev # (which still holds the source path until apply_migration_to_db runs). - pred_composite = f"{tgt_node.lvstore}/{_snap_short_name(pred_snap) + _MIGRATION_BDEV_SUFFIX}" + pred_composite = f"{tgt_node.lvstore}/{_snap_tgt_short_name(pred_snap)}" ret = tgt_rpc.bdev_lvol_add_clone(tgt_composite, pred_composite) if not ret: return False, f"bdev_lvol_add_clone failed for {snap_uuid}" @@ -875,8 +875,7 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): except KeyError: return False, True, f"Snapshot {snap_uuid} not found in DB" - snap_short_src = _snap_short_name(snap) - snap_short_tgt = snap_short_src + _MIGRATION_BDEV_SUFFIX + snap_short_tgt = _snap_tgt_short_name(snap) src_composite = _snap_composite(src_node.lvstore, snap) tgt_composite = f"{tgt_node.lvstore}/{snap_short_tgt}" @@ -1087,8 +1086,7 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): if tgt_sec: sec_rpc = _make_rpc(tgt_sec) - snap_short_src = _snap_short_name(snap) - snap_short_tgt = snap_short_src + _MIGRATION_BDEV_SUFFIX + snap_short_tgt = _snap_tgt_short_name(snap) src_composite = _snap_composite(src_node.lvstore, snap) tgt_composite = f"{tgt_node.lvstore}/{snap_short_tgt}" @@ -1336,7 +1334,7 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): src_rpc.bdev_nvme_detach_controller(ctrl_name) _delete_bdev_blocking(tgt_lvol_composite, tgt_rpc, secondary_rpc=sec_setup_rpc) return False, True, f"Last snapshot {last_snap_uuid} not found" - tgt_snap_composite = f"{tgt_node.lvstore}/{_snap_short_name(last_snap) + _MIGRATION_BDEV_SUFFIX}" + tgt_snap_composite = f"{tgt_node.lvstore}/{_snap_tgt_short_name(last_snap)}" else: last_snap_uuid = migration.snaps_preexisting_on_target[-1] last_snap = db.get_snapshot_by_id(last_snap_uuid) @@ -1742,12 +1740,6 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): # bdev_nvme_detach_controller can still reach the hub bdev. # Step 8: delete source primary NVMe-oF subsystem. # Then: delete source lvol bdev. - # Step 7: release the hub controller back to the manager. - # HubControllerManager keeps it alive for IDLE_TIMEOUT seconds so that a - # back-to-back migration to the same target can reuse it without hitting - # the 'deleting' state race that an immediate detach would cause. - hub_manager.release(src_node.get_id(), tgt_node.get_id()) - logger.info(f"Step 7: hub controller released to idle manager (src={src_node.get_id()[:8]} tgt={tgt_node.get_id()[:8]})") lvol = None try: lvol = db.get_lvol_by_id(migration.lvol_id) From a13a1b21b88dc57653c0a2200fc0a877ad7ea5bb Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Thu, 25 Jun 2026 15:09:24 +0330 Subject: [PATCH 04/49] fixing an issue with tertiary nodes not being registered on the target --- .../services/hub_controller_manager.py | 8 - .../services/tasks_runner_lvol_migration.py | 138 ++++++++++-------- 2 files changed, 78 insertions(+), 68 deletions(-) diff --git a/simplyblock_core/services/hub_controller_manager.py b/simplyblock_core/services/hub_controller_manager.py index c0184dbcc..195ccfa50 100644 --- a/simplyblock_core/services/hub_controller_manager.py +++ b/simplyblock_core/services/hub_controller_manager.py @@ -158,14 +158,6 @@ def acquire(self, src_node_id: str, src_rpc, tgt_node, trtype: str): ) return ctrl_name, hub_bdev, None - def release(self, src_node_id: str, tgt_node_id: str): # noqa: ARG002 - """ - No-op — kept for call-site compatibility. - - The idle timer is driven entirely by acquire(); explicit release calls - are not needed and were a source of refcount imbalance bugs. - """ - def detach_now(self, src_node_id: str, tgt_node_id: str, src_rpc=None): """ Immediately detach the hub controller (failure / cancel path). diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index d86bf72db..10415e3b3 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -394,22 +394,22 @@ def _log_spdk_bdev_size(rpc, composite_name, label): return None -def _delete_bdev_blocking(bdev_name, primary_rpc, secondary_rpc=None): +def _delete_bdev_blocking(bdev_name, primary_rpc, secondary_rpc=None, tertiary_rpc=None): """ Delete a bdev cleanly in a single RPC call. special_delete=True handles blob reference cleanup atomically — no - async-start/poll/finalize sequence needed. secondary_rpc deletion is - best-effort (non-fatal if the secondary is unreachable). + async-start/poll/finalize sequence needed. secondary/tertiary deletion is + best-effort (non-fatal if the node is unreachable). """ ret, _ = primary_rpc.delete_lvol(bdev_name, special_delete=True) if not ret: logger.warning(f"delete bdev {bdev_name}: special_delete failed (continuing)") - if secondary_rpc: + for extra_rpc in filter(None, [secondary_rpc, tertiary_rpc]): try: - secondary_rpc.delete_lvol(bdev_name, special_delete=True) + extra_rpc.delete_lvol(bdev_name, special_delete=True) except Exception as e: - logger.warning(f"delete bdev {bdev_name} on secondary (non-fatal): {e}") + logger.warning(f"delete bdev {bdev_name} on secondary/tertiary (non-fatal): {e}") # --------------------------------------------------------------------------- @@ -471,39 +471,6 @@ def _get_target_tertiary_node(tgt_node, src_node_id=None): -def _get_target_secondary_nodes(tgt_node): - """ - Return ``(nodes, error_string)`` for all secondary/tertiary nodes of tgt_node. - - Rules (applied independently to each secondary and tertiary slot): - - Not configured → skip - - STATUS_ONLINE → include in result list - - STATUS_OFFLINE → skip (administratively down, non-blocking) - - Any other status → block: return ([], error_str) immediately - - If any node blocks, the entire result is empty and the error describes - which node and what state it's in. - """ - nodes = [] - for attr in ("secondary_node_id", "tertiary_node_id"): - node_id = getattr(tgt_node, attr, None) - if not node_id: - continue - try: - n = db.get_storage_node_by_id(node_id) - except KeyError: - continue - if n.status == StorageNode.STATUS_ONLINE: - nodes.append(n) - elif n.status == StorageNode.STATUS_OFFLINE: - continue - else: - return [], ( - f"Target node {node_id} is in state '{n.status}'; " - f"cannot proceed with migration" - ) - return nodes, None - def _build_paths(src_node, tgt_node, src_rpc, tgt_rpc): """Build ordered path lists for source and target nodes and compute overlap. @@ -614,13 +581,14 @@ def _cleanup_final_migration(src_rpc, ctx, tgt_rpc=None, rollback_target=False): # --------------------------------------------------------------------------- -def _setup_snap_transfer(snap, snap_index, migration, src_node, tgt_node, +def _setup_snap_transfer(snap, snap_index, src_node, tgt_node, src_rpc, tgt_rpc, trtype, - tgt_sec=None, sec_rpc=None, lvol_size_mib=None): + tgt_sec=None, sec_rpc=None, tgt_ter=None, ter_rpc=None, + lvol_size_mib=None): """ Prepare a single snapshot for async transfer: 1. Create writable lvol on target primary - 2. Register on target secondary immediately (keeps secondary consistent) + 2. Register on target secondary/tertiary immediately (keeps HA nodes consistent) 3. Set migration flag on primary 4. Get map_id of target bdev for hub-based transfer 5. Ensure hub NVMe-oF controller is attached on source @@ -654,6 +622,7 @@ def _setup_snap_transfer(snap, snap_index, migration, src_node, tgt_node, # If registration fails we clean up the primary bdev and abort — continuing # with an unregistered secondary would leave the cluster in split state. sec_registered = False + ter_registered = False if tgt_sec and sec_rpc: bdev_info = tgt_rpc.get_bdevs(tgt_composite) if not bdev_info: @@ -668,11 +637,20 @@ def _setup_snap_transfer(snap, snap_index, migration, src_node, tgt_node, _delete_bdev_blocking(tgt_composite, tgt_rpc, sec_rpc) return None, f"bdev_lvol_register on secondary failed for snap {snap_uuid}" sec_registered = True - - # Helper: clean primary and secondary (if registered) on error + if tgt_ter and ter_rpc: + ret_ter = ter_rpc.bdev_lvol_register( + snap_short, tgt_node.lvstore, snap_uuid_on_tgt, snap_blobid, + snap.lvol.lvol_priority_class if hasattr(snap, 'lvol') else 0) + if not ret_ter: + _delete_bdev_blocking(tgt_composite, tgt_rpc, sec_rpc, ter_rpc) + return None, f"bdev_lvol_register on tertiary failed for snap {snap_uuid}" + ter_registered = True + + # Helper: clean primary, secondary, and tertiary (if registered) on error def _cleanup(): _delete_bdev_blocking(tgt_composite, tgt_rpc, - secondary_rpc=sec_rpc if sec_registered else None) + secondary_rpc=sec_rpc if sec_registered else None, + tertiary_rpc=ter_rpc if ter_registered else None) # Step 3: migration flag on primary ret = tgt_rpc.bdev_lvol_set_migration_flag(tgt_composite) @@ -718,7 +696,8 @@ def _cleanup(): def _post_process_snap(snap: SnapShot, tgt_node: StorageNode, tgt_rpc: RPCClient, migration: LVolMigration, - transfer: dict, tgt_sec:Optional[StorageNode]=None, sec_rpc: Optional[RPCClient]=None): + transfer: dict, tgt_sec:Optional[StorageNode]=None, sec_rpc: Optional[RPCClient]=None, + tgt_ter:Optional[StorageNode]=None, ter_rpc: Optional[RPCClient]=None): """ Post-transfer steps for a single snapshot whose data has been fully copied: add_clone → convert (on primary, then mirrored on secondary) → cleanup. @@ -756,6 +735,10 @@ def _post_process_snap(snap: SnapShot, tgt_node: StorageNode, tgt_rpc: RPCClient ret_sec = sec_rpc.bdev_lvol_add_clone(tgt_composite, pred_composite) if not ret_sec: return False, f"bdev_lvol_add_clone on secondary failed for {snap_uuid}" + if tgt_ter and ter_rpc: + ret_ter = ter_rpc.bdev_lvol_add_clone(tgt_composite, pred_composite) + if not ret_ter: + return False, f"bdev_lvol_add_clone on tertiary failed for {snap_uuid}" except KeyError: logger.warning(f"Predecessor snap {pred_uuid} not found; skipping add_clone") @@ -770,6 +753,10 @@ def _post_process_snap(snap: SnapShot, tgt_node: StorageNode, tgt_rpc: RPCClient ret_sec = sec_rpc.bdev_lvol_convert(tgt_composite) if not ret_sec: return False, f"bdev_lvol_convert on secondary failed for {snap_uuid}" + if tgt_ter and ter_rpc: + ret_ter = ter_rpc.bdev_lvol_convert(tgt_composite) + if not ret_ter: + return False, f"bdev_lvol_convert on tertiary failed for {snap_uuid}" # Early partial DB update: route health-check and delete to the target node # immediately after convert. snap_bdev keeps its source path here; the full @@ -840,6 +827,11 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): except KeyError: _snap_lvol_size_mib = None + tgt_sec = None + sec_rpc = None + tgt_ter = None + ter_rpc = None + # ── A. Launch / resume planned snapshots one at a time ─────────────────── # SPDK only supports one bdev_lvol_transfer per poller group at a time; # launching multiple causes "poller already exists" and stuck transfers. @@ -849,9 +841,7 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): unprocessed = all_unprocessed[:_PARALLEL_BATCH] if unprocessed: - # HA secondary gate – check once; all snaps belong to the same volume - tgt_sec = None - sec_rpc = None + # HA secondary/tertiary gate – check once; all snaps belong to the same volume for snap_uuid in unprocessed: try: snap = db.get_snapshot_by_id(snap_uuid) @@ -865,7 +855,22 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): return False, True, _WAIT if tgt_sec: sec_rpc = _make_rpc(tgt_sec) - break # one check is enough + elif snap.lvol.ha_type == "ha3": + tgt_sec, sec_err = _get_target_secondary_node(tgt_node) + if sec_err: + migration.error_message = sec_err + migration.write_to_db(db.kv_store) + return False, True, _WAIT + if tgt_sec: + sec_rpc = _make_rpc(tgt_sec) + tgt_ter, ter_err = _get_target_tertiary_node(tgt_node) + if ter_err: + migration.error_message = ter_err + migration.write_to_db(db.kv_store) + return False, True, _WAIT + if tgt_ter: + ter_rpc = _make_rpc(tgt_ter) + break # one check is enough transfers: list[dict] = [] for snap_uuid in unprocessed: @@ -915,9 +920,10 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): logger.warning(f"Pre-cleanup of {tgt_composite} failed (continuing): {e}") t, err = _setup_snap_transfer( - snap, snap_index, migration, src_node, tgt_node, + snap, snap_index, src_node, tgt_node, src_rpc, tgt_rpc, trtype, tgt_sec=tgt_sec, sec_rpc=sec_rpc, + tgt_ter=tgt_ter, ter_rpc=ter_rpc, lvol_size_mib=_snap_lvol_size_mib) if t is None: return False, True, err @@ -999,7 +1005,8 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): ok, err = _post_process_snap( snap, tgt_node, tgt_rpc, migration, t, - tgt_sec=tgt_sec, sec_rpc=sec_rpc) + tgt_sec=tgt_sec, sec_rpc=sec_rpc, + tgt_ter=tgt_ter, ter_rpc=ter_rpc) if not ok: migration.transfer_context = {} if err is _WAIT: @@ -1077,14 +1084,24 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): tgt_sec = None sec_rpc = None - if snap.lvol.ha_type == "ha": - tgt_sec, sec_err = _get_target_secondary_node(tgt_node, src_node.get_id()) + tgt_ter = None + ter_rpc = None + if snap.lvol.ha_type in ("ha", "ha3"): + tgt_sec, sec_err = _get_target_secondary_node(tgt_node) if sec_err: migration.error_message = sec_err migration.write_to_db(db.kv_store) return False, True, _WAIT if tgt_sec: sec_rpc = _make_rpc(tgt_sec) + if snap.lvol.ha_type == "ha3": + tgt_ter, ter_err = _get_target_tertiary_node(tgt_node) + if ter_err: + migration.error_message = ter_err + migration.write_to_db(db.kv_store) + return False, True, _WAIT + if tgt_ter: + ter_rpc = _make_rpc(tgt_ter) snap_short_tgt = _snap_tgt_short_name(snap) src_composite = _snap_composite(src_node.lvstore, snap) @@ -1100,9 +1117,10 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): logger.warning(f"Pre-cleanup of {tgt_composite} failed (continuing): {e}") t, err = _setup_snap_transfer( - snap, snap_index, migration, src_node, tgt_node, + snap, snap_index, src_node, tgt_node, src_rpc, tgt_rpc, trtype, tgt_sec=tgt_sec, sec_rpc=sec_rpc, + tgt_ter=tgt_ter, ter_rpc=ter_rpc, lvol_size_mib=_snap_lvol_size_mib) if t is None: return False, True, err @@ -1133,7 +1151,8 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): ok, err = _post_process_snap( snap, tgt_node, tgt_rpc, migration, t, - tgt_sec=tgt_sec, sec_rpc=sec_rpc) + tgt_sec=tgt_sec, sec_rpc=sec_rpc, + tgt_ter=tgt_ter, ter_rpc=ter_rpc) if not ok: if err is _WAIT: migration.error_message = ( @@ -1304,7 +1323,6 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): return False, True, f"Could not find map_id for {lvol.lvol_bdev} on target" logger.info(f"[MAP_ID] {tgt_lvol_bdev} map_id={tgt_map_id} uuid={tgt_uuid} on {tgt_node.get_id()[:8]}") - sec_setup_rpc = None # NVMe-oF subsystem setup is deferred to the Done handler — the subsystem # is deleted and recreated fresh after transfer completes so all paths get @@ -1322,7 +1340,7 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): # Step 4: locate the last migrated snapshot's composite name on the target if not migration.snaps_migrated and not migration.snaps_preexisting_on_target: src_rpc.bdev_nvme_detach_controller(ctrl_name) - _delete_bdev_blocking(tgt_lvol_composite, tgt_rpc, secondary_rpc=sec_setup_rpc) + _delete_bdev_blocking(tgt_lvol_composite, tgt_rpc, secondary_rpc=None) return False, True, "No snapshots migrated; cannot perform final migration" tgt_snap_composite = "" @@ -1332,7 +1350,7 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): last_snap = db.get_snapshot_by_id(last_snap_uuid) except KeyError: src_rpc.bdev_nvme_detach_controller(ctrl_name) - _delete_bdev_blocking(tgt_lvol_composite, tgt_rpc, secondary_rpc=sec_setup_rpc) + _delete_bdev_blocking(tgt_lvol_composite, tgt_rpc, secondary_rpc=None) return False, True, f"Last snapshot {last_snap_uuid} not found" tgt_snap_composite = f"{tgt_node.lvstore}/{_snap_tgt_short_name(last_snap)}" else: @@ -1353,7 +1371,7 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): if not tgt_snap_composite: src_rpc.bdev_nvme_detach_controller(ctrl_name) - _delete_bdev_blocking(tgt_lvol_composite, tgt_rpc, secondary_rpc=sec_setup_rpc) + _delete_bdev_blocking(tgt_lvol_composite, tgt_rpc, secondary_rpc=None) return False, True, f"snapshot {last_snap_uuid} not found on target" # Step 5: start final migration — synchronous: blocks until SPDK completes From e67146bbc4f27d80169b923006fe452764d88432 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Thu, 25 Jun 2026 15:54:57 +0330 Subject: [PATCH 05/49] fixed an issue where the migration will report failure desptie successful transfer --- .../controllers/migration_controller.py | 6 +- .../services/tasks_runner_lvol_migration.py | 55 +++++++++++++++---- 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/simplyblock_core/controllers/migration_controller.py b/simplyblock_core/controllers/migration_controller.py index 624080662..60a2918dc 100644 --- a/simplyblock_core/controllers/migration_controller.py +++ b/simplyblock_core/controllers/migration_controller.py @@ -222,7 +222,8 @@ def _cleanup_created(migration): return nqn = lvol.nqn - bdev_short = lvol.lvol_bdev + _MIGRATION_BDEV_SUFFIX + _lvol_base = lvol.lvol_bdev[:-len(_MIGRATION_BDEV_SUFFIX)] if lvol.lvol_bdev.endswith(_MIGRATION_BDEV_SUFFIX) else lvol.lvol_bdev + bdev_short = _lvol_base + _MIGRATION_BDEV_SUFFIX composite = f"{tgt_node.lvstore}/{bdev_short}" # Compute overlap: nodes shared between SRC and TGT paths. @@ -666,7 +667,8 @@ def create_migration(lvol_id, target_node_id, cluster = db.get_cluster_by_id(tgt_node.cluster_id) tgt_rpc = tgt_node.rpc_client() nqn = lvol.nqn - bdev_short = lvol.lvol_bdev + _MIGRATION_BDEV_SUFFIX + _lvol_base = lvol.lvol_bdev[:-len(_MIGRATION_BDEV_SUFFIX)] if lvol.lvol_bdev.endswith(_MIGRATION_BDEV_SUFFIX) else lvol.lvol_bdev + bdev_short = _lvol_base + _MIGRATION_BDEV_SUFFIX composite = f"{tgt_node.lvstore}/{bdev_short}" size_in_mib = convert_size(lvol.size, 'MiB') tgt_port = tgt_node.get_lvol_subsys_port(tgt_node.lvstore) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index 10415e3b3..819b03418 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -343,6 +343,18 @@ def _snap_tgt_short_name(snap): return short + _MIGRATION_BDEV_SUFFIX +def _lvol_tgt_bdev_name(lvol_bdev: str) -> str: + """Return the migration-target bdev short name for a writable lvol. + + Same strip-then-add logic as _snap_tgt_short_name: prevents 'LVOL_Xmm' + accumulation on back-to-back migrations where lvol_bdev already ends in + the suffix from the previous run. + """ + if lvol_bdev.endswith(_MIGRATION_BDEV_SUFFIX): + lvol_bdev = lvol_bdev[:-len(_MIGRATION_BDEV_SUFFIX)] + return lvol_bdev + _MIGRATION_BDEV_SUFFIX + + def _snap_composite(lvstore, snap): """SPDK composite bdev name for a snapshot on a given node: ``/``.""" return f"{lvstore}/{_snap_short_name(snap)}" @@ -916,6 +928,12 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): f"Removing leftover target bdev {tgt_composite} from failed attempt") try: _delete_bdev_blocking(tgt_composite, tgt_rpc, sec_rpc) + # SPDK delete is async — poll until the bdev is actually gone + # before letting the caller attempt a create on the same name. + for _ in range(10): + if not tgt_rpc.get_bdevs(tgt_composite): + break + time.sleep(0.2) except Exception as e: logger.warning(f"Pre-cleanup of {tgt_composite} failed (continuing): {e}") @@ -1113,6 +1131,12 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): logger.info(f"Pre-cleanup: removing stale intermediate bdev {tgt_composite}") try: _delete_bdev_blocking(tgt_composite, tgt_rpc) + # SPDK delete is async — poll until the bdev is actually gone + # before letting the caller attempt a create on the same name. + for _ in range(10): + if not tgt_rpc.get_bdevs(tgt_composite): + break + time.sleep(0.2) except Exception as e: logger.warning(f"Pre-cleanup of {tgt_composite} failed (continuing): {e}") @@ -1248,7 +1272,7 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): trtype, _ = _get_migration_nic(tgt_node) src_lvol_composite = f"{src_node.lvstore}/{lvol.lvol_bdev}" - tgt_lvol_bdev = lvol.lvol_bdev + _MIGRATION_BDEV_SUFFIX + tgt_lvol_bdev = _lvol_tgt_bdev_name(lvol.lvol_bdev) tgt_lvol_composite = f"{tgt_node.lvstore}/{tgt_lvol_bdev}" ctx = migration.transfer_context or {} @@ -1382,16 +1406,25 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): ret = src_rpc.bdev_lvol_final_migration( src_lvol_composite, tgt_map_id, tgt_snap_composite, 2, hub_bdev) if ret is None: - src_rpc.bdev_nvme_detach_controller(ctrl_name) - # Do NOT delete the target bdev on transfer failure — the bdev is - # still valid and retaining it keeps the map_id stable across retries. - # Deleting it would force a recreate at a higher map_id (due to - # concurrent migrations creating bdevs in the interim). - return False, True, "bdev_lvol_final_migration failed" - - logger.info( - f"[IO-RESUME] {_now_ms()} final migration Done: " - f"lvol={migration.lvol_id} io now live on target") + # Connection timeout or SPDK error (e.g. "File exists" = already in progress). + # SPDK may have completed the migration while the RPC connection dropped. + # Check transfer_stat before treating this as a hard failure. + stat = src_rpc.bdev_lvol_transfer_stat(src_lvol_composite) + state = (stat or {}).get('transfer_state') if stat is not None else None + if state not in ('Done', 'No process'): + src_rpc.bdev_nvme_detach_controller(ctrl_name) + # Do NOT delete the target bdev on transfer failure — the bdev is + # still valid and retaining it keeps the map_id stable across retries. + # Deleting it would force a recreate at a higher map_id (due to + # concurrent migrations creating bdevs in the interim). + return False, True, "bdev_lvol_final_migration failed" + logger.info( + f"[IO-RESUME] {_now_ms()} final migration complete (recovered from RPC error, " + f"transfer_state={state}): lvol={migration.lvol_id} io now live on target") + else: + logger.info( + f"[IO-RESUME] {_now_ms()} final migration Done: " + f"lvol={migration.lvol_id} io now live on target") # add_clone on secondary and tertiary to link the final migrated lvol to # its predecessor snapshot in their ancestry chain. From b87e39e233a2bc7141644cda57c4487163fd6056 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Thu, 25 Jun 2026 16:03:48 +0330 Subject: [PATCH 06/49] fixing mypy type checker issue for constants --- simplyblock_core/constants.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/simplyblock_core/constants.py b/simplyblock_core/constants.py index e6288e1c6..bc7336146 100644 --- a/simplyblock_core/constants.py +++ b/simplyblock_core/constants.py @@ -211,8 +211,8 @@ def get_config_var(name, default=None): MAX_SNAP_COUNT = 100 -SPDK_PROXY_MULTI_THREADING_ENABLED=True -SPDK_PROXY_TIMEOUT=60*5 +SPDK_PROXY_MULTI_THREADING_ENABLED: bool = True +SPDK_PROXY_TIMEOUT: int = 60 * 5 LVOL_NVME_CONNECT_RECONNECT_DELAY=2 LVOL_NVME_CONNECT_CTRL_LOSS_TMO=60*60 LVOL_NVME_CONNECT_FAST_IO_FAIL_TO=1 From 79cbe598054a8dd42b96c985eaaa4f0bad49f484 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Thu, 25 Jun 2026 17:57:53 +0330 Subject: [PATCH 07/49] prevent the migration from trying to delete snapshots migrated from previous migrations --- .../services/tasks_runner_lvol_migration.py | 76 +++++++++++++------ 1 file changed, 53 insertions(+), 23 deletions(-) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index 819b03418..1404fbb7f 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -185,8 +185,8 @@ def _apply_migration_to_db(migration, tgt_lvol_uuid=None, tgt_lvol_bdev=None): f"_apply_migration_to_db: queried {len(spdk_info)} bdevs " f"from target lvstore {tgt_node.lvstore}") subsys = tgt_rpc.subsystem_list(lvol.nqn) - if subsys: - for ns in subsys[0].get('namespaces'): + if subsys and isinstance(subsys[0], dict): + for ns in subsys[0].get('namespaces') or []: if ns['uuid'] == lvol.uuid: lvol.ns_id = ns['nsid'] break @@ -844,12 +844,52 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): tgt_ter = None ter_rpc = None + # ── PRE-SCAN: mark snapshots already on target as pre-existing ──────────── + # Query the target lvstore once and cross-reference the full migration plan. + # Any planned snap whose target bdev already exists AND whose DB snap_bdev + # already points to the target was fully migrated by a prior migration — + # mark it pre-existing so we never attempt to delete+recreate it. + # Writable leftovers (DB snap_bdev still points to source) are NOT marked + # here; they fall through to the per-snap pre-cleanup to be deleted and + # retransferred as before. + if ctx.get('stage') != 'parallel_transfer' and plan: + try: + _tgt_lvols = tgt_rpc.bdev_lvol_get_lvols(tgt_node.lvstore) or [] + _tgt_short_set = {e.get('name', '').split('/')[-1] for e in _tgt_lvols} + except Exception as _pre_e: + logger.warning(f"Pre-scan: bdev_lvol_get_lvols failed ({_pre_e}); skipping") + _tgt_short_set = set() + + _pre_scan_updated = False + for _snap_uuid in plan: + if (_snap_uuid in migration.snaps_migrated + or _snap_uuid in migration.snaps_preexisting_on_target): + continue + try: + _s = db.get_snapshot_by_id(_snap_uuid) + except KeyError: + continue + _short_tgt = _snap_tgt_short_name(_s) + if _short_tgt not in _tgt_short_set: + continue + _tgt_comp = f"{tgt_node.lvstore}/{_short_tgt}" + if _s.snap_bdev == _tgt_comp: + logger.info( + f"Pre-scan: {_snap_uuid} ({_short_tgt}) already on target " + f"(DB snap_bdev match); marking pre-existing") + migration.snaps_preexisting_on_target.append(_snap_uuid) + _pre_scan_updated = True + if _pre_scan_updated: + migration.write_to_db(db.kv_store) + # ── A. Launch / resume planned snapshots one at a time ─────────────────── # SPDK only supports one bdev_lvol_transfer per poller group at a time; # launching multiple causes "poller already exists" and stuck transfers. _PARALLEL_BATCH = 1 if ctx.get('stage') != 'parallel_transfer': - all_unprocessed = [u for u in plan if u not in migration.snaps_migrated] + all_unprocessed = [u for u in plan + if u not in migration.snaps_migrated + and u not in migration.snaps_preexisting_on_target] unprocessed = all_unprocessed[:_PARALLEL_BATCH] if unprocessed: @@ -911,25 +951,14 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): }) continue - # Check for a bdev already present on the target. - # Two distinct cases must be handled: - # 1. Genuinely pre-existing (placed by a sibling migration that - # already ran) — snap is already in snaps_migrated → skip. - # 2. Leftover from a previous failed attempt of THIS migration — - # snap is NOT yet in snaps_migrated → delete and retry, so we - # do not resume from a partially-written bdev. + # Pre-existing (immutable) bdevs were caught by the pre-scan above and + # excluded from unprocessed. Anything still found here is a writable + # leftover from a previous failed attempt — delete and retry. if tgt_rpc.get_bdevs(tgt_composite): - if snap_uuid in migration.snaps_migrated: - logger.info( - f"Snapshot {snap_uuid} already on target; skipping transfer") - migration.snaps_preexisting_on_target.append(snap_uuid) - continue logger.info( - f"Removing leftover target bdev {tgt_composite} from failed attempt") + f"Removing writable leftover target bdev {tgt_composite}") try: _delete_bdev_blocking(tgt_composite, tgt_rpc, sec_rpc) - # SPDK delete is async — poll until the bdev is actually gone - # before letting the caller attempt a create on the same name. for _ in range(10): if not tgt_rpc.get_bdevs(tgt_composite): break @@ -1053,7 +1082,9 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): # If there are more unprocessed snaps, return now so the next tick # launches the next batch. - remaining = [u for u in plan if u not in migration.snaps_migrated] + remaining = [u for u in plan + if u not in migration.snaps_migrated + and u not in migration.snaps_preexisting_on_target] if remaining: return False, False, None @@ -1125,14 +1156,13 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): src_composite = _snap_composite(src_node.lvstore, snap) tgt_composite = f"{tgt_node.lvstore}/{snap_short_tgt}" - # Pre-cleanup: only delete if the bdev actually exists on the target - # (stale from a previous crashed run). Deleting blindly masks real errors. + # Pre-cleanup: if a bdev exists on the target it is a writable leftover + # from a previous crashed run — intermediate snaps are always freshly + # created by this migration so they can never be pre-existing. if tgt_rpc.get_bdevs(tgt_composite): logger.info(f"Pre-cleanup: removing stale intermediate bdev {tgt_composite}") try: _delete_bdev_blocking(tgt_composite, tgt_rpc) - # SPDK delete is async — poll until the bdev is actually gone - # before letting the caller attempt a create on the same name. for _ in range(10): if not tgt_rpc.get_bdevs(tgt_composite): break From 64934f88e13f712705c60e2a172bbb4d4f4d60cf Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Thu, 25 Jun 2026 19:06:09 +0330 Subject: [PATCH 08/49] the pre-scan now checks for is_snapshot field --- .../services/tasks_runner_lvol_migration.py | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index 1404fbb7f..ddca872b1 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -845,20 +845,26 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): ter_rpc = None # ── PRE-SCAN: mark snapshots already on target as pre-existing ──────────── - # Query the target lvstore once and cross-reference the full migration plan. - # Any planned snap whose target bdev already exists AND whose DB snap_bdev - # already points to the target was fully migrated by a prior migration — - # mark it pre-existing so we never attempt to delete+recreate it. - # Writable leftovers (DB snap_bdev still points to source) are NOT marked - # here; they fall through to the per-snap pre-cleanup to be deleted and - # retransferred as before. + # Query the target lvstore once. Any planned snap whose target bdev already + # exists AND is immutable (is_snapshot=true in SPDK) was fully converted by + # a prior migration — mark pre-existing so we skip the transfer entirely. + # We use the SPDK is_snapshot flag rather than the DB snap_bdev field because + # snap_bdev is not updated until apply_migration_to_db() runs at the very end + # of CLEANUP_SOURCE; a migration that succeeded at SNAP_COPY but failed later + # would leave immutable snapshots on the target with stale DB records. + # Writable bdevs (is_snapshot=false) are leftovers from a crashed transfer — + # they fall through to the per-snap pre-cleanup to be deleted and retried. if ctx.get('stage') != 'parallel_transfer' and plan: try: _tgt_lvols = tgt_rpc.bdev_lvol_get_lvols(tgt_node.lvstore) or [] - _tgt_short_set = {e.get('name', '').split('/')[-1] for e in _tgt_lvols} + _tgt_immutable = { + e.get('name', '').split('/')[-1] + for e in _tgt_lvols + if e.get('is_snapshot', False) + } except Exception as _pre_e: logger.warning(f"Pre-scan: bdev_lvol_get_lvols failed ({_pre_e}); skipping") - _tgt_short_set = set() + _tgt_immutable = set() _pre_scan_updated = False for _snap_uuid in plan: @@ -870,13 +876,10 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): except KeyError: continue _short_tgt = _snap_tgt_short_name(_s) - if _short_tgt not in _tgt_short_set: - continue - _tgt_comp = f"{tgt_node.lvstore}/{_short_tgt}" - if _s.snap_bdev == _tgt_comp: + if _short_tgt in _tgt_immutable: logger.info( - f"Pre-scan: {_snap_uuid} ({_short_tgt}) already on target " - f"(DB snap_bdev match); marking pre-existing") + f"Pre-scan: {_snap_uuid} ({_short_tgt}) is already an immutable " + f"snapshot on target; marking pre-existing") migration.snaps_preexisting_on_target.append(_snap_uuid) _pre_scan_updated = True if _pre_scan_updated: From 735c420abb882cbd081e0964726dd28f7a8c70e5 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Fri, 26 Jun 2026 14:14:13 +0330 Subject: [PATCH 09/49] removing the logic to generate a snapshot if the source lvol has none --- .../services/tasks_runner_lvol_migration.py | 35 +++++++------------ 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index ddca872b1..986272e49 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -1099,19 +1099,12 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): # Before each round check the current dirty delta. If it is already below # the threshold the remaining freeze window will be short enough that no # additional shrink pass is worth the overhead. - # If no snapshots have been migrated at all (volume had no pre-existing snapshots - # and none were planned), we must take at least one intermediate snapshot so that - # _handle_lvol_migrate has a base to clone from. Skip the delta threshold check - # on the first pass in this case. - _must_snap = (not migration.snaps_migrated - and not migration.snaps_preexisting_on_target) - while migration.intermediate_snap_rounds < migration.max_intermediate_snap_rounds: _lvol = db.get_lvol_by_id(migration.lvol_id) _src_composite = f"{src_node.lvstore}/{_lvol.lvol_bdev}" _delta = _get_lvol_delta_bytes(src_rpc, _src_composite) _threshold = constants.LVOL_MIG_INTERMEDIATE_SNAP_THRESHOLD_BYTES - if not _must_snap and _delta is not None and _delta <= _threshold: + if _delta is not None and _delta <= _threshold: logger.info( f"Intermediate snapshot skipped: delta {convert_size(_delta, 'MiB')} MiB " f"<= {convert_size(_threshold, 'MiB')} MiB threshold " @@ -1122,10 +1115,9 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): f"{'unknown' if _delta is None else str(convert_size(_delta, 'MiB')) + ' MiB'} " f"exceeds {convert_size(_threshold, 'MiB')} MiB threshold") _take_intermediate_snapshot(migration) - _must_snap = False plan = migration.snap_migration_plan if not plan: - return False, True, "Intermediate snapshot failed and no snapshots available for migration" + return False, True, "Intermediate snapshot failed" snap_uuid = plan[-1] snap_index = len(plan) - 1 @@ -1394,12 +1386,10 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): # which changes its map_id and breaks concurrent migration tracking. return False, True, hub_err - # Step 4: locate the last migrated snapshot's composite name on the target - if not migration.snaps_migrated and not migration.snaps_preexisting_on_target: - src_rpc.bdev_nvme_detach_controller(ctrl_name) - _delete_bdev_blocking(tgt_lvol_composite, tgt_rpc, secondary_rpc=None) - return False, True, "No snapshots migrated; cannot perform final migration" - + # Step 4: locate the last migrated snapshot's composite name on the target. + # If no snapshots were migrated (volume had no snapshots and delta was + # below the intermediate-snap threshold), pass an empty string — SPDK + # performs a full-lvol transfer without a snapshot base. tgt_snap_composite = "" if migration.snaps_migrated: last_snap_uuid = migration.snaps_migrated[-1] @@ -1410,7 +1400,7 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): _delete_bdev_blocking(tgt_lvol_composite, tgt_rpc, secondary_rpc=None) return False, True, f"Last snapshot {last_snap_uuid} not found" tgt_snap_composite = f"{tgt_node.lvstore}/{_snap_tgt_short_name(last_snap)}" - else: + elif migration.snaps_preexisting_on_target: last_snap_uuid = migration.snaps_preexisting_on_target[-1] last_snap = db.get_snapshot_by_id(last_snap_uuid) if last_snap.lvol.node_id == tgt_node.get_id(): @@ -1425,11 +1415,12 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): if isinstance(snap_bdev, str): tgt_snap_composite = snap_bdev break - - if not tgt_snap_composite: - src_rpc.bdev_nvme_detach_controller(ctrl_name) - _delete_bdev_blocking(tgt_lvol_composite, tgt_rpc, secondary_rpc=None) - return False, True, f"snapshot {last_snap_uuid} not found on target" + if not tgt_snap_composite: + src_rpc.bdev_nvme_detach_controller(ctrl_name) + _delete_bdev_blocking(tgt_lvol_composite, tgt_rpc, secondary_rpc=None) + return False, True, f"snapshot {last_snap_uuid} not found on target" + # else: no snapshots at all — tgt_snap_composite stays "" and SPDK + # performs a full-lvol transfer without a snapshot base. # Step 5: start final migration — synchronous: blocks until SPDK completes # the IO drain and delta copy. Returns success/failure directly; no polling needed. From 6c92126805ccd2ad6545497ac65c14dce769e626 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Fri, 26 Jun 2026 15:32:46 +0330 Subject: [PATCH 10/49] remvoing forced snapshot creation on start_migration --- .../controllers/migration_controller.py | 6 ------ .../services/tasks_runner_lvol_migration.py | 18 ++++++++++-------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/simplyblock_core/controllers/migration_controller.py b/simplyblock_core/controllers/migration_controller.py index 60a2918dc..bda5e6dfa 100644 --- a/simplyblock_core/controllers/migration_controller.py +++ b/simplyblock_core/controllers/migration_controller.py @@ -121,12 +121,6 @@ def start_migration(migration_id, raise ValueError(f"Target node is not online (status={target_node.status})") snap_plan = get_snapshot_chain(lvol_id, source_node_id) - if not snap_plan: - snap_name = f"_mig_{migration.uuid[:8]}_lvol_{migration.lvol_id[:8]}" - snap_uuid, err = snapshot_controller.add(lvol_id, snap_name, bypass_migration_check=True) - if err: - raise ValueError(f"Failed to create snapshot: {err}") - snap_plan = [snap_uuid] snaps_found_on_target = [s for s in snap_plan if _is_snap_on_node(s, target_node_id)] snap_migration_plan = [s for s in snap_plan if s not in snaps_found_on_target] diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index 986272e49..ba15a5182 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -1104,16 +1104,18 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): _src_composite = f"{src_node.lvstore}/{_lvol.lvol_bdev}" _delta = _get_lvol_delta_bytes(src_rpc, _src_composite) _threshold = constants.LVOL_MIG_INTERMEDIATE_SNAP_THRESHOLD_BYTES - if _delta is not None and _delta <= _threshold: + if migration.intermediate_snap_rounds > 0 and _delta is not None and _delta <= _threshold: logger.info( f"Intermediate snapshot skipped: delta {convert_size(_delta, 'MiB')} MiB " f"<= {convert_size(_threshold, 'MiB')} MiB threshold " f"(round {migration.intermediate_snap_rounds}/{migration.max_intermediate_snap_rounds})") break - logger.info( - f"Intermediate snapshot triggered: delta " - f"{'unknown' if _delta is None else str(convert_size(_delta, 'MiB')) + ' MiB'} " - f"exceeds {convert_size(_threshold, 'MiB')} MiB threshold") + _reason = ( + "forced (round 0)" if migration.intermediate_snap_rounds == 0 + else f"delta {'unknown' if _delta is None else str(convert_size(_delta, 'MiB')) + ' MiB'} " + f"exceeds {convert_size(_threshold, 'MiB')} MiB threshold" + ) + logger.info(f"Intermediate snapshot triggered: {_reason}") _take_intermediate_snapshot(migration) plan = migration.snap_migration_plan if not plan: @@ -1387,9 +1389,9 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): return False, True, hub_err # Step 4: locate the last migrated snapshot's composite name on the target. - # If no snapshots were migrated (volume had no snapshots and delta was - # below the intermediate-snap threshold), pass an empty string — SPDK - # performs a full-lvol transfer without a snapshot base. + # At least one intermediate snapshot is always taken (round 0 is unconditional), + # so snaps_migrated is non-empty in the normal path. snaps_preexisting_on_target + # covers the case where a prior migration already placed immutable snapshots. tgt_snap_composite = "" if migration.snaps_migrated: last_snap_uuid = migration.snaps_migrated[-1] From 696c79a78e989987a08fa190775a74a139a6be5b Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Fri, 26 Jun 2026 15:42:41 +0330 Subject: [PATCH 11/49] fixing an issue with namespace not being added for tertiary node --- simplyblock_core/controllers/migration_controller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/simplyblock_core/controllers/migration_controller.py b/simplyblock_core/controllers/migration_controller.py index bda5e6dfa..d79e0e329 100644 --- a/simplyblock_core/controllers/migration_controller.py +++ b/simplyblock_core/controllers/migration_controller.py @@ -775,7 +775,7 @@ def create_migration(lvol_id, target_node_id, pass tgt_ter_node = None - if tgt_node.tertiary_node_id: + if lvol.ha_type == "ha3" and tgt_node.tertiary_node_id: tgt_ter_node = (_pre_ter_node if _pre_ter_node is not None else None) if tgt_ter_node is None: try: From 4746b1bf9f59741b19cf53264a8027fd537e65ed Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Fri, 26 Jun 2026 16:17:26 +0330 Subject: [PATCH 12/49] followup for fixing tertiary ns add issue --- simplyblock_core/controllers/migration_controller.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/simplyblock_core/controllers/migration_controller.py b/simplyblock_core/controllers/migration_controller.py index d79e0e329..3858aebb8 100644 --- a/simplyblock_core/controllers/migration_controller.py +++ b/simplyblock_core/controllers/migration_controller.py @@ -725,7 +725,7 @@ def create_migration(lvol_id, target_node_id, f"create_migration: secondary registration error (continuing): {_e}") _pre_ter_node = None - if lvol.ha_type == "ha3" and tgt_node.tertiary_node_id: + if tgt_node.tertiary_node_id: try: _pre_ter_node = db.get_storage_node_by_id(tgt_node.tertiary_node_id) _ter_rpc_reg = _pre_ter_node.rpc_client() @@ -775,7 +775,7 @@ def create_migration(lvol_id, target_node_id, pass tgt_ter_node = None - if lvol.ha_type == "ha3" and tgt_node.tertiary_node_id: + if tgt_node.tertiary_node_id: tgt_ter_node = (_pre_ter_node if _pre_ter_node is not None else None) if tgt_ter_node is None: try: From 665135204a81c0b523cfd3612293730c3b75b559 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Mon, 29 Jun 2026 11:45:23 +0330 Subject: [PATCH 13/49] fixing the delete logic on the migration --- .../services/tasks_runner_lvol_migration.py | 155 +++++++++++++----- 1 file changed, 110 insertions(+), 45 deletions(-) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index ba15a5182..497e17ae9 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -408,20 +408,24 @@ def _log_spdk_bdev_size(rpc, composite_name, label): def _delete_bdev_blocking(bdev_name, primary_rpc, secondary_rpc=None, tertiary_rpc=None): """ - Delete a bdev cleanly in a single RPC call. - - special_delete=True handles blob reference cleanup atomically — no - async-start/poll/finalize sequence needed. secondary/tertiary deletion is - best-effort (non-fatal if the node is unreachable). + Two-phase blocking bdev delete with special_delete=True (non-coalescing). + + Phase 1 — primary only, sync=False (del_async=False): initiates the + async special delete. special_delete=True tells SPDK to free the bdev's + own clusters without merging them into any child — correct for source + cleanup, rollback, and any path where no child needs to inherit data. + Phase 2 — all nodes (primary + secondary + tertiary), sync=True + (del_async=True): finalises the deletion on every replica. """ - ret, _ = primary_rpc.delete_lvol(bdev_name, special_delete=True) + ret, _ = primary_rpc.delete_lvol(bdev_name, del_async=False, special_delete=True) if not ret: - logger.warning(f"delete bdev {bdev_name}: special_delete failed (continuing)") - for extra_rpc in filter(None, [secondary_rpc, tertiary_rpc]): + logger.warning(f"delete bdev {bdev_name}: special_delete initiation failed (continuing)") + return + for rpc in filter(None, [primary_rpc, secondary_rpc, tertiary_rpc]): try: - extra_rpc.delete_lvol(bdev_name, special_delete=True) + rpc.delete_lvol(bdev_name, del_async=True, special_delete=True) except Exception as e: - logger.warning(f"delete bdev {bdev_name} on secondary/tertiary (non-fatal): {e}") + logger.warning(f"delete bdev {bdev_name} finalize on replica (non-fatal): {e}") # --------------------------------------------------------------------------- @@ -554,7 +558,8 @@ def _swap_namespace(rpc, nqn, new_bdev, uuid, guid, label): # --------------------------------------------------------------------------- -def _cleanup_final_migration(src_rpc, ctx, tgt_rpc=None, rollback_target=False): +def _cleanup_final_migration(src_rpc, ctx, tgt_rpc=None, rollback_target=False, + tgt_sec_rpc=None, tgt_ter_rpc=None): """Clean up after a final lvol migration attempt. On the success path (rollback_target=False) the hub controller is kept @@ -583,7 +588,8 @@ def _cleanup_final_migration(src_rpc, ctx, tgt_rpc=None, rollback_target=False): logger.warning(f"cleanup target subsystem {nqn}: {e}") if tgt_composite and tgt_rpc.get_bdevs(tgt_composite): try: - _delete_bdev_blocking(tgt_composite, tgt_rpc) + _delete_bdev_blocking(tgt_composite, tgt_rpc, + secondary_rpc=tgt_sec_rpc, tertiary_rpc=tgt_ter_rpc) except Exception as e: logger.warning(f"cleanup target lvol {tgt_composite}: {e}") @@ -961,7 +967,7 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): logger.info( f"Removing writable leftover target bdev {tgt_composite}") try: - _delete_bdev_blocking(tgt_composite, tgt_rpc, sec_rpc) + _delete_bdev_blocking(tgt_composite, tgt_rpc, sec_rpc, ter_rpc) for _ in range(10): if not tgt_rpc.get_bdevs(tgt_composite): break @@ -1159,7 +1165,8 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): if tgt_rpc.get_bdevs(tgt_composite): logger.info(f"Pre-cleanup: removing stale intermediate bdev {tgt_composite}") try: - _delete_bdev_blocking(tgt_composite, tgt_rpc) + _delete_bdev_blocking(tgt_composite, tgt_rpc, + secondary_rpc=sec_rpc, tertiary_rpc=ter_rpc) for _ in range(10): if not tgt_rpc.get_bdevs(tgt_composite): break @@ -1184,19 +1191,22 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): for _ in range(_INTERMEDIATE_POLL_MAX): result = src_rpc.bdev_lvol_transfer_stat(src_composite) if result is None: - _delete_bdev_blocking(tgt_composite, tgt_rpc, secondary_rpc=sec_rpc) + _delete_bdev_blocking(tgt_composite, tgt_rpc, + secondary_rpc=sec_rpc, tertiary_rpc=ter_rpc) return False, True, ( f"Transfer stat failed for intermediate snap {snap_uuid}") state = result.get('transfer_state', 'No process') if state == 'Done': break if state in ('Failed', 'No process'): - _delete_bdev_blocking(tgt_composite, tgt_rpc, secondary_rpc=sec_rpc) + _delete_bdev_blocking(tgt_composite, tgt_rpc, + secondary_rpc=sec_rpc, tertiary_rpc=ter_rpc) return False, True, ( f"Intermediate snap transfer {state} for {snap_uuid}") time.sleep(_INTERMEDIATE_POLL_INTERVAL_S) else: - _delete_bdev_blocking(tgt_composite, tgt_rpc, secondary_rpc=sec_rpc) + _delete_bdev_blocking(tgt_composite, tgt_rpc, + secondary_rpc=sec_rpc, tertiary_rpc=ter_rpc) return False, True, ( f"Intermediate snap transfer timed out for {snap_uuid}") @@ -1302,6 +1312,8 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): tgt_lvol_bdev = _lvol_tgt_bdev_name(lvol.lvol_bdev) tgt_lvol_composite = f"{tgt_node.lvstore}/{tgt_lvol_bdev}" ctx = migration.transfer_context or {} + tgt_sec_rpc = _get_secondary_rpc(tgt_node) + tgt_ter_rpc = _get_tertiary_rpc(tgt_node) # --- Crash recovery: Done handler was interrupted mid-run --- # bdev_lvol_final_migration is synchronous — it blocks until SPDK completes. @@ -1310,13 +1322,15 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): if ctx.get('stage') == 'transfer': result = src_rpc.bdev_lvol_transfer_stat(src_lvol_composite) if result is None: - _cleanup_final_migration(src_rpc, ctx, tgt_rpc, rollback_target=True) + _cleanup_final_migration(src_rpc, ctx, tgt_rpc, rollback_target=True, + tgt_sec_rpc=tgt_sec_rpc, tgt_ter_rpc=tgt_ter_rpc) migration.transfer_context = {} migration.write_to_db(db.kv_store) return False, True, "bdev_lvol_transfer_stat returned None (crash recovery)" state = result.get('transfer_state', 'No process') if state == 'Failed': - _cleanup_final_migration(src_rpc, ctx, tgt_rpc, rollback_target=True) + _cleanup_final_migration(src_rpc, ctx, tgt_rpc, rollback_target=True, + tgt_sec_rpc=tgt_sec_rpc, tgt_ter_rpc=tgt_ter_rpc) migration.transfer_context = {} migration.write_to_db(db.kv_store) return False, True, "Final migration Failed (crash recovery)" @@ -1399,7 +1413,8 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): last_snap = db.get_snapshot_by_id(last_snap_uuid) except KeyError: src_rpc.bdev_nvme_detach_controller(ctrl_name) - _delete_bdev_blocking(tgt_lvol_composite, tgt_rpc, secondary_rpc=None) + _delete_bdev_blocking(tgt_lvol_composite, tgt_rpc, + secondary_rpc=tgt_sec_rpc, tertiary_rpc=tgt_ter_rpc) return False, True, f"Last snapshot {last_snap_uuid} not found" tgt_snap_composite = f"{tgt_node.lvstore}/{_snap_tgt_short_name(last_snap)}" elif migration.snaps_preexisting_on_target: @@ -1419,7 +1434,8 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): break if not tgt_snap_composite: src_rpc.bdev_nvme_detach_controller(ctrl_name) - _delete_bdev_blocking(tgt_lvol_composite, tgt_rpc, secondary_rpc=None) + _delete_bdev_blocking(tgt_lvol_composite, tgt_rpc, + secondary_rpc=tgt_sec_rpc, tertiary_rpc=tgt_ter_rpc) return False, True, f"snapshot {last_snap_uuid} not found on target" # else: no snapshots at all — tgt_snap_composite stays "" and SPDK # performs a full-lvol transfer without a snapshot base. @@ -1650,7 +1666,7 @@ def _cleanup_subsystem_or_ns(nqn, ns_id, subsystem_was_created_by_migration, rpc rpc.subsystem_delete(nqn) -def _delete_intermediate_snaps_on_target(migration, tgt_rpc, tgt_sec_rpc=None): +def _delete_intermediate_snaps_on_target(migration, tgt_rpc, tgt_sec_rpc=None, tgt_ter_rpc=None): """ Delete migration-created intermediate ('shrink') snapshots from the target after a successful migration. @@ -1658,10 +1674,14 @@ def _delete_intermediate_snaps_on_target(migration, tgt_rpc, tgt_sec_rpc=None): Must be called AFTER apply_migration_to_db() — at that point snap.snap_bdev already holds the target composite path (e.g. LVS_TGT/SNAP_xxxm). - No migration flag is set before deletion so SPDK actually coalesces the - clusters into the child bdev and frees them. Deletion proceeds oldest-first - so each snapshot has at most one child when it is removed, satisfying SPDK's - snapshot-deletion constraint. + Two-phase coalescing delete: + Phase 1 — primary only, sync=False (del_async=False): initiates the + coalescing operation so SPDK merges the snapshot's clusters into its + child bdev before freeing the snapshot bdev. Using special_delete=True + here would do a non-coalescing unmap that frees the clusters directly, + breaking the child's parent chain and losing pre-migration data. + Phase 2 — all nodes (primary + secondary + tertiary), sync=True + (del_async=True): finalises the deletion on every replica. """ for snap_uuid in migration.intermediate_snaps: try: @@ -1672,16 +1692,32 @@ def _delete_intermediate_snaps_on_target(migration, tgt_rpc, tgt_sec_rpc=None): tgt_composite = snap.snap_bdev # updated to target path by apply_migration_to_db - if tgt_rpc.get_bdevs(tgt_composite): + if not tgt_rpc.get_bdevs(tgt_composite): + logger.info( + f"Intermediate snap bdev {tgt_composite} absent from target; skipping SPDK delete") + else: try: - _delete_bdev_blocking(tgt_composite, tgt_rpc, secondary_rpc=tgt_sec_rpc) - logger.info(f"Deleted intermediate snap bdev {tgt_composite} from target") + # Phase 1: initiate coalescing delete on primary (sync=False) + ret, _ = tgt_rpc.delete_lvol(tgt_composite, del_async=False, special_delete=False) + if not ret: + logger.warning(f"Coalescing delete initiation of {tgt_composite} on primary failed") + else: + # Phase 2: finalize on all nodes (sync=True) + for label, rpc in [("primary", tgt_rpc), + ("secondary", tgt_sec_rpc), + ("tertiary", tgt_ter_rpc)]: + if rpc is None: + continue + try: + rpc.delete_lvol(tgt_composite, del_async=True, special_delete=False) + except Exception as e: + logger.warning( + f"Could not finalize intermediate snap delete {tgt_composite} " + f"on {label}: {e}") + logger.info(f"Deleted intermediate snap bdev {tgt_composite} from target") except Exception as e: logger.warning( f"Could not delete intermediate snap {tgt_composite} from target: {e}") - else: - logger.info( - f"Intermediate snap bdev {tgt_composite} absent from target; skipping SPDK delete") try: snap.remove(db.kv_store) @@ -1691,7 +1727,7 @@ def _delete_intermediate_snaps_on_target(migration, tgt_rpc, tgt_sec_rpc=None): def _get_secondary_rpc(node): - """Return RPC client for node's secondary if reachable (online or suspended).""" + """Return RPC client for node's online secondary, or None.""" if not node.secondary_node_id: return None try: @@ -1703,6 +1739,19 @@ def _get_secondary_rpc(node): return None +def _get_tertiary_rpc(node): + """Return RPC client for node's online tertiary, or None.""" + if not node.tertiary_node_id: + return None + try: + ter = db.get_storage_node_by_id(node.tertiary_node_id) + if ter.status == StorageNode.STATUS_ONLINE: + return _make_rpc(ter) + except KeyError: + pass + return None + + _SKIP_CLEANUP_SOURCE = False # DEBUG: set True to skip source cleanup @@ -1730,7 +1779,9 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): tgt_lvol_bdev=_ctx.get('tgt_lvol_bdev')) if migration.intermediate_snaps: tgt_sec_rpc = _get_secondary_rpc(tgt_node) - _delete_intermediate_snaps_on_target(migration, tgt_rpc, tgt_sec_rpc) + _tgt_ter = _get_target_tertiary_node(tgt_node)[0] + tgt_ter_rpc = _make_rpc(_tgt_ter) if _tgt_ter else None + _delete_intermediate_snaps_on_target(migration, tgt_rpc, tgt_sec_rpc, tgt_ter_rpc) return True, False, None ctx = migration.transfer_context or {} @@ -1785,6 +1836,7 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): migration.write_to_db(db.kv_store) src_sec_rpc = _get_secondary_rpc(src_node) + src_ter_rpc = _get_tertiary_rpc(src_node) # --- Delete source snapshots (blocking, idempotent) --- # Recompute to_delete here so re-entries after crash work without stored pending list. @@ -1804,7 +1856,10 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): src_rpc.bdev_lvol_set_migration_flag(bdev_name) if src_sec_rpc: src_sec_rpc.bdev_lvol_set_migration_flag(bdev_name) - _delete_bdev_blocking(bdev_name, src_rpc, secondary_rpc=src_sec_rpc) + if src_ter_rpc: + src_ter_rpc.bdev_lvol_set_migration_flag(bdev_name) + _delete_bdev_blocking(bdev_name, src_rpc, + secondary_rpc=src_sec_rpc, tertiary_rpc=src_ter_rpc) logger.info(f"Deleted source bdev {bdev_name}") except KeyError: logger.warning(f"Source snapshot {snap_uuid} not found in DB; skipping") @@ -1849,13 +1904,17 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): # freeing the physical clusters — data now lives on the target. try: src_rpc.bdev_lvol_set_migration_flag(src_lvol_composite) + if src_sec_rpc: + src_sec_rpc.bdev_lvol_set_migration_flag(src_lvol_composite) + if src_ter_rpc: + src_ter_rpc.bdev_lvol_set_migration_flag(src_lvol_composite) except Exception as _mf_err: logger.warning( f"bdev_lvol_set_migration_flag for source lvol failed " f"(non-fatal): {_mf_err}") _delete_bdev_blocking( src_lvol_composite, src_rpc, - secondary_rpc=src_sec_rpc) + secondary_rpc=src_sec_rpc, tertiary_rpc=src_ter_rpc) logger.info(f"Deleted source lvol bdev {src_lvol_composite}") except Exception as e: logger.warning(f"Source lvol delete failed (non-fatal): {e}") @@ -1872,7 +1931,9 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): # coalesces and frees their clusters into the child bdev. if migration.intermediate_snaps: tgt_sec_rpc = _get_secondary_rpc(tgt_node) - _delete_intermediate_snaps_on_target(migration, tgt_rpc, tgt_sec_rpc) + _tgt_ter = _get_target_tertiary_node(tgt_node)[0] + tgt_ter_rpc = _make_rpc(_tgt_ter) if _tgt_ter else None + _delete_intermediate_snaps_on_target(migration, tgt_rpc, tgt_sec_rpc, tgt_ter_rpc) return True, False, None @@ -1894,6 +1955,7 @@ def _handle_cleanup_target(migration, tgt_node, tgt_rpc, src_rpc=None): ctx = migration.transfer_context or {} tgt_sec_rpc = _get_secondary_rpc(tgt_node) + tgt_ter_rpc = _get_tertiary_rpc(tgt_node) # --- Step 0: delete dangling target lvol/subsystems from a failed LVOL_MIGRATE --- # Also handles the pre-create case where bdev/subsystems were set up by @@ -1922,18 +1984,20 @@ def _handle_cleanup_target(migration, tgt_node, tgt_rpc, src_rpc=None): _cleanup_subsystem_or_ns(_nqn_to_clean, tgt_ns_id, subsystem_created_on_target, tgt_rpc) except Exception as e: logger.warning(f"cleanup target subsystem {_nqn_to_clean}: {e}") - if tgt_sec_rpc: - try: - _cleanup_subsystem_or_ns(_nqn_to_clean, tgt_ns_id, subsystem_created_on_target, - tgt_sec_rpc) - except Exception as e: - logger.warning(f"cleanup target secondary subsystem {_nqn_to_clean}: {e}") + for _label, _extra_rpc in [("secondary", tgt_sec_rpc), ("tertiary", tgt_ter_rpc)]: + if _extra_rpc: + try: + _cleanup_subsystem_or_ns(_nqn_to_clean, tgt_ns_id, + subsystem_created_on_target, _extra_rpc) + except Exception as e: + logger.warning(f"cleanup target {_label} subsystem {_nqn_to_clean}: {e}") # Delete target migration bdev — prefer ctx composite, fall back to derived name. _bdev_to_delete = tgt_lvol_composite or _pre_bdev if _bdev_to_delete and tgt_rpc.get_bdevs(_bdev_to_delete): try: - _delete_bdev_blocking(_bdev_to_delete, tgt_rpc, secondary_rpc=tgt_sec_rpc) + _delete_bdev_blocking(_bdev_to_delete, tgt_rpc, + secondary_rpc=tgt_sec_rpc, tertiary_rpc=tgt_ter_rpc) logger.info(f"Deleted target lvol {_bdev_to_delete}") except Exception as e: logger.warning(f"delete target lvol {_bdev_to_delete} (non-fatal): {e}") @@ -1954,7 +2018,8 @@ def _handle_cleanup_target(migration, tgt_node, tgt_rpc, src_rpc=None): if not tgt_rpc.get_bdevs(bdev_name): logger.info(f"Target bdev {bdev_name} not found; skipping (already cleaned up)") continue - _delete_bdev_blocking(bdev_name, tgt_rpc, secondary_rpc=tgt_sec_rpc) + _delete_bdev_blocking(bdev_name, tgt_rpc, + secondary_rpc=tgt_sec_rpc, tertiary_rpc=tgt_ter_rpc) logger.info(f"Deleted target snapshot bdev {bdev_name}") except KeyError: logger.warning(f"Target snapshot {snap_uuid} not found in DB; skipping") From 6d53c5ed912a59f2daa96bad815f9ddf8e27343b Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Mon, 29 Jun 2026 17:34:44 +0330 Subject: [PATCH 14/49] removed setting migration flag for snapshots to be deleted --- .../services/tasks_runner_lvol_migration.py | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index 497e17ae9..7744a666c 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -410,20 +410,29 @@ def _delete_bdev_blocking(bdev_name, primary_rpc, secondary_rpc=None, tertiary_r """ Two-phase blocking bdev delete with special_delete=True (non-coalescing). - Phase 1 — primary only, sync=False (del_async=False): initiates the - async special delete. special_delete=True tells SPDK to free the bdev's - own clusters without merging them into any child — correct for source - cleanup, rollback, and any path where no child needs to inherit data. + Phase 1 — primary only, sync=False (del_async=False): initiates the async + special delete. special_delete=True tells SPDK to free the bdev's own + clusters without merging them into any child — correct for source cleanup, + rollback, and any path where no child needs to inherit data. + Wait — poll bdev_lvol_get_lvol_delete_status on primary until done. Phase 2 — all nodes (primary + secondary + tertiary), sync=True - (del_async=True): finalises the deletion on every replica. + (del_async=True, special_delete=False): finalises the deletion on every replica. """ ret, _ = primary_rpc.delete_lvol(bdev_name, del_async=False, special_delete=True) if not ret: - logger.warning(f"delete bdev {bdev_name}: special_delete initiation failed (continuing)") + logger.warning(f"delete bdev {bdev_name}: initiation failed (continuing)") return + + while True: + status = primary_rpc.bdev_lvol_get_lvol_delete_status(bdev_name) + if status == 1: + time.sleep(0.2) + else: + break + for rpc in filter(None, [primary_rpc, secondary_rpc, tertiary_rpc]): try: - rpc.delete_lvol(bdev_name, del_async=True, special_delete=True) + rpc.delete_lvol(bdev_name, del_async=True, special_delete=False) except Exception as e: logger.warning(f"delete bdev {bdev_name} finalize on replica (non-fatal): {e}") @@ -1851,13 +1860,6 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): bdev_name = source_snap_bdevs[snap_uuid] else: bdev_name = f"{src_node.lvstore}/{_snap_short_name(snap)}" - # Mark as migration-source so SPDK drops only the blob reference - # without freeing the physical clusters (data lives on the target now). - src_rpc.bdev_lvol_set_migration_flag(bdev_name) - if src_sec_rpc: - src_sec_rpc.bdev_lvol_set_migration_flag(bdev_name) - if src_ter_rpc: - src_ter_rpc.bdev_lvol_set_migration_flag(bdev_name) _delete_bdev_blocking(bdev_name, src_rpc, secondary_rpc=src_sec_rpc, tertiary_rpc=src_ter_rpc) logger.info(f"Deleted source bdev {bdev_name}") From 589b9a2bdaaef2c914bdc85ccffc7fb4bdaded92 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Tue, 30 Jun 2026 12:01:47 +0330 Subject: [PATCH 15/49] TEMP: testing snapshot resume on retry instead of delete --- .../services/tasks_runner_lvol_migration.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index 7744a666c..38b002538 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -116,6 +116,11 @@ _INTERMEDIATE_POLL_INTERVAL_S = 1 # seconds between stat checks _INTERMEDIATE_POLL_MAX = 300 # max iterations ≈ 5 min +# Fault-injection: set _INJECT_SNAP_FAIL = True to make each snapshot's first +# transfer attempt fail before launching, then succeed on retry. Remove when done. +_INJECT_SNAP_FAIL = True +_snap_fail_injected: set = set() + def _now_ms(): """Return current wall-clock time as an ISO-8601 string with milliseconds.""" @@ -984,6 +989,14 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): except Exception as e: logger.warning(f"Pre-cleanup of {tgt_composite} failed (continuing): {e}") + if _INJECT_SNAP_FAIL and snap_uuid not in _snap_fail_injected: + _snap_fail_injected.add(snap_uuid) + migration.transfer_context = {} + migration.write_to_db(db.kv_store) + logger.warning( + f"[INJECT] Artificial snap transfer failure for {snap_uuid}") + return False, True, f"[INJECT] snap transfer failure for {snap_uuid}" + t, err = _setup_snap_transfer( snap, snap_index, src_node, tgt_node, src_rpc, tgt_rpc, trtype, From 1302994babb00e34e283bc94bfe09b1a673527b7 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Tue, 30 Jun 2026 13:29:04 +0330 Subject: [PATCH 16/49] TEMP: disabling the cleanup from source and intermediates --- .../services/tasks_runner_lvol_migration.py | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index 38b002538..c7ba1a942 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -116,10 +116,9 @@ _INTERMEDIATE_POLL_INTERVAL_S = 1 # seconds between stat checks _INTERMEDIATE_POLL_MAX = 300 # max iterations ≈ 5 min -# Fault-injection: set _INJECT_SNAP_FAIL = True to make each snapshot's first -# transfer attempt fail before launching, then succeed on retry. Remove when done. -_INJECT_SNAP_FAIL = True -_snap_fail_injected: set = set() +# Test flags — remove when done +_SKIP_CLEANUP_SOURCE = True +_SKIP_INTERMEDIATE_SNAP_DELETE = True def _now_ms(): @@ -989,14 +988,6 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): except Exception as e: logger.warning(f"Pre-cleanup of {tgt_composite} failed (continuing): {e}") - if _INJECT_SNAP_FAIL and snap_uuid not in _snap_fail_injected: - _snap_fail_injected.add(snap_uuid) - migration.transfer_context = {} - migration.write_to_db(db.kv_store) - logger.warning( - f"[INJECT] Artificial snap transfer failure for {snap_uuid}") - return False, True, f"[INJECT] snap transfer failure for {snap_uuid}" - t, err = _setup_snap_transfer( snap, snap_index, src_node, tgt_node, src_rpc, tgt_rpc, trtype, @@ -1799,7 +1790,7 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): migration, tgt_lvol_uuid=_ctx.get('tgt_lvol_uuid'), tgt_lvol_bdev=_ctx.get('tgt_lvol_bdev')) - if migration.intermediate_snaps: + if migration.intermediate_snaps and not _SKIP_INTERMEDIATE_SNAP_DELETE: tgt_sec_rpc = _get_secondary_rpc(tgt_node) _tgt_ter = _get_target_tertiary_node(tgt_node)[0] tgt_ter_rpc = _make_rpc(_tgt_ter) if _tgt_ter else None @@ -1944,7 +1935,7 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): # Delete intermediate (shrink) snapshots from the target — they are migration # artifacts and do not need to be preserved. No migration flag so SPDK # coalesces and frees their clusters into the child bdev. - if migration.intermediate_snaps: + if migration.intermediate_snaps and not _SKIP_INTERMEDIATE_SNAP_DELETE: tgt_sec_rpc = _get_secondary_rpc(tgt_node) _tgt_ter = _get_target_tertiary_node(tgt_node)[0] tgt_ter_rpc = _make_rpc(_tgt_ter) if _tgt_ter else None From 6ecbcec9846137d9dba5bf8e2202c7b5c6aec24d Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Tue, 30 Jun 2026 23:20:52 +0330 Subject: [PATCH 17/49] TEMP: disabling the cleanup source --- simplyblock_core/services/tasks_runner_lvol_migration.py | 1 - 1 file changed, 1 deletion(-) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index c7ba1a942..d46aadd88 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -1767,7 +1767,6 @@ def _get_tertiary_rpc(node): -_SKIP_CLEANUP_SOURCE = False # DEBUG: set True to skip source cleanup def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): From fb9c75462eea3339cde512db02d903b703c98dcb Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Fri, 3 Jul 2026 17:09:36 +0330 Subject: [PATCH 18/49] setting the spdk image ref for debugging --- simplyblock_core/env_var | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/simplyblock_core/env_var b/simplyblock_core/env_var index 5abdecd44..b970b118b 100644 --- a/simplyblock_core/env_var +++ b/simplyblock_core/env_var @@ -2,4 +2,4 @@ SIMPLY_BLOCK_COMMAND_NAME=sbcli-dev SIMPLY_BLOCK_VERSION=19.2.34 SIMPLY_BLOCK_DOCKER_IMAGE=public.ecr.aws/simply-block/simplyblock:main -SIMPLY_BLOCK_SPDK_ULTRA_IMAGE=public.ecr.aws/simply-block/ultra:main-latest +SIMPLY_BLOCK_SPDK_ULTRA_IMAGE=public.ecr.aws/simply-block/debuge-migration From 05c0e7b4eded48f5ae89bba973f0eccb8bed7d20 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Fri, 3 Jul 2026 17:45:01 +0330 Subject: [PATCH 19/49] fixing the image ref for spdk --- simplyblock_core/env_var | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/simplyblock_core/env_var b/simplyblock_core/env_var index b970b118b..9d3307a59 100644 --- a/simplyblock_core/env_var +++ b/simplyblock_core/env_var @@ -2,4 +2,4 @@ SIMPLY_BLOCK_COMMAND_NAME=sbcli-dev SIMPLY_BLOCK_VERSION=19.2.34 SIMPLY_BLOCK_DOCKER_IMAGE=public.ecr.aws/simply-block/simplyblock:main -SIMPLY_BLOCK_SPDK_ULTRA_IMAGE=public.ecr.aws/simply-block/debuge-migration +SIMPLY_BLOCK_SPDK_ULTRA_IMAGE=public.ecr.aws/simply-block/debuge-migration-latest From 4231193d144cb34d7a25fd12bb39a8951db56e08 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Fri, 3 Jul 2026 18:12:01 +0330 Subject: [PATCH 20/49] fixing the image ref for spdk again --- simplyblock_core/env_var | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/simplyblock_core/env_var b/simplyblock_core/env_var index 9d3307a59..e498d47fd 100644 --- a/simplyblock_core/env_var +++ b/simplyblock_core/env_var @@ -2,4 +2,4 @@ SIMPLY_BLOCK_COMMAND_NAME=sbcli-dev SIMPLY_BLOCK_VERSION=19.2.34 SIMPLY_BLOCK_DOCKER_IMAGE=public.ecr.aws/simply-block/simplyblock:main -SIMPLY_BLOCK_SPDK_ULTRA_IMAGE=public.ecr.aws/simply-block/debuge-migration-latest +SIMPLY_BLOCK_SPDK_ULTRA_IMAGE=public.ecr.aws/simply-block/ultra:debuge-migration-latest From 9b06344b519a88f02e074823ebce2fe36f2dcc61 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Sat, 4 Jul 2026 07:51:42 +0330 Subject: [PATCH 21/49] setting the env_var to main --- simplyblock_core/constants.py | 2 +- simplyblock_core/env_var | 2 +- .../services/tasks_runner_lvol_migration.py | 94 ++++++++++++++----- 3 files changed, 72 insertions(+), 26 deletions(-) diff --git a/simplyblock_core/constants.py b/simplyblock_core/constants.py index bc7336146..d8ab33144 100644 --- a/simplyblock_core/constants.py +++ b/simplyblock_core/constants.py @@ -370,7 +370,7 @@ def get_config_var(name, default=None): # Live volume migration constants LVOL_MIG_MAX_RETRIES = 5 # max retry attempts before aborting -LVOL_MIG_DEADLINE_SEC = 360 # default 4-hour deadline (0 = no deadline) +LVOL_MIG_DEADLINE_SEC = 3600 # 1-hour deadline (0 = no deadline) LVOL_MIG_MAX_INTERMEDIATE_SNAPS = 3 # max recursive "shrink" snapshot rounds LVOL_MIG_INTERMEDIATE_SNAP_THRESHOLD_BYTES = 500 * 1024 * 1024 # 500 MiB — skip if delta is smaller LVOL_MIG_BDEV_SUFFIX = 'm' # appended to every migration bdev on the target to avoid collision with real bdevs diff --git a/simplyblock_core/env_var b/simplyblock_core/env_var index e498d47fd..5abdecd44 100644 --- a/simplyblock_core/env_var +++ b/simplyblock_core/env_var @@ -2,4 +2,4 @@ SIMPLY_BLOCK_COMMAND_NAME=sbcli-dev SIMPLY_BLOCK_VERSION=19.2.34 SIMPLY_BLOCK_DOCKER_IMAGE=public.ecr.aws/simply-block/simplyblock:main -SIMPLY_BLOCK_SPDK_ULTRA_IMAGE=public.ecr.aws/simply-block/ultra:debuge-migration-latest +SIMPLY_BLOCK_SPDK_ULTRA_IMAGE=public.ecr.aws/simply-block/ultra:main-latest diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index d46aadd88..584b97a9a 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -116,9 +116,8 @@ _INTERMEDIATE_POLL_INTERVAL_S = 1 # seconds between stat checks _INTERMEDIATE_POLL_MAX = 300 # max iterations ≈ 5 min -# Test flags — remove when done -_SKIP_CLEANUP_SOURCE = True -_SKIP_INTERMEDIATE_SNAP_DELETE = True +_SKIP_CLEANUP_SOURCE = False +_SKIP_INTERMEDIATE_SNAP_DELETE = False def _now_ms(): @@ -410,7 +409,8 @@ def _log_spdk_bdev_size(rpc, composite_name, label): return None -def _delete_bdev_blocking(bdev_name, primary_rpc, secondary_rpc=None, tertiary_rpc=None): +def _delete_bdev_blocking(bdev_name, primary_rpc, secondary_rpc=None, tertiary_rpc=None, + timeout_s=120): """ Two-phase blocking bdev delete with special_delete=True (non-coalescing). @@ -427,12 +427,20 @@ def _delete_bdev_blocking(bdev_name, primary_rpc, secondary_rpc=None, tertiary_r logger.warning(f"delete bdev {bdev_name}: initiation failed (continuing)") return + deadline = time.monotonic() + timeout_s while True: status = primary_rpc.bdev_lvol_get_lvol_delete_status(bdev_name) - if status == 1: - time.sleep(0.2) - else: + if status != 1: break + if time.monotonic() > deadline: + if not primary_rpc.get_bdevs(bdev_name): + logger.warning( + f"delete bdev {bdev_name}: poll timed out after {timeout_s}s " + f"but bdev is gone — treating as success") + break + raise RuntimeError( + f"delete bdev {bdev_name}: timed out after {timeout_s}s, bdev still present") + time.sleep(0.2) for rpc in filter(None, [primary_rpc, secondary_rpc, tertiary_rpc]): try: @@ -1554,27 +1562,51 @@ def _flip(rpc, ip, port, trtype, state, label): rpc.nvmf_subsystem_listener_set_ana_state( nqn, ip, port, trtype=trtype, ana=state) logger.info(f"ANA {nqn} {label} {ip}:{port} → {state}") + return True except Exception as e: - logger.error(f"ANA {label} failed (non-fatal): {e}") + logger.error(f"ANA {label} failed: {e}") + return False + + def _flip_required(rpc, ip, port, trtype, state, label, attempts=3): + for i in range(attempts): + if _flip(rpc, ip, port, trtype, state, label): + return True + if i < attempts - 1: + time.sleep(1.0) + return False if not overlap_ids: - # Step 1 (no-overlap): set all TGT to final states before killing SRC - for i, tgt in enumerate(tgt_paths): - state = "optimized" if i == 0 else "non_optimized" + # Step 1 (no-overlap): TGT primary must be confirmed optimized before + # making SRC inaccessible — otherwise clients lose all I/O paths. + primary_tgt = tgt_paths[0] + if not _flip_required(primary_tgt['rpc'], primary_tgt['ip'], primary_tgt['port'], + primary_tgt['trtype'], "optimized", + f"TGT-{primary_tgt['node_id'][:8]}"): + return False, False, ( + "ANA flip: TGT primary→optimized failed after retries " + "— aborting to keep SRC paths accessible") + + # Secondary TGT paths best-effort — clients survive without them + for tgt in tgt_paths[1:]: _flip(tgt['rpc'], tgt['ip'], tgt['port'], tgt['trtype'], - state, f"TGT-{tgt['node_id'][:8]}") + "non_optimized", f"TGT-{tgt['node_id'][:8]}") # Step 3 (no-overlap): all SRC paths → inaccessible for src in src_paths: _flip(src['rpc'], src['ip'], src['port'], src['trtype'], "inaccessible", f"SRC-{src['node_id'][:8]}") else: - # Step 1: first non-overlap TGT → optimized (live path before touching overlap) - for tgt in tgt_paths: - if tgt['node_id'] not in overlap_ids: - _flip(tgt['rpc'], tgt['ip'], tgt['port'], tgt['trtype'], - "optimized", f"TGT-{tgt['node_id'][:8]}(pre)") - break + # Step 1: first non-overlap TGT → optimized. Must succeed before + # making any SRC path inaccessible. + non_overlap_tgt = next( + (t for t in tgt_paths if t['node_id'] not in overlap_ids), None) + if non_overlap_tgt: + if not _flip_required(non_overlap_tgt['rpc'], non_overlap_tgt['ip'], + non_overlap_tgt['port'], non_overlap_tgt['trtype'], + "optimized", f"TGT-{non_overlap_tgt['node_id'][:8]}(pre)"): + return False, False, ( + "ANA flip: non-overlap TGT primary→optimized failed after retries " + "— aborting to keep SRC paths accessible") # Step 2: overlap SRC paths → inaccessible at SRC port for src in src_paths: @@ -1599,11 +1631,17 @@ def _flip(rpc, ip, port, trtype, state, label): except Exception as e: logger.error(f"Namespace swap on {tgt['node_id'][:8]} failed: {e}") - # Step 5: all TGT paths → correct ANA state at TGT port - for i, tgt in enumerate(tgt_paths): - state = "optimized" if i == 0 else "non_optimized" + # Step 5: all TGT paths → correct ANA state at TGT port. + # Primary required; secondaries best-effort. + primary_tgt = tgt_paths[0] + if not _flip_required(primary_tgt['rpc'], primary_tgt['ip'], primary_tgt['port'], + primary_tgt['trtype'], "optimized", + f"TGT-{primary_tgt['node_id'][:8]}"): + return False, False, ( + "ANA flip: TGT primary→optimized (step 5) failed after retries") + for tgt in tgt_paths[1:]: _flip(tgt['rpc'], tgt['ip'], tgt['port'], tgt['trtype'], - state, f"TGT-{tgt['node_id'][:8]}") + "non_optimized", f"TGT-{tgt['node_id'][:8]}") # Step 6: overlap TGT paths → remove old SRC listener if port changed for tgt in tgt_paths: @@ -1814,6 +1852,11 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): # bdevs even after apply_migration_to_db renamed them to target names in DB. source_snap_bdevs_saved = ctx.get('source_snap_bdevs', {}) source_lvol_bdev_saved = ctx.get('source_lvol_bdev', '') + if not source_lvol_bdev_saved: + return False, False, ( + "source_lvol_bdev missing from transfer_context — cannot safely " + "identify source bdev; apply_migration_to_db may have already " + "renamed lvol.lvol_bdev to the target name") to_delete = migration_controller.get_snaps_safe_to_delete_on_source(migration) @@ -1902,8 +1945,11 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): try: # Use the saved pre-apply bdev name; apply_migration_to_db already # renamed lvol.lvol_bdev to the target 'm'-suffix name in DB. - _raw_src_bdev = ctx.get('source_lvol_bdev') - src_bdev_short = _raw_src_bdev if isinstance(_raw_src_bdev, str) and _raw_src_bdev else lvol.lvol_bdev + src_bdev_short = ctx.get('source_lvol_bdev') + if not src_bdev_short: + raise RuntimeError( + "source_lvol_bdev missing from ctx at delete site — " + "refusing to fall back to lvol.lvol_bdev which may point to target") src_lvol_composite = f"{src_node.lvstore}/{src_bdev_short}" # Set migration flag so SPDK drops only the blob reference without # freeing the physical clusters — data now lives on the target. From 53ac4d9939b78e0363b024c01019dc1d1d8d705c Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Sat, 4 Jul 2026 19:32:36 +0330 Subject: [PATCH 22/49] removing the status cutover as it's no longer needed --- simplyblock_core/models/lvol_migration.py | 7 +------ simplyblock_core/services/tasks_runner_lvol_migration.py | 6 ------ 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/simplyblock_core/models/lvol_migration.py b/simplyblock_core/models/lvol_migration.py index 367a5654c..2b52673b4 100644 --- a/simplyblock_core/models/lvol_migration.py +++ b/simplyblock_core/models/lvol_migration.py @@ -30,10 +30,6 @@ class LVolMigration(BaseModel): STATUS_NEW = 'new' STATUS_RUNNING = 'running' STATUS_SUSPENDED = 'suspended' - # CUTOVER: the target subsystem is live and the client must reconnect before - # the source subsystem is removed. Set by PHASE_LVOL_MIGRATE (tgt_is_src_secondary - # path only); cleared to STATUS_DONE once PHASE_CLEANUP_SOURCE completes. - STATUS_CUTOVER = 'cutover' STATUS_DONE = 'done' STATUS_FAILED = 'failed' STATUS_CANCELLED = 'cancelled' @@ -49,7 +45,6 @@ class LVolMigration(BaseModel): STATUS_NEW: 0, STATUS_RUNNING: 1, STATUS_SUSPENDED: 2, - STATUS_CUTOVER: 6, STATUS_DONE: 3, STATUS_FAILED: 4, STATUS_CANCELLED: 5, @@ -124,7 +119,7 @@ def write_to_db(self, kv_store=None): def is_active(self): return self.status in ( self.STATUS_NEW, self.STATUS_RUNNING, - self.STATUS_SUSPENDED, self.STATUS_CUTOVER, + self.STATUS_SUSPENDED, ) def has_deadline_passed(self): diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index 584b97a9a..39f829325 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -1836,12 +1836,6 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): ctx = migration.transfer_context or {} - # --- Cutover grace period --- - # Wait here until the client has had time to reconnect to the new TGT - # subsystem before we tear down the source subsystem. The grace period - # begins when PHASE_LVOL_MIGRATE sets cutover_notified_at and changes - # migration.status to STATUS_CUTOVER — the test script polls for that - # status and triggers client reconnect immediately upon seeing it. # --- First entry: initialize cleanup state --- if ctx.get('stage') != 'cleanup_src': # Preserve the target lvol UUID and bdev name written by PHASE_LVOL_MIGRATE From 93d7450e23373d436e45c02c820aa8b574f98b06 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Sat, 4 Jul 2026 22:53:47 +0330 Subject: [PATCH 23/49] fixing ndcs and npcs --- simplyblock_core/services/tasks_runner_lvol_migration.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index 39f829325..02ef5f005 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -652,7 +652,9 @@ def _setup_snap_transfer(snap, snap_index, src_node, tgt_node, f"size_in_mib={size_in_mib} (lvol_size_mib={lvol_size_mib})" ) _log_spdk_bdev_size(src_rpc, src_composite, f"SRC snap[{snap_uuid[:8]}] pre-create") - ret = tgt_rpc.create_lvol(snap_short, size_in_mib, tgt_node.lvstore) + _ndcs = snap.lvol.ndcs if hasattr(snap, 'lvol') and snap.lvol else 0 + _npcs = snap.lvol.npcs if hasattr(snap, 'lvol') and snap.lvol else 0 + ret = tgt_rpc.create_lvol(snap_short, size_in_mib, tgt_node.lvstore, ndcs=_ndcs, npcs=_npcs) if not ret: return None, f"Failed to create target lvol for snap {snap_uuid}" _log_spdk_bdev_size(tgt_rpc, tgt_composite, f"TGT snap[{snap_uuid[:8]}] post-create") @@ -2055,7 +2057,9 @@ def _handle_cleanup_target(migration, tgt_node, tgt_rpc, src_rpc=None): # Reverse order: children/leaves before parents/roots (SPDK open-ref constraint). # _delete_bdev_blocking handles "not found" (status 2) gracefully, so a # crash-recovery re-run that re-deletes already-removed bdevs is safe. - for snap_uuid in reversed(migration.snaps_migrated): + # Uses get_snaps_to_delete_on_target to skip pre-existing snaps and snaps + # still referenced by other lvols already on the target node. + for snap_uuid in reversed(migration_controller.get_snaps_to_delete_on_target(migration)): try: snap = db.get_snapshot_by_id(snap_uuid) snap_short = _snap_tgt_short_name(snap) From fe73f8a3891284b1d8d9dff65d7bf8ada8fe392e Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Sat, 4 Jul 2026 23:03:22 +0330 Subject: [PATCH 24/49] setting env_var for spdk image --- simplyblock_core/env_var | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/simplyblock_core/env_var b/simplyblock_core/env_var index 5abdecd44..e498d47fd 100644 --- a/simplyblock_core/env_var +++ b/simplyblock_core/env_var @@ -2,4 +2,4 @@ SIMPLY_BLOCK_COMMAND_NAME=sbcli-dev SIMPLY_BLOCK_VERSION=19.2.34 SIMPLY_BLOCK_DOCKER_IMAGE=public.ecr.aws/simply-block/simplyblock:main -SIMPLY_BLOCK_SPDK_ULTRA_IMAGE=public.ecr.aws/simply-block/ultra:main-latest +SIMPLY_BLOCK_SPDK_ULTRA_IMAGE=public.ecr.aws/simply-block/ultra:debuge-migration-latest From f4c08ebfd68243c6ee617697ace3d86ff46d2e5f Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Sat, 4 Jul 2026 23:23:43 +0330 Subject: [PATCH 25/49] disabling cleanup after migration for testing --- simplyblock_core/services/tasks_runner_lvol_migration.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index 02ef5f005..19e220d70 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -116,8 +116,8 @@ _INTERMEDIATE_POLL_INTERVAL_S = 1 # seconds between stat checks _INTERMEDIATE_POLL_MAX = 300 # max iterations ≈ 5 min -_SKIP_CLEANUP_SOURCE = False -_SKIP_INTERMEDIATE_SNAP_DELETE = False +_SKIP_CLEANUP_SOURCE = True +_SKIP_INTERMEDIATE_SNAP_DELETE = True def _now_ms(): From 1df670b928f961a371ea0ab5d118fbd9d8d6e965 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Sun, 5 Jul 2026 01:58:08 +0330 Subject: [PATCH 26/49] adding rename logic to fix the issue with migrating the same lvol over and over again --- .../controllers/migration_controller.py | 12 +- simplyblock_core/rpc_client.py | 4 + .../services/tasks_runner_lvol_migration.py | 194 ++++++++++++++++-- 3 files changed, 188 insertions(+), 22 deletions(-) diff --git a/simplyblock_core/controllers/migration_controller.py b/simplyblock_core/controllers/migration_controller.py index 3858aebb8..7f5075955 100644 --- a/simplyblock_core/controllers/migration_controller.py +++ b/simplyblock_core/controllers/migration_controller.py @@ -406,11 +406,21 @@ def _add(uid): def _is_snap_on_node(snap_id, node_id): - """Return True if *snap_id* already has an instance on *node_id*.""" + """Return True if *snap_id* already resides on *node_id*. + + Checks two sources: + - snap.lvol.node_id: set to the current node by apply_migration_to_db after + a local migration — this is the primary signal for re-migrations. + - snap.instances: cross-cluster replicated copies (populated for remote snapshots). + """ try: snap = db.get_snapshot_by_id(snap_id) except KeyError: return False + # Local migration: parent lvol was moved to this node + if snap.lvol and snap.lvol.node_id == node_id: + return True + # Cross-cluster replica return any( inst.get('lvol', {}).get('node_id') == node_id for inst in (snap.instances or []) diff --git a/simplyblock_core/rpc_client.py b/simplyblock_core/rpc_client.py index 19a6379e0..e5e26d5be 100755 --- a/simplyblock_core/rpc_client.py +++ b/simplyblock_core/rpc_client.py @@ -1309,6 +1309,10 @@ def bdev_lvol_get_lvstores(self, name): params = {"lvs_name": name} return self._request("bdev_lvol_get_lvstores", params) + def bdev_lvol_rename(self, old_name, new_name): + params = {"old_name": old_name, "new_name": new_name} + return self._request("bdev_lvol_rename", params) + def bdev_lvol_resize(self, name, size_in_mib): params = { "name": name, diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index 19e220d70..c6db35caf 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -116,8 +116,8 @@ _INTERMEDIATE_POLL_INTERVAL_S = 1 # seconds between stat checks _INTERMEDIATE_POLL_MAX = 300 # max iterations ≈ 5 min -_SKIP_CLEANUP_SOURCE = True -_SKIP_INTERMEDIATE_SNAP_DELETE = True +_SKIP_CLEANUP_SOURCE = False +_SKIP_INTERMEDIATE_SNAP_DELETE = False def _now_ms(): @@ -140,6 +140,10 @@ def _get_migration_nic(node): _MIGRATION_BDEV_SUFFIX = constants.LVOL_MIG_BDEV_SUFFIX +# Suffix applied when the canonical (no-suffix) name is already taken on the +# target after migration. "am" = "after migration" — stays distinct from the +# in-flight 'm' suffix so pre-existing migrated snapshots remain identifiable. +_MIGRATION_BDEV_SUFFIX_DONE = 'am' def _apply_migration_to_db(migration, tgt_lvol_uuid=None, tgt_lvol_bdev=None): @@ -904,10 +908,16 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): _s = db.get_snapshot_by_id(_snap_uuid) except KeyError: continue - _short_tgt = _snap_tgt_short_name(_s) - if _short_tgt in _tgt_immutable: + _short_tgt = _snap_tgt_short_name(_s) # SNAP_Xm (in-flight) + _short_canonical = _snap_short_name(_s) # SNAP_X (post-rename) + _short_am = _short_canonical + _MIGRATION_BDEV_SUFFIX_DONE # SNAP_Xam (fallback) + _found_as = next( + (n for n in (_short_tgt, _short_canonical, _short_am) + if n in _tgt_immutable), + None) + if _found_as: logger.info( - f"Pre-scan: {_snap_uuid} ({_short_tgt}) is already an immutable " + f"Pre-scan: {_snap_uuid} ({_found_as}) is already an immutable " f"snapshot on target; marking pre-existing") migration.snaps_preexisting_on_target.append(_snap_uuid) _pre_scan_updated = True @@ -1809,6 +1819,125 @@ def _get_tertiary_rpc(node): +def _rename_migrated_bdevs(migration, tgt_node, tgt_rpc, tgt_sec_rpc=None, tgt_ter_rpc=None): + """ + After migration completes, rename 'm'-suffixed bdevs on the target back to + their canonical names (without the suffix). This prevents suffix accumulation + on repeated migrations of the same volume. + + A pre-check ensures we only rename when the canonical name is free on the + target. If the canonical name already exists the 'm' name is kept as-is so + both snapshots can coexist without conflict. + + Must be called AFTER _apply_migration_to_db() — snap.snap_bdev and + lvol.lvol_bdev already point to the 'm'-suffixed target paths at that point. + """ + lvstore = tgt_node.lvstore + preexisting = set(migration.snaps_preexisting_on_target or []) + + try: + existing_bdevs = { + e.get('name', '').split('/')[-1] + for e in (tgt_rpc.bdev_lvol_get_lvols(lvstore) or []) + } + except Exception as exc: + logger.warning(f"_rename_migrated_bdevs: could not query target bdevs: {exc}") + return + + def _do_rename(old_composite, new_composite, label): + tgt_rpc.bdev_lvol_rename(old_composite, new_composite) + for role, rpc in [("sec", tgt_sec_rpc), ("ter", tgt_ter_rpc)]: + if rpc: + try: + rpc.bdev_lvol_rename(old_composite, new_composite) + except Exception as exc: + logger.warning( + f"_rename_migrated_bdevs: {role} rename {label} (non-fatal): {exc}") + + # --- Snapshots --- + for snap_uuid in migration.snaps_migrated: + if snap_uuid in preexisting: + continue + try: + snap = db.get_snapshot_by_id(snap_uuid) + except KeyError: + continue + + snap_bdev = snap.snap_bdev # e.g. LVS_TGT/SNAP_Xm + if '/' not in snap_bdev: + continue + current_short = snap_bdev.split('/', 1)[1] + if not current_short.endswith(_MIGRATION_BDEV_SUFFIX): + continue # already canonical + + canonical_short = current_short[:-len(_MIGRATION_BDEV_SUFFIX)] + if canonical_short in existing_bdevs: + fallback_short = canonical_short + _MIGRATION_BDEV_SUFFIX_DONE + if fallback_short in existing_bdevs: + logger.warning( + f"_rename_migrated_bdevs: both {canonical_short} and {fallback_short} " + f"exist — cannot rename {current_short}, leaving as-is") + continue + target_short = fallback_short + logger.info( + f"_rename_migrated_bdevs: {canonical_short} taken — renaming {current_short} → {fallback_short}") + else: + target_short = canonical_short + + try: + _do_rename(f"{lvstore}/{current_short}", f"{lvstore}/{target_short}", current_short) + snap.snap_bdev = f"{lvstore}/{target_short}" + snap.write_to_db(db.kv_store) + existing_bdevs.discard(current_short) + existing_bdevs.add(target_short) + logger.info(f"_rename_migrated_bdevs: snap {current_short} → {target_short}") + except Exception as exc: + logger.warning( + f"_rename_migrated_bdevs: snap rename {current_short} failed: {exc}") + + # --- Lvol --- + try: + lvol = db.get_lvol_by_id(migration.lvol_id) + except KeyError: + logger.warning(f"_rename_migrated_bdevs: lvol {migration.lvol_id} not found") + return + + current_lvol_short = lvol.lvol_bdev + if not current_lvol_short.endswith(_MIGRATION_BDEV_SUFFIX): + return # already canonical + + canonical_lvol_short = current_lvol_short[:-len(_MIGRATION_BDEV_SUFFIX)] + if canonical_lvol_short in existing_bdevs: + fallback_lvol_short = canonical_lvol_short + _MIGRATION_BDEV_SUFFIX_DONE + if fallback_lvol_short in existing_bdevs: + logger.warning( + f"_rename_migrated_bdevs: both {canonical_lvol_short} and {fallback_lvol_short} " + f"exist — cannot rename {current_lvol_short}, leaving as-is") + return + target_lvol_short = fallback_lvol_short + logger.info( + f"_rename_migrated_bdevs: {canonical_lvol_short} taken — renaming {current_lvol_short} → {fallback_lvol_short}") + else: + target_lvol_short = canonical_lvol_short + + try: + _do_rename( + f"{lvstore}/{current_lvol_short}", + f"{lvstore}/{target_lvol_short}", + current_lvol_short) + lvol.lvol_bdev = target_lvol_short + lvol.top_bdev = f"{lvstore}/{target_lvol_short}" + for entry in lvol.bdev_stack: + if (entry.get('type') == 'bdev_lvol' + and entry.get('params', {}).get('name') == current_lvol_short): + entry['params']['name'] = target_lvol_short + lvol.write_to_db(db.kv_store) + logger.info(f"_rename_migrated_bdevs: lvol {current_lvol_short} → {target_lvol_short}") + except Exception as exc: + logger.warning( + f"_rename_migrated_bdevs: lvol rename failed: {exc}") + + def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): """ Delete snapshots from the source node that are exclusively owned by the @@ -1829,11 +1958,12 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): migration, tgt_lvol_uuid=_ctx.get('tgt_lvol_uuid'), tgt_lvol_bdev=_ctx.get('tgt_lvol_bdev')) + _tgt_sec_rpc = _get_secondary_rpc(tgt_node) + _tgt_ter = _get_target_tertiary_node(tgt_node)[0] + _tgt_ter_rpc = _make_rpc(_tgt_ter) if _tgt_ter else None if migration.intermediate_snaps and not _SKIP_INTERMEDIATE_SNAP_DELETE: - tgt_sec_rpc = _get_secondary_rpc(tgt_node) - _tgt_ter = _get_target_tertiary_node(tgt_node)[0] - tgt_ter_rpc = _make_rpc(_tgt_ter) if _tgt_ter else None - _delete_intermediate_snaps_on_target(migration, tgt_rpc, tgt_sec_rpc, tgt_ter_rpc) + _delete_intermediate_snaps_on_target(migration, tgt_rpc, _tgt_sec_rpc, _tgt_ter_rpc) + _rename_migrated_bdevs(migration, tgt_node, tgt_rpc, _tgt_sec_rpc, _tgt_ter_rpc) return True, False, None ctx = migration.transfer_context or {} @@ -1857,17 +1987,25 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): to_delete = migration_controller.get_snaps_safe_to_delete_on_source(migration) # Verify each snapshot to be deleted physically exists on the target - # before we remove anything from the source. Target bdevs carry the - # migration suffix (e.g. SNAP_xxxm) — derive from source short name. + # before we remove anything from the source. Bdevs may carry the + # migration suffix (SNAP_Xm), canonical name (SNAP_X), or the + # post-rename fallback (SNAP_Xam) — check all three so that a + # crash-recovery re-run after a partial rename still passes. tgt_lvols = tgt_rpc.bdev_lvol_get_lvols(tgt_node.lvstore) or [] tgt_names = {e.get('name', '').split('/')[-1] for e in tgt_lvols} for snap_uuid in to_delete: try: snap = db.get_snapshot_by_id(snap_uuid) - snap_short_tgt = _snap_tgt_short_name(snap) - if snap_short_tgt not in tgt_names: + # snap.snap_bdev was updated to the target path by apply_migration_to_db; + # use it as the primary lookup, then fall back to derived names. + _snap_bdev = snap.snap_bdev or '' + _primary = _snap_bdev.split('/', 1)[1] if '/' in _snap_bdev else _snap_bdev + _m_name = _snap_tgt_short_name(snap) + _canonical = _snap_short_name(snap) + _am_name = _canonical + _MIGRATION_BDEV_SUFFIX_DONE + if not any(n in tgt_names for n in (_primary, _m_name, _canonical, _am_name)): return False, False, ( - f"Target missing snapshot {snap_short_tgt} ({snap_uuid}) " + f"Target missing snapshot {_m_name} ({snap_uuid}) " "— aborting source cleanup to prevent data loss" ) except KeyError: @@ -1973,15 +2111,18 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): migration, tgt_lvol_uuid=tgt_lvol_uuid, tgt_lvol_bdev=tgt_lvol_bdev): return False, False, "Failed to update DB records after source cleanup" + tgt_sec_rpc = _get_secondary_rpc(tgt_node) + _tgt_ter = _get_target_tertiary_node(tgt_node)[0] + tgt_ter_rpc = _make_rpc(_tgt_ter) if _tgt_ter else None + # Delete intermediate (shrink) snapshots from the target — they are migration # artifacts and do not need to be preserved. No migration flag so SPDK # coalesces and frees their clusters into the child bdev. if migration.intermediate_snaps and not _SKIP_INTERMEDIATE_SNAP_DELETE: - tgt_sec_rpc = _get_secondary_rpc(tgt_node) - _tgt_ter = _get_target_tertiary_node(tgt_node)[0] - tgt_ter_rpc = _make_rpc(_tgt_ter) if _tgt_ter else None _delete_intermediate_snaps_on_target(migration, tgt_rpc, tgt_sec_rpc, tgt_ter_rpc) + _rename_migrated_bdevs(migration, tgt_node, tgt_rpc, tgt_sec_rpc, tgt_ter_rpc) + return True, False, None @@ -2062,10 +2203,21 @@ def _handle_cleanup_target(migration, tgt_node, tgt_rpc, src_rpc=None): for snap_uuid in reversed(migration_controller.get_snaps_to_delete_on_target(migration)): try: snap = db.get_snapshot_by_id(snap_uuid) - snap_short = _snap_tgt_short_name(snap) - bdev_name = f"{tgt_node.lvstore}/{snap_short}" - if not tgt_rpc.get_bdevs(bdev_name): - logger.info(f"Target bdev {bdev_name} not found; skipping (already cleaned up)") + # Try all possible bdev name variants: in-flight (m), canonical, am-fallback. + # After a partial rename (crash mid-cleanup) the bdev may have been + # renamed before the rollback was triggered. + _lvstore = tgt_node.lvstore + _m_name = _snap_tgt_short_name(snap) + _canonical = _snap_short_name(snap) + _am_name = _canonical + _MIGRATION_BDEV_SUFFIX_DONE + bdev_name = next( + (f"{_lvstore}/{n}" for n in (_m_name, _canonical, _am_name) + if tgt_rpc.get_bdevs(f"{_lvstore}/{n}")), + None) + if not bdev_name: + logger.info( + f"Target bdev {_lvstore}/{_m_name} not found in any variant; " + f"skipping (already cleaned up)") continue _delete_bdev_blocking(bdev_name, tgt_rpc, secondary_rpc=tgt_sec_rpc, tertiary_rpc=tgt_ter_rpc) From fdcf214edcab39e7db6f1e5ed55feaa2f3cdb3d4 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Sun, 5 Jul 2026 13:03:46 +0330 Subject: [PATCH 27/49] fixing an issue with migrating trees --- .../controllers/migration_controller.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/simplyblock_core/controllers/migration_controller.py b/simplyblock_core/controllers/migration_controller.py index 7f5075955..acfedc32a 100644 --- a/simplyblock_core/controllers/migration_controller.py +++ b/simplyblock_core/controllers/migration_controller.py @@ -429,10 +429,18 @@ def _is_snap_on_node(snap_id, node_id): def _get_snap_ancestry(snap_uuid): """ - Walk the ``snap_ref_id`` chain from *snap_uuid* upward to the root and + Walk the blobstore parent chain from *snap_uuid* upward to the root and return the UUIDs in root-first order (oldest ancestor first). - ``snap_ref_id`` points from a child snapshot to its parent snapshot. + Two parent pointers are tried in order: + - ``snap_ref_id``: set for snapshots taken from a clone volume (points to + the clone's parent snapshot, used for ref-count tracking). + - ``prev_snap_uuid``: set for all snapshots and records the previous + snapshot in the lvol's own sequence — this is the blobstore parent for + snapshots taken from a non-clone volume (e.g. snap_A3 → snap_A2 → + snap_A1 taken from the original lvol_A). Without this fallback, the + ancestry walk stops at the clone's immediate parent and misses the older + ancestor snapshots whose data is not copied standalone. """ chain = [] current = snap_uuid @@ -444,7 +452,7 @@ def _get_snap_ancestry(snap_uuid): except KeyError: break chain.append(current) - current = snap.snap_ref_id + current = snap.snap_ref_id or snap.prev_snap_uuid chain.reverse() # oldest → newest return chain @@ -538,7 +546,7 @@ def _protect_snap_and_ancestors(snap_uuid, candidate_set): candidate_set.discard(current) try: snap = db.get_snapshot_by_id(current) - current = snap.snap_ref_id + current = snap.snap_ref_id or snap.prev_snap_uuid except KeyError: break @@ -552,7 +560,7 @@ def _collect_snap_ancestry(snap_uuid, out_set): out_set.add(current) try: snap = db.get_snapshot_by_id(current) - current = snap.snap_ref_id + current = snap.snap_ref_id or snap.prev_snap_uuid except KeyError: break From f775a597f9bcbd173864d74f0e0d975ff0d1480a Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Sun, 5 Jul 2026 15:11:13 +0330 Subject: [PATCH 28/49] fixing an issue where ancestor snapshots would not be renamed after migration --- .../services/tasks_runner_lvol_migration.py | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index c6db35caf..2d28799e0 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -1895,6 +1895,57 @@ def _do_rename(old_composite, new_composite, label): logger.warning( f"_rename_migrated_bdevs: snap rename {current_short} failed: {exc}") + # --- Ancestor chain blobs (non-owned snaps) --- + # These belong to another lvol's chain; _apply_migration_to_db did NOT update + # snap.snap_bdev for them — it only added an instances entry with the _m name. + # Find the _m bdev via instances, rename it on TGT, and update the entry. + for snap_uuid in migration.snaps_migrated: + if snap_uuid in preexisting: + continue + try: + snap = db.get_snapshot_by_id(snap_uuid) + except KeyError: + continue + if snap.lvol.uuid == migration.lvol_id: + continue # owned snap — handled above + + updated = False + for inst in snap.instances: + inst_bdev = inst.get('snap_bdev', '') + if '/' not in inst_bdev: + continue + inst_lvstore, inst_short = inst_bdev.split('/', 1) + if inst_lvstore != lvstore: + continue # not on this migration's TGT lvstore + if not inst_short.endswith(_MIGRATION_BDEV_SUFFIX): + continue # already renamed by a previous call + + canonical_short = inst_short[:-len(_MIGRATION_BDEV_SUFFIX)] + if canonical_short in existing_bdevs: + fallback_short = canonical_short + _MIGRATION_BDEV_SUFFIX_DONE + if fallback_short in existing_bdevs: + logger.warning( + f"_rename_migrated_bdevs: ancestor {inst_short} — both " + f"{canonical_short} and {fallback_short} exist, leaving as-is") + continue + target_short = fallback_short + else: + target_short = canonical_short + + try: + _do_rename(f"{lvstore}/{inst_short}", f"{lvstore}/{target_short}", inst_short) + inst['snap_bdev'] = f"{lvstore}/{target_short}" + existing_bdevs.discard(inst_short) + existing_bdevs.add(target_short) + updated = True + logger.info(f"_rename_migrated_bdevs: ancestor snap {inst_short} → {target_short}") + except Exception as exc: + logger.warning( + f"_rename_migrated_bdevs: ancestor snap rename {inst_short} failed: {exc}") + + if updated: + snap.write_to_db(db.kv_store) + # --- Lvol --- try: lvol = db.get_lvol_by_id(migration.lvol_id) From 7ef1a904d0ef318e72207f692777dd0e52e9ddce Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Mon, 6 Jul 2026 09:22:05 +0330 Subject: [PATCH 29/49] disabling source cleanup for testing --- simplyblock_core/services/tasks_runner_lvol_migration.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index 2d28799e0..ce064a182 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -116,8 +116,8 @@ _INTERMEDIATE_POLL_INTERVAL_S = 1 # seconds between stat checks _INTERMEDIATE_POLL_MAX = 300 # max iterations ≈ 5 min -_SKIP_CLEANUP_SOURCE = False -_SKIP_INTERMEDIATE_SNAP_DELETE = False +_SKIP_CLEANUP_SOURCE = True +_SKIP_INTERMEDIATE_SNAP_DELETE = True def _now_ms(): From 7e16f1eac15b5b8d51652dc0d006c94b1e6cf483 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Mon, 6 Jul 2026 19:33:03 +0330 Subject: [PATCH 30/49] enabling cleanup and disabling rename for now --- simplyblock_core/services/tasks_runner_lvol_migration.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index ce064a182..ba4cb6090 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -116,8 +116,9 @@ _INTERMEDIATE_POLL_INTERVAL_S = 1 # seconds between stat checks _INTERMEDIATE_POLL_MAX = 300 # max iterations ≈ 5 min -_SKIP_CLEANUP_SOURCE = True -_SKIP_INTERMEDIATE_SNAP_DELETE = True +_SKIP_CLEANUP_SOURCE = False +_SKIP_INTERMEDIATE_SNAP_DELETE = False +_SKIP_RENAME_BDEVS = True def _now_ms(): @@ -1832,6 +1833,10 @@ def _rename_migrated_bdevs(migration, tgt_node, tgt_rpc, tgt_sec_rpc=None, tgt_t Must be called AFTER _apply_migration_to_db() — snap.snap_bdev and lvol.lvol_bdev already point to the 'm'-suffixed target paths at that point. """ + if _SKIP_RENAME_BDEVS: + logger.info("_rename_migrated_bdevs: skipped (SKIP_RENAME_BDEVS=True)") + return + lvstore = tgt_node.lvstore preexisting = set(migration.snaps_preexisting_on_target or []) From e4072df7ba9b374d2dd28f0e009598a8628cf3a9 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Mon, 6 Jul 2026 21:11:38 +0330 Subject: [PATCH 31/49] enabling lvol rename --- .../services/tasks_runner_lvol_migration.py | 159 ++++++++---------- 1 file changed, 71 insertions(+), 88 deletions(-) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index ba4cb6090..fcdafb9f4 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -118,7 +118,7 @@ _SKIP_CLEANUP_SOURCE = False _SKIP_INTERMEDIATE_SNAP_DELETE = False -_SKIP_RENAME_BDEVS = True +_SKIP_RENAME_BDEVS = False def _now_ms(): @@ -1826,9 +1826,9 @@ def _rename_migrated_bdevs(migration, tgt_node, tgt_rpc, tgt_sec_rpc=None, tgt_t their canonical names (without the suffix). This prevents suffix accumulation on repeated migrations of the same volume. - A pre-check ensures we only rename when the canonical name is free on the - target. If the canonical name already exists the 'm' name is kept as-is so - both snapshots can coexist without conflict. + bdev_lvol_rename returns "EXISTS" when the target name is already taken; + we use that signal to try the fallback (_MIGRATION_BDEV_SUFFIX_DONE) instead + of doing a pre-query of all bdevs. Must be called AFTER _apply_migration_to_db() — snap.snap_bdev and lvol.lvol_bdev already point to the 'm'-suffixed target paths at that point. @@ -1840,26 +1840,53 @@ def _rename_migrated_bdevs(migration, tgt_node, tgt_rpc, tgt_sec_rpc=None, tgt_t lvstore = tgt_node.lvstore preexisting = set(migration.snaps_preexisting_on_target or []) - try: - existing_bdevs = { - e.get('name', '').split('/')[-1] - for e in (tgt_rpc.bdev_lvol_get_lvols(lvstore) or []) - } - except Exception as exc: - logger.warning(f"_rename_migrated_bdevs: could not query target bdevs: {exc}") - return + _EXISTS = "EXISTS" def _do_rename(old_composite, new_composite, label): - tgt_rpc.bdev_lvol_rename(old_composite, new_composite) + """Rename on prim + sec + ter. Returns 'EXISTS' if any node reports the + target name is already taken, True on success. Raises on hard failure.""" + ret = tgt_rpc.bdev_lvol_rename(old_composite, new_composite) + exists = (ret == _EXISTS) for role, rpc in [("sec", tgt_sec_rpc), ("ter", tgt_ter_rpc)]: if rpc: try: - rpc.bdev_lvol_rename(old_composite, new_composite) + r = rpc.bdev_lvol_rename(old_composite, new_composite) + if r == _EXISTS: + exists = True except Exception as exc: logger.warning( f"_rename_migrated_bdevs: {role} rename {label} (non-fatal): {exc}") + if exists: + return _EXISTS + if not ret: + raise RuntimeError(f"bdev_lvol_rename {label}: ret={ret!r}") + return True + + def _rename_with_fallback(current_short, label): + """Try canonical name; on EXISTS try the _done fallback. + Returns the target short name on success, None if skipped.""" + canonical = current_short[:-len(_MIGRATION_BDEV_SUFFIX)] + old = f"{lvstore}/{current_short}" + + ret = _do_rename(old, f"{lvstore}/{canonical}", label) + if ret == _EXISTS: + fallback = canonical + _MIGRATION_BDEV_SUFFIX_DONE + logger.info( + f"_rename_migrated_bdevs: {canonical} exists — trying fallback {fallback}") + ret2 = _do_rename(old, f"{lvstore}/{fallback}", label) + if ret2 == _EXISTS: + logger.warning( + f"_rename_migrated_bdevs: both {canonical} and {fallback} " + f"exist — leaving {current_short} as-is") + return None + target = fallback + else: + target = canonical - # --- Snapshots --- + logger.info(f"_rename_migrated_bdevs: {current_short} → {target}") + return target + + # --- Snapshots (owned) --- for snap_uuid in migration.snaps_migrated: if snap_uuid in preexisting: continue @@ -1868,42 +1895,28 @@ def _do_rename(old_composite, new_composite, label): except KeyError: continue - snap_bdev = snap.snap_bdev # e.g. LVS_TGT/SNAP_Xm + snap_bdev = snap.snap_bdev if '/' not in snap_bdev: continue current_short = snap_bdev.split('/', 1)[1] if not current_short.endswith(_MIGRATION_BDEV_SUFFIX): - continue # already canonical + continue - canonical_short = current_short[:-len(_MIGRATION_BDEV_SUFFIX)] - if canonical_short in existing_bdevs: - fallback_short = canonical_short + _MIGRATION_BDEV_SUFFIX_DONE - if fallback_short in existing_bdevs: - logger.warning( - f"_rename_migrated_bdevs: both {canonical_short} and {fallback_short} " - f"exist — cannot rename {current_short}, leaving as-is") - continue - target_short = fallback_short - logger.info( - f"_rename_migrated_bdevs: {canonical_short} taken — renaming {current_short} → {fallback_short}") - else: - target_short = canonical_short + if snap.lvol.uuid != migration.lvol_id: + continue # ancestor snap — handled below try: - _do_rename(f"{lvstore}/{current_short}", f"{lvstore}/{target_short}", current_short) - snap.snap_bdev = f"{lvstore}/{target_short}" - snap.write_to_db(db.kv_store) - existing_bdevs.discard(current_short) - existing_bdevs.add(target_short) - logger.info(f"_rename_migrated_bdevs: snap {current_short} → {target_short}") + target = _rename_with_fallback(current_short, current_short) except Exception as exc: - logger.warning( - f"_rename_migrated_bdevs: snap rename {current_short} failed: {exc}") + logger.warning(f"_rename_migrated_bdevs: snap {current_short} failed: {exc}") + continue + if target: + snap.snap_bdev = f"{lvstore}/{target}" + snap.write_to_db(db.kv_store) # --- Ancestor chain blobs (non-owned snaps) --- - # These belong to another lvol's chain; _apply_migration_to_db did NOT update - # snap.snap_bdev for them — it only added an instances entry with the _m name. - # Find the _m bdev via instances, rename it on TGT, and update the entry. + # _apply_migration_to_db added an instances entry with the _m bdev name. + # Rename it and update the entry in place. for snap_uuid in migration.snaps_migrated: if snap_uuid in preexisting: continue @@ -1921,32 +1934,19 @@ def _do_rename(old_composite, new_composite, label): continue inst_lvstore, inst_short = inst_bdev.split('/', 1) if inst_lvstore != lvstore: - continue # not on this migration's TGT lvstore + continue if not inst_short.endswith(_MIGRATION_BDEV_SUFFIX): - continue # already renamed by a previous call - - canonical_short = inst_short[:-len(_MIGRATION_BDEV_SUFFIX)] - if canonical_short in existing_bdevs: - fallback_short = canonical_short + _MIGRATION_BDEV_SUFFIX_DONE - if fallback_short in existing_bdevs: - logger.warning( - f"_rename_migrated_bdevs: ancestor {inst_short} — both " - f"{canonical_short} and {fallback_short} exist, leaving as-is") - continue - target_short = fallback_short - else: - target_short = canonical_short + continue try: - _do_rename(f"{lvstore}/{inst_short}", f"{lvstore}/{target_short}", inst_short) - inst['snap_bdev'] = f"{lvstore}/{target_short}" - existing_bdevs.discard(inst_short) - existing_bdevs.add(target_short) - updated = True - logger.info(f"_rename_migrated_bdevs: ancestor snap {inst_short} → {target_short}") + target = _rename_with_fallback(inst_short, inst_short) except Exception as exc: logger.warning( - f"_rename_migrated_bdevs: ancestor snap rename {inst_short} failed: {exc}") + f"_rename_migrated_bdevs: ancestor {inst_short} failed: {exc}") + continue + if target: + inst['snap_bdev'] = f"{lvstore}/{target}" + updated = True if updated: snap.write_to_db(db.kv_store) @@ -1960,38 +1960,21 @@ def _do_rename(old_composite, new_composite, label): current_lvol_short = lvol.lvol_bdev if not current_lvol_short.endswith(_MIGRATION_BDEV_SUFFIX): - return # already canonical - - canonical_lvol_short = current_lvol_short[:-len(_MIGRATION_BDEV_SUFFIX)] - if canonical_lvol_short in existing_bdevs: - fallback_lvol_short = canonical_lvol_short + _MIGRATION_BDEV_SUFFIX_DONE - if fallback_lvol_short in existing_bdevs: - logger.warning( - f"_rename_migrated_bdevs: both {canonical_lvol_short} and {fallback_lvol_short} " - f"exist — cannot rename {current_lvol_short}, leaving as-is") - return - target_lvol_short = fallback_lvol_short - logger.info( - f"_rename_migrated_bdevs: {canonical_lvol_short} taken — renaming {current_lvol_short} → {fallback_lvol_short}") - else: - target_lvol_short = canonical_lvol_short + return try: - _do_rename( - f"{lvstore}/{current_lvol_short}", - f"{lvstore}/{target_lvol_short}", - current_lvol_short) - lvol.lvol_bdev = target_lvol_short - lvol.top_bdev = f"{lvstore}/{target_lvol_short}" + target = _rename_with_fallback(current_lvol_short, current_lvol_short) + except Exception as exc: + logger.warning(f"_rename_migrated_bdevs: lvol rename failed: {exc}") + return + if target: + lvol.lvol_bdev = target + lvol.top_bdev = f"{lvstore}/{target}" for entry in lvol.bdev_stack: if (entry.get('type') == 'bdev_lvol' and entry.get('params', {}).get('name') == current_lvol_short): - entry['params']['name'] = target_lvol_short + entry['params']['name'] = target lvol.write_to_db(db.kv_store) - logger.info(f"_rename_migrated_bdevs: lvol {current_lvol_short} → {target_lvol_short}") - except Exception as exc: - logger.warning( - f"_rename_migrated_bdevs: lvol rename failed: {exc}") def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): From d24b0ed8ac6ffbefdc316c16f424131489584aae Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Tue, 7 Jul 2026 05:47:44 +0330 Subject: [PATCH 32/49] changing the comments after fact checking about rename --- .../services/tasks_runner_lvol_migration.py | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index fcdafb9f4..1b2055a8f 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -1826,9 +1826,9 @@ def _rename_migrated_bdevs(migration, tgt_node, tgt_rpc, tgt_sec_rpc=None, tgt_t their canonical names (without the suffix). This prevents suffix accumulation on repeated migrations of the same volume. - bdev_lvol_rename returns "EXISTS" when the target name is already taken; - we use that signal to try the fallback (_MIGRATION_BDEV_SUFFIX_DONE) instead - of doing a pre-query of all bdevs. + bdev_lvol_rename returns JSON-RPC error -32602 "File exists" (None to caller) + when the target name is already taken; we use that to try the fallback + (_MIGRATION_BDEV_SUFFIX_DONE) instead of doing a pre-query of all bdevs. Must be called AFTER _apply_migration_to_db() — snap.snap_bdev and lvol.lvol_bdev already point to the 'm'-suffixed target paths at that point. @@ -1842,24 +1842,26 @@ def _rename_migrated_bdevs(migration, tgt_node, tgt_rpc, tgt_sec_rpc=None, tgt_t _EXISTS = "EXISTS" - def _do_rename(old_composite, new_composite, label): - """Rename on prim + sec + ter. Returns 'EXISTS' if any node reports the - target name is already taken, True on success. Raises on hard failure.""" - ret = tgt_rpc.bdev_lvol_rename(old_composite, new_composite) - exists = (ret == _EXISTS) + def _do_rename(old_composite, new_short, label): + """Rename on prim + sec + ter. Returns 'EXISTS' if the target name is + already taken (SPDK returns JSON-RPC error -32602 'File exists' -> None), + True on success. new_short must be the short name only (no lvstore prefix).""" + ret = tgt_rpc.bdev_lvol_rename(old_composite, new_short) + logger.debug(f"_do_rename prim: {old_composite!r} -> {new_short!r}: ret={ret!r}") + # SPDK returns None on name collision (-32602 "File exists"); treat any falsy as collision. + exists = (not ret) or (ret == _EXISTS) for role, rpc in [("sec", tgt_sec_rpc), ("ter", tgt_ter_rpc)]: if rpc: try: - r = rpc.bdev_lvol_rename(old_composite, new_composite) - if r == _EXISTS: + r = rpc.bdev_lvol_rename(old_composite, new_short) + logger.debug(f"_do_rename {role}: {old_composite!r} -> {new_short!r}: ret={r!r}") + if (not r) or r == _EXISTS: exists = True except Exception as exc: logger.warning( f"_rename_migrated_bdevs: {role} rename {label} (non-fatal): {exc}") if exists: return _EXISTS - if not ret: - raise RuntimeError(f"bdev_lvol_rename {label}: ret={ret!r}") return True def _rename_with_fallback(current_short, label): @@ -1868,22 +1870,22 @@ def _rename_with_fallback(current_short, label): canonical = current_short[:-len(_MIGRATION_BDEV_SUFFIX)] old = f"{lvstore}/{current_short}" - ret = _do_rename(old, f"{lvstore}/{canonical}", label) + ret = _do_rename(old, canonical, label) if ret == _EXISTS: fallback = canonical + _MIGRATION_BDEV_SUFFIX_DONE logger.info( - f"_rename_migrated_bdevs: {canonical} exists — trying fallback {fallback}") - ret2 = _do_rename(old, f"{lvstore}/{fallback}", label) + f"_rename_migrated_bdevs: {canonical} exists - trying fallback {fallback}") + ret2 = _do_rename(old, fallback, label) if ret2 == _EXISTS: logger.warning( f"_rename_migrated_bdevs: both {canonical} and {fallback} " - f"exist — leaving {current_short} as-is") + f"exist - leaving {current_short} as-is") return None target = fallback else: target = canonical - logger.info(f"_rename_migrated_bdevs: {current_short} → {target}") + logger.info(f"_rename_migrated_bdevs: {current_short} -> {target}") return target # --- Snapshots (owned) --- From c317b7644666025a3b609ce5993297c9fae5069f Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Fri, 26 Jun 2026 15:42:41 +0330 Subject: [PATCH 33/49] fixing an issue with namespace not being added for tertiary node --- simplyblock_core/controllers/migration_controller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/simplyblock_core/controllers/migration_controller.py b/simplyblock_core/controllers/migration_controller.py index acfedc32a..c53f00f3e 100644 --- a/simplyblock_core/controllers/migration_controller.py +++ b/simplyblock_core/controllers/migration_controller.py @@ -793,7 +793,7 @@ def create_migration(lvol_id, target_node_id, pass tgt_ter_node = None - if tgt_node.tertiary_node_id: + if lvol.ha_type == "ha3" and tgt_node.tertiary_node_id: tgt_ter_node = (_pre_ter_node if _pre_ter_node is not None else None) if tgt_ter_node is None: try: From d023ee830fd820f8de473c86e14b11e7e26e51f5 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Fri, 26 Jun 2026 16:17:26 +0330 Subject: [PATCH 34/49] followup for fixing tertiary ns add issue --- simplyblock_core/controllers/migration_controller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/simplyblock_core/controllers/migration_controller.py b/simplyblock_core/controllers/migration_controller.py index c53f00f3e..acfedc32a 100644 --- a/simplyblock_core/controllers/migration_controller.py +++ b/simplyblock_core/controllers/migration_controller.py @@ -793,7 +793,7 @@ def create_migration(lvol_id, target_node_id, pass tgt_ter_node = None - if lvol.ha_type == "ha3" and tgt_node.tertiary_node_id: + if tgt_node.tertiary_node_id: tgt_ter_node = (_pre_ter_node if _pre_ter_node is not None else None) if tgt_ter_node is None: try: From f1a2b0784614cff5458fc7ae59f1c293cc5027dc Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Tue, 30 Jun 2026 12:01:47 +0330 Subject: [PATCH 35/49] TEMP: testing snapshot resume on retry instead of delete --- simplyblock_core/services/tasks_runner_lvol_migration.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index 1b2055a8f..f132c64b1 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -1009,6 +1009,14 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): except Exception as e: logger.warning(f"Pre-cleanup of {tgt_composite} failed (continuing): {e}") + if _INJECT_SNAP_FAIL and snap_uuid not in _snap_fail_injected: + _snap_fail_injected.add(snap_uuid) + migration.transfer_context = {} + migration.write_to_db(db.kv_store) + logger.warning( + f"[INJECT] Artificial snap transfer failure for {snap_uuid}") + return False, True, f"[INJECT] snap transfer failure for {snap_uuid}" + t, err = _setup_snap_transfer( snap, snap_index, src_node, tgt_node, src_rpc, tgt_rpc, trtype, From 6f9d2d1aab911db5de3276c4574726a816dc3c0d Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Tue, 30 Jun 2026 13:29:04 +0330 Subject: [PATCH 36/49] TEMP: disabling the cleanup from source and intermediates --- simplyblock_core/services/tasks_runner_lvol_migration.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index f132c64b1..1b2055a8f 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -1009,14 +1009,6 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): except Exception as e: logger.warning(f"Pre-cleanup of {tgt_composite} failed (continuing): {e}") - if _INJECT_SNAP_FAIL and snap_uuid not in _snap_fail_injected: - _snap_fail_injected.add(snap_uuid) - migration.transfer_context = {} - migration.write_to_db(db.kv_store) - logger.warning( - f"[INJECT] Artificial snap transfer failure for {snap_uuid}") - return False, True, f"[INJECT] snap transfer failure for {snap_uuid}" - t, err = _setup_snap_transfer( snap, snap_index, src_node, tgt_node, src_rpc, tgt_rpc, trtype, From c0eb29fec72bb4b0fa3c2bc55f68f51133e6c359 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Sat, 4 Jul 2026 07:51:42 +0330 Subject: [PATCH 37/49] setting the env_var to main --- simplyblock_core/env_var | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/simplyblock_core/env_var b/simplyblock_core/env_var index e498d47fd..5abdecd44 100644 --- a/simplyblock_core/env_var +++ b/simplyblock_core/env_var @@ -2,4 +2,4 @@ SIMPLY_BLOCK_COMMAND_NAME=sbcli-dev SIMPLY_BLOCK_VERSION=19.2.34 SIMPLY_BLOCK_DOCKER_IMAGE=public.ecr.aws/simply-block/simplyblock:main -SIMPLY_BLOCK_SPDK_ULTRA_IMAGE=public.ecr.aws/simply-block/ultra:debuge-migration-latest +SIMPLY_BLOCK_SPDK_ULTRA_IMAGE=public.ecr.aws/simply-block/ultra:main-latest From 7da99e2f4ec9398fd8bb4d0e7305d56dfda47213 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Sat, 4 Jul 2026 23:03:22 +0330 Subject: [PATCH 38/49] setting env_var for spdk image --- simplyblock_core/env_var | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/simplyblock_core/env_var b/simplyblock_core/env_var index 5abdecd44..e498d47fd 100644 --- a/simplyblock_core/env_var +++ b/simplyblock_core/env_var @@ -2,4 +2,4 @@ SIMPLY_BLOCK_COMMAND_NAME=sbcli-dev SIMPLY_BLOCK_VERSION=19.2.34 SIMPLY_BLOCK_DOCKER_IMAGE=public.ecr.aws/simply-block/simplyblock:main -SIMPLY_BLOCK_SPDK_ULTRA_IMAGE=public.ecr.aws/simply-block/ultra:main-latest +SIMPLY_BLOCK_SPDK_ULTRA_IMAGE=public.ecr.aws/simply-block/ultra:debuge-migration-latest From 2effec62d8676008a9dde0948a68b8008742e75e Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Sun, 5 Jul 2026 15:11:13 +0330 Subject: [PATCH 39/49] fixing an issue where ancestor snapshots would not be renamed after migration --- .../services/tasks_runner_lvol_migration.py | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index 1b2055a8f..003121e75 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -1953,6 +1953,57 @@ def _rename_with_fallback(current_short, label): if updated: snap.write_to_db(db.kv_store) + # --- Ancestor chain blobs (non-owned snaps) --- + # These belong to another lvol's chain; _apply_migration_to_db did NOT update + # snap.snap_bdev for them — it only added an instances entry with the _m name. + # Find the _m bdev via instances, rename it on TGT, and update the entry. + for snap_uuid in migration.snaps_migrated: + if snap_uuid in preexisting: + continue + try: + snap = db.get_snapshot_by_id(snap_uuid) + except KeyError: + continue + if snap.lvol.uuid == migration.lvol_id: + continue # owned snap — handled above + + updated = False + for inst in snap.instances: + inst_bdev = inst.get('snap_bdev', '') + if '/' not in inst_bdev: + continue + inst_lvstore, inst_short = inst_bdev.split('/', 1) + if inst_lvstore != lvstore: + continue # not on this migration's TGT lvstore + if not inst_short.endswith(_MIGRATION_BDEV_SUFFIX): + continue # already renamed by a previous call + + canonical_short = inst_short[:-len(_MIGRATION_BDEV_SUFFIX)] + if canonical_short in existing_bdevs: + fallback_short = canonical_short + _MIGRATION_BDEV_SUFFIX_DONE + if fallback_short in existing_bdevs: + logger.warning( + f"_rename_migrated_bdevs: ancestor {inst_short} — both " + f"{canonical_short} and {fallback_short} exist, leaving as-is") + continue + target_short = fallback_short + else: + target_short = canonical_short + + try: + _do_rename(f"{lvstore}/{inst_short}", f"{lvstore}/{target_short}", inst_short) + inst['snap_bdev'] = f"{lvstore}/{target_short}" + existing_bdevs.discard(inst_short) + existing_bdevs.add(target_short) + updated = True + logger.info(f"_rename_migrated_bdevs: ancestor snap {inst_short} → {target_short}") + except Exception as exc: + logger.warning( + f"_rename_migrated_bdevs: ancestor snap rename {inst_short} failed: {exc}") + + if updated: + snap.write_to_db(db.kv_store) + # --- Lvol --- try: lvol = db.get_lvol_by_id(migration.lvol_id) From b56c429704c4f5e348cb0ff34cc46048cf7bbf72 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Mon, 6 Jul 2026 21:11:38 +0330 Subject: [PATCH 40/49] enabling lvol rename --- .../services/tasks_runner_lvol_migration.py | 34 ++++++------------- 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index 003121e75..6f83e7944 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -1954,9 +1954,8 @@ def _rename_with_fallback(current_short, label): snap.write_to_db(db.kv_store) # --- Ancestor chain blobs (non-owned snaps) --- - # These belong to another lvol's chain; _apply_migration_to_db did NOT update - # snap.snap_bdev for them — it only added an instances entry with the _m name. - # Find the _m bdev via instances, rename it on TGT, and update the entry. + # _apply_migration_to_db added an instances entry with the _m bdev name. + # Rename it and update the entry in place. for snap_uuid in migration.snaps_migrated: if snap_uuid in preexisting: continue @@ -1974,32 +1973,19 @@ def _rename_with_fallback(current_short, label): continue inst_lvstore, inst_short = inst_bdev.split('/', 1) if inst_lvstore != lvstore: - continue # not on this migration's TGT lvstore + continue if not inst_short.endswith(_MIGRATION_BDEV_SUFFIX): - continue # already renamed by a previous call - - canonical_short = inst_short[:-len(_MIGRATION_BDEV_SUFFIX)] - if canonical_short in existing_bdevs: - fallback_short = canonical_short + _MIGRATION_BDEV_SUFFIX_DONE - if fallback_short in existing_bdevs: - logger.warning( - f"_rename_migrated_bdevs: ancestor {inst_short} — both " - f"{canonical_short} and {fallback_short} exist, leaving as-is") - continue - target_short = fallback_short - else: - target_short = canonical_short + continue try: - _do_rename(f"{lvstore}/{inst_short}", f"{lvstore}/{target_short}", inst_short) - inst['snap_bdev'] = f"{lvstore}/{target_short}" - existing_bdevs.discard(inst_short) - existing_bdevs.add(target_short) - updated = True - logger.info(f"_rename_migrated_bdevs: ancestor snap {inst_short} → {target_short}") + target = _rename_with_fallback(inst_short, inst_short) except Exception as exc: logger.warning( - f"_rename_migrated_bdevs: ancestor snap rename {inst_short} failed: {exc}") + f"_rename_migrated_bdevs: ancestor {inst_short} failed: {exc}") + continue + if target: + inst['snap_bdev'] = f"{lvstore}/{target}" + updated = True if updated: snap.write_to_db(db.kv_store) From f0cec346e1feb31a73ea925fdfbcc127e0be38e4 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Tue, 7 Jul 2026 06:10:10 +0330 Subject: [PATCH 41/49] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> --- .../services/hub_controller_manager.py | 19 ++++++++++++++----- .../services/tasks_runner_lvol_migration.py | 7 ++++++- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/simplyblock_core/services/hub_controller_manager.py b/simplyblock_core/services/hub_controller_manager.py index 195ccfa50..fa50a339d 100644 --- a/simplyblock_core/services/hub_controller_manager.py +++ b/simplyblock_core/services/hub_controller_manager.py @@ -113,7 +113,10 @@ def acquire(self, src_node_id: str, src_rpc, tgt_node, trtype: str): ) return entry.ctrl_name, entry.hub_bdev, None except Exception: - pass + logger.exception( + f"[HubMgr] failed to validate cached hub entry {entry.ctrl_name}; " + "treating as stale and re-attaching" + ) logger.info(f"[HubMgr] stale entry for {entry.ctrl_name}; re-attaching") del self._entries[key] @@ -142,11 +145,17 @@ def acquire(self, src_node_id: str, src_rpc, tgt_node, trtype: str): existing.src_rpc = src_rpc try: src_rpc.bdev_nvme_detach_controller(ctrl_name) - except Exception: - pass + except Exception as e: + logger.warning( + f"[HubMgr] best-effort detach of duplicate controller failed " + f"(ctrl={ctrl_name}, src={src_node_id[:8]} tgt={tgt_node.get_id()[:8]}): {e}" + ) return existing.ctrl_name, existing.hub_bdev, None - except Exception: - pass + except Exception as exc: + logger.debug( + f"[HubMgr] failed to validate concurrently attached controller " + f"{existing.ctrl_name}: {exc}" + ) entry = _HubEntry(ctrl_name, hub_bdev, src_rpc, tgt_node) self._entries[key] = entry diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index 6f83e7944..5daf82fab 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -1812,7 +1812,12 @@ def _get_tertiary_rpc(node): if ter.status == StorageNode.STATUS_ONLINE: return _make_rpc(ter) except KeyError: - pass + # Tertiary reference may be stale (node removed/not found); treat as unavailable. + logger.debug( + "Tertiary node not found for node_id=%s tertiary_node_id=%s", + getattr(node, "id", None), + node.tertiary_node_id, + ) return None From 94532a6ef09a8188f509ed90bb9d10d195a1693d Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Tue, 7 Jul 2026 06:21:41 +0330 Subject: [PATCH 42/49] fixing linting errors --- simplyblock_core/controllers/migration_controller.py | 2 +- simplyblock_core/services/hub_controller_manager.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/simplyblock_core/controllers/migration_controller.py b/simplyblock_core/controllers/migration_controller.py index acfedc32a..610b66f28 100644 --- a/simplyblock_core/controllers/migration_controller.py +++ b/simplyblock_core/controllers/migration_controller.py @@ -44,7 +44,7 @@ from datetime import datetime from simplyblock_core import constants -from simplyblock_core.controllers import migration_events, tasks_controller, snapshot_controller +from simplyblock_core.controllers import migration_events, tasks_controller from simplyblock_core.exceptions import MigrationConflictError, PreconditionError from simplyblock_core.controllers.host_auth import _reapply_allowed_hosts from simplyblock_core.kms import create_kms_connection, lvol_dek_path, pool_kek_name diff --git a/simplyblock_core/services/hub_controller_manager.py b/simplyblock_core/services/hub_controller_manager.py index fa50a339d..192886bb3 100644 --- a/simplyblock_core/services/hub_controller_manager.py +++ b/simplyblock_core/services/hub_controller_manager.py @@ -41,6 +41,7 @@ import threading import time +from typing import Optional from simplyblock_core import utils @@ -82,7 +83,7 @@ def __init__(self): self._lock = threading.Lock() self._entries: dict = {} # (src_node_id, tgt_node_id) → _HubEntry self._detach_ts: dict = {} # (src_node_id, tgt_node_id) → monotonic ts of last detach - self._gc_thread: threading.Thread | None = None + self._gc_thread: Optional[threading.Thread] = None # ------------------------------------------------------------------ # Public API From c99d4f7c6025b122cedb28c9f165c9752f7fdc2b Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Tue, 7 Jul 2026 07:02:29 +0330 Subject: [PATCH 43/49] fixed an issue that made unit test fail --- simplyblock_core/services/storage_node_monitor.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/simplyblock_core/services/storage_node_monitor.py b/simplyblock_core/services/storage_node_monitor.py index 782965583..771ed35e3 100644 --- a/simplyblock_core/services/storage_node_monitor.py +++ b/simplyblock_core/services/storage_node_monitor.py @@ -216,7 +216,9 @@ def get_next_cluster_status(cluster_id): else: node_offline_devices += 1 - if node_offline_devices > 0 or (node_online_devices == 0 and node.status != StorageNode.STATUS_REMOVED): + if (node_offline_devices > 0 + or (node_online_devices == 0 and node.status != StorageNode.STATUS_REMOVED) + or node.status == StorageNode.STATUS_OFFLINE): affected_nodes += 1 if node.mgmt_ip not in affected_physical_nodes: affected_physical_nodes.append(node.mgmt_ip) From e9941c750b871da84d1b95d07ce5bc8277f34858 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Tue, 7 Jul 2026 07:39:36 +0330 Subject: [PATCH 44/49] setting correct image refernces for env var --- simplyblock_core/env_var | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/simplyblock_core/env_var b/simplyblock_core/env_var index e498d47fd..5abdecd44 100644 --- a/simplyblock_core/env_var +++ b/simplyblock_core/env_var @@ -2,4 +2,4 @@ SIMPLY_BLOCK_COMMAND_NAME=sbcli-dev SIMPLY_BLOCK_VERSION=19.2.34 SIMPLY_BLOCK_DOCKER_IMAGE=public.ecr.aws/simply-block/simplyblock:main -SIMPLY_BLOCK_SPDK_ULTRA_IMAGE=public.ecr.aws/simply-block/ultra:debuge-migration-latest +SIMPLY_BLOCK_SPDK_ULTRA_IMAGE=public.ecr.aws/simply-block/ultra:main-latest From 164e5d60b30c037baa3f16fdfbda4609198184ac Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Thu, 9 Jul 2026 05:32:01 +0330 Subject: [PATCH 45/49] using request3 for new rpc call --- simplyblock_core/rpc_client.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/simplyblock_core/rpc_client.py b/simplyblock_core/rpc_client.py index e5e26d5be..4b95040b3 100755 --- a/simplyblock_core/rpc_client.py +++ b/simplyblock_core/rpc_client.py @@ -1310,8 +1310,7 @@ def bdev_lvol_get_lvstores(self, name): return self._request("bdev_lvol_get_lvstores", params) def bdev_lvol_rename(self, old_name, new_name): - params = {"old_name": old_name, "new_name": new_name} - return self._request("bdev_lvol_rename", params) + return self._request3("bdev_lvol_rename", old_name=old_name, new_name=new_name) def bdev_lvol_resize(self, name, size_in_mib): params = { From 88bda5b19d071f516af0e76386540b0e320d3405 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Thu, 9 Jul 2026 05:36:13 +0330 Subject: [PATCH 46/49] fix: escalate migration errors and deduplicate ancestry helpers --- .../controllers/migration_controller.py | 29 ++++------- .../services/tasks_runner_lvol_migration.py | 51 ++++++++----------- 2 files changed, 32 insertions(+), 48 deletions(-) diff --git a/simplyblock_core/controllers/migration_controller.py b/simplyblock_core/controllers/migration_controller.py index 610b66f28..e8af3ded8 100644 --- a/simplyblock_core/controllers/migration_controller.py +++ b/simplyblock_core/controllers/migration_controller.py @@ -529,7 +529,7 @@ def get_snaps_to_delete_on_target(migration): if lvol.uuid == migration.lvol_id: continue if lvol.cloned_from_snap: - _collect_snap_ancestry(lvol.cloned_from_snap, protected) + protected |= _collect_snap_ancestry(lvol.cloned_from_snap) return [ uid for uid in migration.snaps_migrated @@ -537,32 +537,23 @@ def get_snaps_to_delete_on_target(migration): ] -def _protect_snap_and_ancestors(snap_uuid, candidate_set): - """Remove *snap_uuid* and all its ancestors from *candidate_set*.""" +def _collect_snap_ancestry(snap_uuid) -> set: + """Return the set of *snap_uuid* and all its ancestors.""" + result: set = set() current = snap_uuid - visited = set() - while current and current not in visited: - visited.add(current) - candidate_set.discard(current) + while current and current not in result: + result.add(current) try: snap = db.get_snapshot_by_id(current) current = snap.snap_ref_id or snap.prev_snap_uuid except KeyError: break + return result -def _collect_snap_ancestry(snap_uuid, out_set): - """Add *snap_uuid* and all its ancestors to *out_set*.""" - current = snap_uuid - visited = set() - while current and current not in visited: - visited.add(current) - out_set.add(current) - try: - snap = db.get_snapshot_by_id(current) - current = snap.snap_ref_id or snap.prev_snap_uuid - except KeyError: - break +def _protect_snap_and_ancestors(snap_uuid, candidate_set): + """Remove *snap_uuid* and all its ancestors from *candidate_set*.""" + candidate_set -= _collect_snap_ancestry(snap_uuid) # --------------------------------------------------------------------------- diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index 5daf82fab..3d7a5fafb 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -260,8 +260,7 @@ def _apply_migration_to_db(migration, tgt_lvol_uuid=None, tgt_lvol_bdev=None): if src_lvstore == tgt_node.lvstore: tgt_short = src_short # already updated by a previous call — idempotent else: - base = src_short[:-len(_MIGRATION_BDEV_SUFFIX)] if src_short.endswith(_MIGRATION_BDEV_SUFFIX) else src_short - tgt_short = base + _MIGRATION_BDEV_SUFFIX + tgt_short = _snap_tgt_short_name(snap) snap.snap_bdev = f"{tgt_node.lvstore}/{tgt_short}" if tgt_short and tgt_short in spdk_info: @@ -429,14 +428,10 @@ def _delete_bdev_blocking(bdev_name, primary_rpc, secondary_rpc=None, tertiary_r """ ret, _ = primary_rpc.delete_lvol(bdev_name, del_async=False, special_delete=True) if not ret: - logger.warning(f"delete bdev {bdev_name}: initiation failed (continuing)") - return + raise RuntimeError(f"delete bdev {bdev_name}: initiation failed") deadline = time.monotonic() + timeout_s - while True: - status = primary_rpc.bdev_lvol_get_lvol_delete_status(bdev_name) - if status != 1: - break + while primary_rpc.bdev_lvol_get_lvol_delete_status(bdev_name) == 1: if time.monotonic() > deadline: if not primary_rpc.get_bdevs(bdev_name): logger.warning( @@ -1641,8 +1636,8 @@ def _flip_required(rpc, ip, port, trtype, state, label, attempts=3): try: _swap_namespace(tgt['rpc'], nqn, tgt_ns_bdev, lvol.uuid, lvol.guid, tgt['node_id'][:8]) - except Exception as e: - logger.error(f"Namespace swap on {tgt['node_id'][:8]} failed: {e}") + except Exception: + raise # Step 5: all TGT paths → correct ANA state at TGT port. # Primary required; secondaries best-effort. @@ -1764,7 +1759,8 @@ def _delete_intermediate_snaps_on_target(migration, tgt_rpc, tgt_sec_rpc=None, t # Phase 1: initiate coalescing delete on primary (sync=False) ret, _ = tgt_rpc.delete_lvol(tgt_composite, del_async=False, special_delete=False) if not ret: - logger.warning(f"Coalescing delete initiation of {tgt_composite} on primary failed") + raise RuntimeError( + f"Coalescing delete initiation of {tgt_composite} on primary failed") else: # Phase 2: finalize on all nodes (sync=True) for label, rpc in [("primary", tgt_rpc), @@ -1912,11 +1908,7 @@ def _rename_with_fallback(current_short, label): if snap.lvol.uuid != migration.lvol_id: continue # ancestor snap — handled below - try: - target = _rename_with_fallback(current_short, current_short) - except Exception as exc: - logger.warning(f"_rename_migrated_bdevs: snap {current_short} failed: {exc}") - continue + target = _rename_with_fallback(current_short, current_short) if target: snap.snap_bdev = f"{lvstore}/{target}" snap.write_to_db(db.kv_store) @@ -2006,11 +1998,7 @@ def _rename_with_fallback(current_short, label): if not current_lvol_short.endswith(_MIGRATION_BDEV_SUFFIX): return - try: - target = _rename_with_fallback(current_lvol_short, current_lvol_short) - except Exception as exc: - logger.warning(f"_rename_migrated_bdevs: lvol rename failed: {exc}") - return + target = _rename_with_fallback(current_lvol_short, current_lvol_short) if target: lvol.lvol_bdev = target lvol.top_bdev = f"{lvstore}/{target}" @@ -2044,9 +2032,12 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): _tgt_sec_rpc = _get_secondary_rpc(tgt_node) _tgt_ter = _get_target_tertiary_node(tgt_node)[0] _tgt_ter_rpc = _make_rpc(_tgt_ter) if _tgt_ter else None - if migration.intermediate_snaps and not _SKIP_INTERMEDIATE_SNAP_DELETE: - _delete_intermediate_snaps_on_target(migration, tgt_rpc, _tgt_sec_rpc, _tgt_ter_rpc) - _rename_migrated_bdevs(migration, tgt_node, tgt_rpc, _tgt_sec_rpc, _tgt_ter_rpc) + try: + if migration.intermediate_snaps and not _SKIP_INTERMEDIATE_SNAP_DELETE: + _delete_intermediate_snaps_on_target(migration, tgt_rpc, _tgt_sec_rpc, _tgt_ter_rpc) + _rename_migrated_bdevs(migration, tgt_node, tgt_rpc, _tgt_sec_rpc, _tgt_ter_rpc) + except Exception as e: + return False, False, str(e) return True, False, None ctx = migration.transfer_context or {} @@ -2201,10 +2192,12 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): # Delete intermediate (shrink) snapshots from the target — they are migration # artifacts and do not need to be preserved. No migration flag so SPDK # coalesces and frees their clusters into the child bdev. - if migration.intermediate_snaps and not _SKIP_INTERMEDIATE_SNAP_DELETE: - _delete_intermediate_snaps_on_target(migration, tgt_rpc, tgt_sec_rpc, tgt_ter_rpc) - - _rename_migrated_bdevs(migration, tgt_node, tgt_rpc, tgt_sec_rpc, tgt_ter_rpc) + try: + if migration.intermediate_snaps and not _SKIP_INTERMEDIATE_SNAP_DELETE: + _delete_intermediate_snaps_on_target(migration, tgt_rpc, tgt_sec_rpc, tgt_ter_rpc) + _rename_migrated_bdevs(migration, tgt_node, tgt_rpc, tgt_sec_rpc, tgt_ter_rpc) + except Exception as e: + return False, False, str(e) return True, False, None @@ -2242,7 +2235,7 @@ def _handle_cleanup_target(migration, tgt_node, tgt_rpc, src_rpc=None): _pre_nqn: Optional[str] = None try: _lvol = db.get_lvol_by_id(migration.lvol_id) - _pre_bdev = f"{tgt_node.lvstore}/{_lvol.lvol_bdev}{_MIGRATION_BDEV_SUFFIX}" + _pre_bdev = f"{tgt_node.lvstore}/{_lvol_tgt_bdev_name(_lvol.lvol_bdev)}" _pre_nqn = _lvol.nqn except Exception: _pre_bdev = None From bc45556fb157203a300f3cdf91c8e9632f303579 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Fri, 10 Jul 2026 15:18:43 +0330 Subject: [PATCH 47/49] fix: dead-code cleanup, exception-safety fixes, stale docs --- .../services/hub_controller_manager.py | 3 - .../services/tasks_runner_lvol_migration.py | 350 +++++++++--------- 2 files changed, 167 insertions(+), 186 deletions(-) diff --git a/simplyblock_core/services/hub_controller_manager.py b/simplyblock_core/services/hub_controller_manager.py index 192886bb3..83d8862c2 100644 --- a/simplyblock_core/services/hub_controller_manager.py +++ b/simplyblock_core/services/hub_controller_manager.py @@ -32,9 +32,6 @@ → attach (or reuse) the controller, refresh the idle timer → returns (ctrl_name, hub_bdev, error) - hub_manager.release(src_node_id, tgt_node_id) - → no-op kept for call-site compatibility; idle timer is driven by acquire() - hub_manager.detach_now(src_node_id, tgt_node_id, src_rpc=None) → immediate detach (failure / cancel path), records detach timestamp """ diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index 3d7a5fafb..ff93c0b6a 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -116,10 +116,6 @@ _INTERMEDIATE_POLL_INTERVAL_S = 1 # seconds between stat checks _INTERMEDIATE_POLL_MAX = 300 # max iterations ≈ 5 min -_SKIP_CLEANUP_SOURCE = False -_SKIP_INTERMEDIATE_SNAP_DELETE = False -_SKIP_RENAME_BDEVS = False - def _now_ms(): """Return current wall-clock time as an ISO-8601 string with milliseconds.""" @@ -414,19 +410,22 @@ def _log_spdk_bdev_size(rpc, composite_name, label): def _delete_bdev_blocking(bdev_name, primary_rpc, secondary_rpc=None, tertiary_rpc=None, - timeout_s=120): + timeout_s=120, coalescing=False): """ - Two-phase blocking bdev delete with special_delete=True (non-coalescing). + Two-phase blocking bdev delete. Phase 1 — primary only, sync=False (del_async=False): initiates the async - special delete. special_delete=True tells SPDK to free the bdev's own - clusters without merging them into any child — correct for source cleanup, - rollback, and any path where no child needs to inherit data. + delete. By default (coalescing=False) special_delete=True tells SPDK to + free the bdev's own clusters without merging them into any child — + correct for source cleanup, rollback, and any path where no child needs + to inherit data. Pass coalescing=True when the bdev's child must + inherit its clusters instead (e.g. deleting a migration intermediate + snapshot, where the surviving lvol/snap needs the merged data). Wait — poll bdev_lvol_get_lvol_delete_status on primary until done. Phase 2 — all nodes (primary + secondary + tertiary), sync=True (del_async=True, special_delete=False): finalises the deletion on every replica. """ - ret, _ = primary_rpc.delete_lvol(bdev_name, del_async=False, special_delete=True) + ret, _ = primary_rpc.delete_lvol(bdev_name, del_async=False, special_delete=not coalescing) if not ret: raise RuntimeError(f"delete bdev {bdev_name}: initiation failed") @@ -453,7 +452,7 @@ def _delete_bdev_blocking(bdev_name, primary_rpc, secondary_rpc=None, tertiary_r # Secondary-node helpers # --------------------------------------------------------------------------- -def _get_target_secondary_node(tgt_node, src_node_id=None): +def _get_target_secondary_node(tgt_node, src_node_id): """ Return ``(sec_node, error_string)`` describing how to handle the target's secondary node when creating a new object on the target primary. @@ -466,6 +465,9 @@ def _get_target_secondary_node(tgt_node, src_node_id=None): overlap drain: source is being drained but is still the target's secondary; migration must continue through it - Any other status → (None, err) block creation on primary + + ``src_node_id`` must always be supplied so the overlap-drain case (SUSPENDED + secondary that is also the migration source) is handled correctly. """ if not tgt_node.secondary_node_id: return None, None @@ -486,8 +488,23 @@ def _get_target_secondary_node(tgt_node, src_node_id=None): ) -def _get_target_tertiary_node(tgt_node, src_node_id=None): - """Like _get_target_secondary_node but for the tertiary node.""" +def _get_target_tertiary_node(tgt_node, src_node_id): + """ + Return ``(ter_node, error_string)`` describing how to handle the target's + tertiary node when creating a new object on the target primary. + + Rules mirror _get_target_secondary_node: + - No tertiary configured → (None, None) skip silently + - Tertiary STATUS_ONLINE → (ter_node, None) register on tertiary + - Tertiary STATUS_OFFLINE → (None, None) administratively down, skip + - Tertiary STATUS_SUSPENDED and node == src_node → (ter_node, None) + overlap drain: source is being drained but is still the target's + tertiary; migration must continue through it + - Any other status → (None, err) block creation on primary + + ``src_node_id`` must always be supplied so the overlap-drain case is + handled correctly. + """ if not tgt_node.tertiary_node_id: return None, None try: @@ -506,6 +523,41 @@ def _get_target_tertiary_node(tgt_node, src_node_id=None): ) +def _get_source_secondary_node(src_node): + """ + Return the secondary node of src_node for source cleanup operations, or None. + + During source cleanup the secondary still holds replica data that must be + deleted. SUSPENDED is accepted because a draining node is still reachable + via RPC; OFFLINE is skipped as unreachable. + """ + if not src_node.secondary_node_id: + return None + try: + sec = db.get_storage_node_by_id(src_node.secondary_node_id) + except KeyError: + return None + if sec.status in (StorageNode.STATUS_ONLINE, StorageNode.STATUS_SUSPENDED): + return sec + return None + + +def _get_source_tertiary_node(src_node): + """ + Return the tertiary node of src_node for source cleanup operations, or None. + + Mirrors _get_source_secondary_node: ONLINE or SUSPENDED are both accepted; + OFFLINE is skipped. + """ + if not src_node.tertiary_node_id: + return None + try: + ter = db.get_storage_node_by_id(src_node.tertiary_node_id) + except KeyError: + return None + if ter.status in (StorageNode.STATUS_ONLINE, StorageNode.STATUS_SUSPENDED): + return ter + return None @@ -652,8 +704,15 @@ def _setup_snap_transfer(snap, snap_index, src_node, tgt_node, f"size_in_mib={size_in_mib} (lvol_size_mib={lvol_size_mib})" ) _log_spdk_bdev_size(src_rpc, src_composite, f"SRC snap[{snap_uuid[:8]}] pre-create") - _ndcs = snap.lvol.ndcs if hasattr(snap, 'lvol') and snap.lvol else 0 - _npcs = snap.lvol.npcs if hasattr(snap, 'lvol') and snap.lvol else 0 + # snap.lvol is the embedded copy of the owning lvol captured at snapshot + # creation time; it should always be populated but we degrade gracefully + # for legacy/corrupt records by falling back to pool defaults (0 = use + # the pool's default replication scheme / priority class) rather than + # failing the migration outright. + snap_lvol = snap.lvol + _ndcs = snap_lvol.ndcs if snap_lvol else 0 + _npcs = snap_lvol.npcs if snap_lvol else 0 + _priority_class = snap_lvol.lvol_priority_class if snap_lvol else 0 ret = tgt_rpc.create_lvol(snap_short, size_in_mib, tgt_node.lvstore, ndcs=_ndcs, npcs=_npcs) if not ret: return None, f"Failed to create target lvol for snap {snap_uuid}" @@ -667,31 +726,43 @@ def _setup_snap_transfer(snap, snap_index, src_node, tgt_node, if tgt_sec and sec_rpc: bdev_info = tgt_rpc.get_bdevs(tgt_composite) if not bdev_info: - _delete_bdev_blocking(tgt_composite, tgt_rpc) + try: + _delete_bdev_blocking(tgt_composite, tgt_rpc) + except Exception as e: + logger.warning(f"cleanup target lvol {tgt_composite} (non-fatal): {e}") return None, f"Could not get bdev info for {tgt_composite} after creation" snap_blobid = bdev_info[0]['driver_specific']['lvol']['blobid'] snap_uuid_on_tgt = bdev_info[0]['uuid'] ret_sec = sec_rpc.bdev_lvol_register( snap_short, tgt_node.lvstore, snap_uuid_on_tgt, snap_blobid, - snap.lvol.lvol_priority_class if hasattr(snap, 'lvol') else 0) + _priority_class) if not ret_sec: - _delete_bdev_blocking(tgt_composite, tgt_rpc, sec_rpc) + try: + _delete_bdev_blocking(tgt_composite, tgt_rpc, sec_rpc) + except Exception as e: + logger.warning(f"cleanup target lvol {tgt_composite} (non-fatal): {e}") return None, f"bdev_lvol_register on secondary failed for snap {snap_uuid}" sec_registered = True if tgt_ter and ter_rpc: ret_ter = ter_rpc.bdev_lvol_register( snap_short, tgt_node.lvstore, snap_uuid_on_tgt, snap_blobid, - snap.lvol.lvol_priority_class if hasattr(snap, 'lvol') else 0) + _priority_class) if not ret_ter: - _delete_bdev_blocking(tgt_composite, tgt_rpc, sec_rpc, ter_rpc) + try: + _delete_bdev_blocking(tgt_composite, tgt_rpc, sec_rpc, ter_rpc) + except Exception as e: + logger.warning(f"cleanup target lvol {tgt_composite} (non-fatal): {e}") return None, f"bdev_lvol_register on tertiary failed for snap {snap_uuid}" ter_registered = True # Helper: clean primary, secondary, and tertiary (if registered) on error def _cleanup(): - _delete_bdev_blocking(tgt_composite, tgt_rpc, - secondary_rpc=sec_rpc if sec_registered else None, - tertiary_rpc=ter_rpc if ter_registered else None) + try: + _delete_bdev_blocking(tgt_composite, tgt_rpc, + secondary_rpc=sec_rpc if sec_registered else None, + tertiary_rpc=ter_rpc if ter_registered else None) + except Exception as e: + logger.warning(f"cleanup target lvol {tgt_composite} (non-fatal): {e}") # Step 3: migration flag on primary ret = tgt_rpc.bdev_lvol_set_migration_flag(tgt_composite) @@ -946,14 +1017,14 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): if tgt_sec: sec_rpc = _make_rpc(tgt_sec) elif snap.lvol.ha_type == "ha3": - tgt_sec, sec_err = _get_target_secondary_node(tgt_node) + tgt_sec, sec_err = _get_target_secondary_node(tgt_node, src_node.get_id()) if sec_err: migration.error_message = sec_err migration.write_to_db(db.kv_store) return False, True, _WAIT if tgt_sec: sec_rpc = _make_rpc(tgt_sec) - tgt_ter, ter_err = _get_target_tertiary_node(tgt_node) + tgt_ter, ter_err = _get_target_tertiary_node(tgt_node, src_node.get_id()) if ter_err: migration.error_message = ter_err migration.write_to_db(db.kv_store) @@ -1038,7 +1109,7 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): if ctx.get('stage') == 'parallel_transfer': transfers = ctx['transfers'] # Resolve secondary once for the whole poll pass - tgt_sec, _sec_err = _get_target_secondary_node(tgt_node) + tgt_sec, _sec_err = _get_target_secondary_node(tgt_node, src_node.get_id()) sec_rpc = _make_rpc(tgt_sec) if tgt_sec and not _sec_err else None # Process in snap_index order: add_clone requires predecessor to be # converted first. prev_post_done tracks whether the predecessor has @@ -1168,7 +1239,7 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): tgt_ter = None ter_rpc = None if snap.lvol.ha_type in ("ha", "ha3"): - tgt_sec, sec_err = _get_target_secondary_node(tgt_node) + tgt_sec, sec_err = _get_target_secondary_node(tgt_node, src_node.get_id()) if sec_err: migration.error_message = sec_err migration.write_to_db(db.kv_store) @@ -1176,7 +1247,7 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): if tgt_sec: sec_rpc = _make_rpc(tgt_sec) if snap.lvol.ha_type == "ha3": - tgt_ter, ter_err = _get_target_tertiary_node(tgt_node) + tgt_ter, ter_err = _get_target_tertiary_node(tgt_node, src_node.get_id()) if ter_err: migration.error_message = ter_err migration.write_to_db(db.kv_store) @@ -1220,22 +1291,31 @@ def _handle_snap_copy(migration, src_node, tgt_node, src_rpc, tgt_rpc): for _ in range(_INTERMEDIATE_POLL_MAX): result = src_rpc.bdev_lvol_transfer_stat(src_composite) if result is None: - _delete_bdev_blocking(tgt_composite, tgt_rpc, - secondary_rpc=sec_rpc, tertiary_rpc=ter_rpc) + try: + _delete_bdev_blocking(tgt_composite, tgt_rpc, + secondary_rpc=sec_rpc, tertiary_rpc=ter_rpc) + except Exception as e: + logger.warning(f"cleanup target snap {tgt_composite} (non-fatal): {e}") return False, True, ( f"Transfer stat failed for intermediate snap {snap_uuid}") state = result.get('transfer_state', 'No process') if state == 'Done': break if state in ('Failed', 'No process'): - _delete_bdev_blocking(tgt_composite, tgt_rpc, - secondary_rpc=sec_rpc, tertiary_rpc=ter_rpc) + try: + _delete_bdev_blocking(tgt_composite, tgt_rpc, + secondary_rpc=sec_rpc, tertiary_rpc=ter_rpc) + except Exception as e: + logger.warning(f"cleanup target snap {tgt_composite} (non-fatal): {e}") return False, True, ( f"Intermediate snap transfer {state} for {snap_uuid}") time.sleep(_INTERMEDIATE_POLL_INTERVAL_S) else: - _delete_bdev_blocking(tgt_composite, tgt_rpc, - secondary_rpc=sec_rpc, tertiary_rpc=ter_rpc) + try: + _delete_bdev_blocking(tgt_composite, tgt_rpc, + secondary_rpc=sec_rpc, tertiary_rpc=ter_rpc) + except Exception as e: + logger.warning(f"cleanup target snap {tgt_composite} (non-fatal): {e}") return False, True, ( f"Intermediate snap transfer timed out for {snap_uuid}") @@ -1324,7 +1404,8 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): Creates the target lvol with the same NQN as the source subsystem, connects the source to the target's hub lvol, and issues bdev_lvol_final_migration (synchronous — blocks until SPDK completes the delta copy). On success, - immediately rebuilds the TGT NVMe-oF subsystem and sets STATUS_CUTOVER. + immediately rebuilds the TGT NVMe-oF subsystem and applies the DB records + so clients polling migration status see the TGT endpoints at cutover time. Note: apply_migration_to_db() is NOT called here; it is deferred to the end of PHASE_CLEANUP_SOURCE after source snap deletion is complete. @@ -1341,8 +1422,10 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): tgt_lvol_bdev = _lvol_tgt_bdev_name(lvol.lvol_bdev) tgt_lvol_composite = f"{tgt_node.lvstore}/{tgt_lvol_bdev}" ctx = migration.transfer_context or {} - tgt_sec_rpc = _get_secondary_rpc(tgt_node) - tgt_ter_rpc = _get_tertiary_rpc(tgt_node) + tgt_sec, _ = _get_target_secondary_node(tgt_node, src_node.get_id()) + tgt_sec_rpc = _make_rpc(tgt_sec) if tgt_sec else None + tgt_ter, _ = _get_target_tertiary_node(tgt_node, src_node.get_id()) + tgt_ter_rpc = _make_rpc(tgt_ter) if tgt_ter else None # --- Crash recovery: Done handler was interrupted mid-run --- # bdev_lvol_final_migration is synchronous — it blocks until SPDK completes. @@ -1442,8 +1525,11 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): last_snap = db.get_snapshot_by_id(last_snap_uuid) except KeyError: src_rpc.bdev_nvme_detach_controller(ctrl_name) - _delete_bdev_blocking(tgt_lvol_composite, tgt_rpc, - secondary_rpc=tgt_sec_rpc, tertiary_rpc=tgt_ter_rpc) + try: + _delete_bdev_blocking(tgt_lvol_composite, tgt_rpc, + secondary_rpc=tgt_sec_rpc, tertiary_rpc=tgt_ter_rpc) + except Exception as e: + logger.warning(f"cleanup target lvol {tgt_lvol_composite} (non-fatal): {e}") return False, True, f"Last snapshot {last_snap_uuid} not found" tgt_snap_composite = f"{tgt_node.lvstore}/{_snap_tgt_short_name(last_snap)}" elif migration.snaps_preexisting_on_target: @@ -1463,8 +1549,11 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): break if not tgt_snap_composite: src_rpc.bdev_nvme_detach_controller(ctrl_name) - _delete_bdev_blocking(tgt_lvol_composite, tgt_rpc, - secondary_rpc=tgt_sec_rpc, tertiary_rpc=tgt_ter_rpc) + try: + _delete_bdev_blocking(tgt_lvol_composite, tgt_rpc, + secondary_rpc=tgt_sec_rpc, tertiary_rpc=tgt_ter_rpc) + except Exception as e: + logger.warning(f"cleanup target lvol {tgt_lvol_composite} (non-fatal): {e}") return False, True, f"snapshot {last_snap_uuid} not found on target" # else: no snapshots at all — tgt_snap_composite stays "" and SPDK # performs a full-lvol transfer without a snapshot base. @@ -1503,8 +1592,8 @@ def _handle_lvol_migrate(migration, src_node, tgt_node, src_rpc, tgt_rpc): # non-primary nodes need an explicit call. _clone_tgt_composite = f"{tgt_node.lvstore}/{tgt_lvol_bdev}" for _extra_node in filter(None, [ - _get_target_secondary_node(tgt_node)[0], - _get_target_tertiary_node(tgt_node)[0], + _get_target_secondary_node(tgt_node, src_node.get_id())[0], + _get_target_tertiary_node(tgt_node, src_node.get_id())[0], ]): _ret = _make_rpc(_extra_node).bdev_lvol_add_clone( _clone_tgt_composite, tgt_snap_composite) @@ -1633,11 +1722,8 @@ def _flip_required(rpc, ip, port, trtype, state, label, attempts=3): # create_migration; for plain it is the raw migration lvol. for tgt in tgt_paths: if tgt['node_id'] in overlap_ids: - try: - _swap_namespace(tgt['rpc'], nqn, tgt_ns_bdev, - lvol.uuid, lvol.guid, tgt['node_id'][:8]) - except Exception: - raise + _swap_namespace(tgt['rpc'], nqn, tgt_ns_bdev, + lvol.uuid, lvol.guid, tgt['node_id'][:8]) # Step 5: all TGT paths → correct ANA state at TGT port. # Primary required; secondaries best-effort. @@ -1733,14 +1819,10 @@ def _delete_intermediate_snaps_on_target(migration, tgt_rpc, tgt_sec_rpc=None, t Must be called AFTER apply_migration_to_db() — at that point snap.snap_bdev already holds the target composite path (e.g. LVS_TGT/SNAP_xxxm). - Two-phase coalescing delete: - Phase 1 — primary only, sync=False (del_async=False): initiates the - coalescing operation so SPDK merges the snapshot's clusters into its - child bdev before freeing the snapshot bdev. Using special_delete=True - here would do a non-coalescing unmap that frees the clusters directly, - breaking the child's parent chain and losing pre-migration data. - Phase 2 — all nodes (primary + secondary + tertiary), sync=True - (del_async=True): finalises the deletion on every replica. + Delegates to _delete_bdev_blocking(coalescing=True): the intermediate + snapshot's clusters must be merged into its child bdev before being freed + (special_delete=False), or the child's parent chain breaks and + pre-migration data is lost. """ for snap_uuid in migration.intermediate_snaps: try: @@ -1756,25 +1838,10 @@ def _delete_intermediate_snaps_on_target(migration, tgt_rpc, tgt_sec_rpc=None, t f"Intermediate snap bdev {tgt_composite} absent from target; skipping SPDK delete") else: try: - # Phase 1: initiate coalescing delete on primary (sync=False) - ret, _ = tgt_rpc.delete_lvol(tgt_composite, del_async=False, special_delete=False) - if not ret: - raise RuntimeError( - f"Coalescing delete initiation of {tgt_composite} on primary failed") - else: - # Phase 2: finalize on all nodes (sync=True) - for label, rpc in [("primary", tgt_rpc), - ("secondary", tgt_sec_rpc), - ("tertiary", tgt_ter_rpc)]: - if rpc is None: - continue - try: - rpc.delete_lvol(tgt_composite, del_async=True, special_delete=False) - except Exception as e: - logger.warning( - f"Could not finalize intermediate snap delete {tgt_composite} " - f"on {label}: {e}") - logger.info(f"Deleted intermediate snap bdev {tgt_composite} from target") + _delete_bdev_blocking(tgt_composite, tgt_rpc, + secondary_rpc=tgt_sec_rpc, tertiary_rpc=tgt_ter_rpc, + coalescing=True) + logger.info(f"Deleted intermediate snap bdev {tgt_composite} from target") except Exception as e: logger.warning( f"Could not delete intermediate snap {tgt_composite} from target: {e}") @@ -1786,41 +1853,6 @@ def _delete_intermediate_snaps_on_target(migration, tgt_rpc, tgt_sec_rpc=None, t logger.warning(f"Could not remove intermediate snap {snap_uuid} from DB: {e}") -def _get_secondary_rpc(node): - """Return RPC client for node's online secondary, or None.""" - if not node.secondary_node_id: - return None - try: - sec = db.get_storage_node_by_id(node.secondary_node_id) - if sec.status in (StorageNode.STATUS_ONLINE, StorageNode.STATUS_SUSPENDED): - return _make_rpc(sec) - except KeyError: - pass - return None - - -def _get_tertiary_rpc(node): - """Return RPC client for node's online tertiary, or None.""" - if not node.tertiary_node_id: - return None - try: - ter = db.get_storage_node_by_id(node.tertiary_node_id) - if ter.status == StorageNode.STATUS_ONLINE: - return _make_rpc(ter) - except KeyError: - # Tertiary reference may be stale (node removed/not found); treat as unavailable. - logger.debug( - "Tertiary node not found for node_id=%s tertiary_node_id=%s", - getattr(node, "id", None), - node.tertiary_node_id, - ) - return None - - - - - - def _rename_migrated_bdevs(migration, tgt_node, tgt_rpc, tgt_sec_rpc=None, tgt_ter_rpc=None): """ After migration completes, rename 'm'-suffixed bdevs on the target back to @@ -1834,10 +1866,6 @@ def _rename_migrated_bdevs(migration, tgt_node, tgt_rpc, tgt_sec_rpc=None, tgt_t Must be called AFTER _apply_migration_to_db() — snap.snap_bdev and lvol.lvol_bdev already point to the 'm'-suffixed target paths at that point. """ - if _SKIP_RENAME_BDEVS: - logger.info("_rename_migrated_bdevs: skipped (SKIP_RENAME_BDEVS=True)") - return - lvstore = tgt_node.lvstore preexisting = set(migration.snaps_preexisting_on_target or []) @@ -1950,43 +1978,6 @@ def _rename_with_fallback(current_short, label): if updated: snap.write_to_db(db.kv_store) - # --- Ancestor chain blobs (non-owned snaps) --- - # _apply_migration_to_db added an instances entry with the _m bdev name. - # Rename it and update the entry in place. - for snap_uuid in migration.snaps_migrated: - if snap_uuid in preexisting: - continue - try: - snap = db.get_snapshot_by_id(snap_uuid) - except KeyError: - continue - if snap.lvol.uuid == migration.lvol_id: - continue # owned snap — handled above - - updated = False - for inst in snap.instances: - inst_bdev = inst.get('snap_bdev', '') - if '/' not in inst_bdev: - continue - inst_lvstore, inst_short = inst_bdev.split('/', 1) - if inst_lvstore != lvstore: - continue - if not inst_short.endswith(_MIGRATION_BDEV_SUFFIX): - continue - - try: - target = _rename_with_fallback(inst_short, inst_short) - except Exception as exc: - logger.warning( - f"_rename_migrated_bdevs: ancestor {inst_short} failed: {exc}") - continue - if target: - inst['snap_bdev'] = f"{lvstore}/{target}" - updated = True - - if updated: - snap.write_to_db(db.kv_store) - # --- Lvol --- try: lvol = db.get_lvol_by_id(migration.lvol_id) @@ -2022,24 +2013,6 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): Returns (done: bool, suspend: bool, error: str|None). """ - if _SKIP_CLEANUP_SOURCE: - logger.info("SKIP_CLEANUP_SOURCE flag is set — skipping source cleanup, applying DB only") - _ctx = migration.transfer_context or {} - _apply_migration_to_db( - migration, - tgt_lvol_uuid=_ctx.get('tgt_lvol_uuid'), - tgt_lvol_bdev=_ctx.get('tgt_lvol_bdev')) - _tgt_sec_rpc = _get_secondary_rpc(tgt_node) - _tgt_ter = _get_target_tertiary_node(tgt_node)[0] - _tgt_ter_rpc = _make_rpc(_tgt_ter) if _tgt_ter else None - try: - if migration.intermediate_snaps and not _SKIP_INTERMEDIATE_SNAP_DELETE: - _delete_intermediate_snaps_on_target(migration, tgt_rpc, _tgt_sec_rpc, _tgt_ter_rpc) - _rename_migrated_bdevs(migration, tgt_node, tgt_rpc, _tgt_sec_rpc, _tgt_ter_rpc) - except Exception as e: - return False, False, str(e) - return True, False, None - ctx = migration.transfer_context or {} # --- First entry: initialize cleanup state --- @@ -2098,8 +2071,10 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): migration.transfer_context = ctx migration.write_to_db(db.kv_store) - src_sec_rpc = _get_secondary_rpc(src_node) - src_ter_rpc = _get_tertiary_rpc(src_node) + src_sec = _get_source_secondary_node(src_node) + src_sec_rpc = _make_rpc(src_sec) if src_sec else None + src_ter = _get_source_tertiary_node(src_node) + src_ter_rpc = _make_rpc(src_ter) if src_ter else None # --- Delete source snapshots (blocking, idempotent) --- # Recompute to_delete here so re-entries after crash work without stored pending list. @@ -2114,9 +2089,12 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): bdev_name = source_snap_bdevs[snap_uuid] else: bdev_name = f"{src_node.lvstore}/{_snap_short_name(snap)}" - _delete_bdev_blocking(bdev_name, src_rpc, - secondary_rpc=src_sec_rpc, tertiary_rpc=src_ter_rpc) - logger.info(f"Deleted source bdev {bdev_name}") + try: + _delete_bdev_blocking(bdev_name, src_rpc, + secondary_rpc=src_sec_rpc, tertiary_rpc=src_ter_rpc) + logger.info(f"Deleted source bdev {bdev_name}") + except Exception as e: + logger.warning(f"delete source bdev {bdev_name} (non-fatal): {e}") except KeyError: logger.warning(f"Source snapshot {snap_uuid} not found in DB; skipping") @@ -2185,15 +2163,16 @@ def _handle_cleanup_source(migration, src_node, src_rpc, tgt_node, tgt_rpc): migration, tgt_lvol_uuid=tgt_lvol_uuid, tgt_lvol_bdev=tgt_lvol_bdev): return False, False, "Failed to update DB records after source cleanup" - tgt_sec_rpc = _get_secondary_rpc(tgt_node) - _tgt_ter = _get_target_tertiary_node(tgt_node)[0] - tgt_ter_rpc = _make_rpc(_tgt_ter) if _tgt_ter else None + tgt_sec, _ = _get_target_secondary_node(tgt_node, src_node.get_id()) + tgt_sec_rpc = _make_rpc(tgt_sec) if tgt_sec else None + tgt_ter, _ = _get_target_tertiary_node(tgt_node, src_node.get_id()) + tgt_ter_rpc = _make_rpc(tgt_ter) if tgt_ter else None # Delete intermediate (shrink) snapshots from the target — they are migration # artifacts and do not need to be preserved. No migration flag so SPDK # coalesces and frees their clusters into the child bdev. try: - if migration.intermediate_snaps and not _SKIP_INTERMEDIATE_SNAP_DELETE: + if migration.intermediate_snaps: _delete_intermediate_snaps_on_target(migration, tgt_rpc, tgt_sec_rpc, tgt_ter_rpc) _rename_migrated_bdevs(migration, tgt_node, tgt_rpc, tgt_sec_rpc, tgt_ter_rpc) except Exception as e: @@ -2218,8 +2197,10 @@ def _handle_cleanup_target(migration, tgt_node, tgt_rpc, src_rpc=None): hub_manager.detach_now(migration.source_node_id, tgt_node.get_id(), src_rpc=src_rpc) ctx = migration.transfer_context or {} - tgt_sec_rpc = _get_secondary_rpc(tgt_node) - tgt_ter_rpc = _get_tertiary_rpc(tgt_node) + tgt_sec, _ = _get_target_secondary_node(tgt_node, migration.source_node_id) + tgt_sec_rpc = _make_rpc(tgt_sec) if tgt_sec else None + tgt_ter, _ = _get_target_tertiary_node(tgt_node, migration.source_node_id) + tgt_ter_rpc = _make_rpc(tgt_ter) if tgt_ter else None # --- Step 0: delete dangling target lvol/subsystems from a failed LVOL_MIGRATE --- # Also handles the pre-create case where bdev/subsystems were set up by @@ -2295,9 +2276,12 @@ def _handle_cleanup_target(migration, tgt_node, tgt_rpc, src_rpc=None): f"Target bdev {_lvstore}/{_m_name} not found in any variant; " f"skipping (already cleaned up)") continue - _delete_bdev_blocking(bdev_name, tgt_rpc, - secondary_rpc=tgt_sec_rpc, tertiary_rpc=tgt_ter_rpc) - logger.info(f"Deleted target snapshot bdev {bdev_name}") + try: + _delete_bdev_blocking(bdev_name, tgt_rpc, + secondary_rpc=tgt_sec_rpc, tertiary_rpc=tgt_ter_rpc) + logger.info(f"Deleted target snapshot bdev {bdev_name}") + except Exception as e: + logger.warning(f"delete target snapshot bdev {bdev_name} (non-fatal): {e}") except KeyError: logger.warning(f"Target snapshot {snap_uuid} not found in DB; skipping") From 4d133622546777c8d1103b2bcd82390adbb6ff3c Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Fri, 10 Jul 2026 15:44:44 +0330 Subject: [PATCH 48/49] fixing a linting error --- simplyblock_core/storage_node_ops.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/simplyblock_core/storage_node_ops.py b/simplyblock_core/storage_node_ops.py index a09d4966e..5660e87a5 100755 --- a/simplyblock_core/storage_node_ops.py +++ b/simplyblock_core/storage_node_ops.py @@ -513,7 +513,7 @@ def connect_device(name: str, device: NVMeDevice, node: StorageNode, bdev_names: # Return the bdev name if it exists; otherwise hint with the canonical # ``n1`` so callers (e.g. _connect_to_remote_jm_devs) can poll for # it via get_bdevs. - for bdev in bdev_names: + for bdev in (bdev_names or []): if bdev.startswith(name): return bdev if rpc_client.get_bdevs(bdev_name): From 9a4f7fcedd36190f6a5d5a54b204b8efd2af0da9 Mon Sep 17 00:00:00 2001 From: ebrahim savari <106956085+EbiRider@users.noreply.github.com> Date: Fri, 10 Jul 2026 18:00:28 +0330 Subject: [PATCH 49/49] fix: dedupe migration-target bdev naming into a shared idempotent helper --- .../controllers/migration_controller.py | 17 ++++------------- .../services/tasks_runner_lvol_migration.py | 9 +++------ simplyblock_core/utils/__init__.py | 13 +++++++++++++ 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/simplyblock_core/controllers/migration_controller.py b/simplyblock_core/controllers/migration_controller.py index 746dd69f0..bd4ea620c 100644 --- a/simplyblock_core/controllers/migration_controller.py +++ b/simplyblock_core/controllers/migration_controller.py @@ -54,7 +54,7 @@ from simplyblock_core.models.nvme_connect import NvmeConnectEntry from simplyblock_core.models.snapshot import SnapShot from simplyblock_core.models.storage_node import StorageNode -from simplyblock_core.utils import convert_size +from simplyblock_core.utils import convert_size, lvol_tgt_bdev_name # Note: JobSchedule is not imported directly here; task creation is delegated to # tasks_controller.add_lvol_mig_task() which handles event logging consistently. @@ -216,8 +216,7 @@ def _cleanup_created(migration): return nqn = lvol.nqn - _lvol_base = lvol.lvol_bdev[:-len(_MIGRATION_BDEV_SUFFIX)] if lvol.lvol_bdev.endswith(_MIGRATION_BDEV_SUFFIX) else lvol.lvol_bdev - bdev_short = _lvol_base + _MIGRATION_BDEV_SUFFIX + bdev_short = lvol_tgt_bdev_name(lvol.lvol_bdev) composite = f"{tgt_node.lvstore}/{bdev_short}" # Compute overlap: nodes shared between SRC and TGT paths. @@ -499,7 +498,7 @@ def get_snaps_safe_to_delete_on_source(migration): if lvol.uuid == migration.lvol_id: continue if lvol.cloned_from_snap and lvol.cloned_from_snap in candidates: - _protect_snap_and_ancestors(lvol.cloned_from_snap, candidates) + candidates -= _collect_snap_ancestry(lvol.cloned_from_snap) return candidates @@ -551,17 +550,10 @@ def _collect_snap_ancestry(snap_uuid) -> set: return result -def _protect_snap_and_ancestors(snap_uuid, candidate_set): - """Remove *snap_uuid* and all its ancestors from *candidate_set*.""" - candidate_set -= _collect_snap_ancestry(snap_uuid) - - # --------------------------------------------------------------------------- # Post-migration DB updates # --------------------------------------------------------------------------- -_MIGRATION_BDEV_SUFFIX = constants.LVOL_MIG_BDEV_SUFFIX - def _build_connect_entries(node, port, lvol, nqn, ctrl_loss_tmo, cluster, host_entry, host_nqn): """Build NVMe connect-string entries for each data NIC on *node* at *port*.""" @@ -670,8 +662,7 @@ def create_migration(lvol_id, target_node_id, cluster = db.get_cluster_by_id(tgt_node.cluster_id) tgt_rpc = tgt_node.rpc_client() nqn = lvol.nqn - _lvol_base = lvol.lvol_bdev[:-len(_MIGRATION_BDEV_SUFFIX)] if lvol.lvol_bdev.endswith(_MIGRATION_BDEV_SUFFIX) else lvol.lvol_bdev - bdev_short = _lvol_base + _MIGRATION_BDEV_SUFFIX + bdev_short = lvol_tgt_bdev_name(lvol.lvol_bdev) composite = f"{tgt_node.lvstore}/{bdev_short}" size_in_mib = convert_size(lvol.size, 'MiB') tgt_port = tgt_node.get_lvol_subsys_port(tgt_node.lvstore) diff --git a/simplyblock_core/services/tasks_runner_lvol_migration.py b/simplyblock_core/services/tasks_runner_lvol_migration.py index ff93c0b6a..b59b41304 100644 --- a/simplyblock_core/services/tasks_runner_lvol_migration.py +++ b/simplyblock_core/services/tasks_runner_lvol_migration.py @@ -349,13 +349,10 @@ def _snap_tgt_short_name(snap): def _lvol_tgt_bdev_name(lvol_bdev: str) -> str: """Return the migration-target bdev short name for a writable lvol. - Same strip-then-add logic as _snap_tgt_short_name: prevents 'LVOL_Xmm' - accumulation on back-to-back migrations where lvol_bdev already ends in - the suffix from the previous run. + Thin wrapper around the shared utils.lvol_tgt_bdev_name so existing + call-sites in this module don't need to change. """ - if lvol_bdev.endswith(_MIGRATION_BDEV_SUFFIX): - lvol_bdev = lvol_bdev[:-len(_MIGRATION_BDEV_SUFFIX)] - return lvol_bdev + _MIGRATION_BDEV_SUFFIX + return utils.lvol_tgt_bdev_name(lvol_bdev) def _snap_composite(lvstore, snap): diff --git a/simplyblock_core/utils/__init__.py b/simplyblock_core/utils/__init__.py index a5eee2872..103ad9111 100644 --- a/simplyblock_core/utils/__init__.py +++ b/simplyblock_core/utils/__init__.py @@ -375,6 +375,19 @@ def sum_records(records): return total +def lvol_tgt_bdev_name(lvol_bdev: str) -> str: + """Return the migration-target bdev short name for a writable lvol. + + Idempotent: strips any existing migration suffix before adding one, so + retries or back-to-back migrations (where lvol_bdev already ends in the + suffix from a previous run) never accumulate it (e.g. 'LVOL_Xmm'). + """ + suffix = constants.LVOL_MIG_BDEV_SUFFIX + if lvol_bdev.endswith(suffix): + lvol_bdev = lvol_bdev[:-len(suffix)] + return lvol_bdev + suffix + + def _used_bdev_name_numbers(db_controller, all_lvols=None, all_snapshots=None): used = set() if not all_lvols: