Skip to content

kvm: extract RBD handle teardown into null-safe helper methods - #4

Merged
calvix merged 1 commit into
fix/rbd-snapshot-exclusive-lock-leakfrom
fix/rbd-snapshot-exclusive-lock-leak-refactor
Aug 11, 2026
Merged

kvm: extract RBD handle teardown into null-safe helper methods#4
calvix merged 1 commit into
fix/rbd-snapshot-exclusive-lock-leakfrom
fix/rbd-snapshot-exclusive-lock-leak-refactor

Conversation

@calvix

@calvix calvix commented Aug 10, 2026

Copy link
Copy Markdown
Owner

Addresses the review comment on the RBD exclusive-lock fix:

io and image are torn down in nested try-catch constructions in a finally clause of the parent try-catch clause. I think this should be re-structured to be in called methods. In addition the same pattern happens below. a lot of re-use can be gained

The inline nested try-catch teardown blocks in the finally clauses of takeRbdVolumeSnapshotOfStoppedVm and createRBDvolumeFromRBDSnapshot are extracted into two reusable, null-safe helpers that log but never throw (the never-throws contract is documented in the javadoc, making them safe to call from finally blocks; naming follows the codebase's plain-verb helper style, e.g. closeChannel, cleanupVMNetworks, destroyStoragePool):

  • closeRbdImage — 3 call sites
  • destroyRadosIoCtx — 2 call sites

The snapUnprotect block stays inline: it is not a close, occurs once, and logs at ERROR severity.

Deliberately out of scope, to keep the lock fix minimal and backportable:

  • deleteSnapshot's bare non-null-safe finally — pre-existing since 2015 (10ae2af) with its own additional leak path (io leaks if rbd.open throws); candidate for a follow-up where these helpers make it a 3-line change
  • replacing the inline Rados connection setup in createRBDvolumeFromRBDSnapshot with radosConnect

No behavior change. All 39 tests in KVMStorageProcessorTest pass, including the two regression tests verifying close/ioCtxDestroy are invoked on the failure path.

Address review feedback: the nested try-catch teardown blocks in the
finally clauses of takeRbdVolumeSnapshotOfStoppedVm and
createRBDvolumeFromRBDSnapshot are extracted into two reusable,
null-safe helpers that log but never throw:

- closeRbdImage (3 call sites)
- destroyRadosIoCtx (2 call sites)

No behavior change.
@calvix
calvix force-pushed the fix/rbd-snapshot-exclusive-lock-leak-refactor branch from 71efc11 to f4ddf95 Compare August 11, 2026 07:08
@calvix
calvix merged commit f3ff36d into fix/rbd-snapshot-exclusive-lock-leak Aug 11, 2026
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant