Fix permanent VDI activation lock leak after host crash#804
Open
olivierlambert wants to merge 1 commit into
Open
Fix permanent VDI activation lock leak after host crash#804olivierlambert wants to merge 1 commit into
olivierlambert wants to merge 1 commit into
Conversation
The 'activating' VDI sm_config key used by blktap2 as a pool-wide activation lock is released in a finally clause, which protects against exceptions but not against the host dying mid-activation (fence, power loss, watchdog). The leaked key then fails every subsequent activation of that VDI pool-wide with MAP_DUPLICATE_KEY, and blocks GC relinking. Unlike the sibling host_<ref> attach keys, it carried no owner and no cleanup path ever removed it: xe cannot touch it (sm-config is read-only in the CLI) and only raw VDI.remove_from_sm_config calls could recover. Encode the owning host ref as the key's value (instead of 'True') so staleness can be arbitrated, and clean the key up in the two places that already give host_<ref> keys their crash-recovery treatment: - resetvdis.reset_sr, run on every sr_attach, now removes an 'activating' entry owned by the re-attaching host, so a crashed host cleans up after itself as soon as it replugs its PBDs; - resetvdis.reset_vdi now removes the entry when --force is given or when the recorded owner is no longer part of the pool, making 'resetvdis single <uuid>' a supported remedy for stale locks left by hosts that never come back. Legacy bare 'True' values written by older code cannot be attributed to an owner and are deliberately only cleared by --force.
olivierlambert
force-pushed
the
fix-stale-activating-lock
branch
from
July 14, 2026 20:51
2865ff0 to
a70508a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #803.
The
activatingVDI sm_config key used by blktap2 as a pool-wide activation lock is released in afinallyclause, which protects against exceptions but not against the host dying mid-activation (HA fence, power loss, watchdog). The leaked key then fails every subsequent activation of that VDI pool-wide withMAP_DUPLICATE_KEYand permanently stalls GC relinking, and no cleanup path ever removes it — unlike the siblinghost_<ref>attach keys, which get crash-recovery treatment both inresetvdis.reset_sr()and in xapi'srefresh_local_vdi_activations.xecannot clear it either (VDIsm-configis a read-only map in the CLI); only rawVDI.remove_from_sm_configcalls could recover. See the linked issue for the full analysis and reproduction.This PR gives
activatingthe same owner encoding and crash-recovery treatment, without renaming the key (the GC'sDB_VDI_ACTIVATINGcheck and the snapshot sm_config filter inLVHDSR.pydepend on the literal name):_add_tag()stores the owning host ref as the key's value instead of'True', so staleness can be attributed;resetvdis.reset_sr(), already run on everysr_attach, now removes anactivatingentry owned by the re-attaching host — a crashed host cleans up after itself as soon as it replugs its PBDs, before anything can attempt an activation;resetvdis.reset_vdi()now removes the entry when--forceis given or when the recorded owner is no longer part of the pool, makingresetvdis single <uuid>a supported remedy for stale locks left by hosts that never come back.Legacy bare
'True'values written by older code cannot be attributed to an owner and are deliberately only cleared by--force, so mixed pools during rolling upgrade behave exactly as before.Acquisition-time arbitration (stealing the key on
MAP_DUPLICATE_KEYwhen nohost_*key is present) was considered and rejected: a concurrent starter can observe the window between the two adjacentadd_to_sm_configcalls of a healthy activation and steal a live lock, allowing two RW tapdisks on shared storage.Testing: new unit tests in
tests/test_resetvdis.pycover the reset_sr owner-match/mismatch/legacy cases and the reset_vdi force/invalid-owner/legacy cases;tests/test_blktap2.pyexpectations updated for the new value. Full suite passes with no new failures (the 5 failures intest_HBASR/test_cleanupare pre-existing on master in my environment).