Skip to content

fix: remove encrypted dm device with linear dm if existing#282

Closed
mantissahz wants to merge 1 commit into
longhorn:mainfrom
mantissahz:issue12842
Closed

fix: remove encrypted dm device with linear dm if existing#282
mantissahz wants to merge 1 commit into
longhorn:mainfrom
mantissahz:issue12842

Conversation

@mantissahz
Copy link
Copy Markdown

@mantissahz mantissahz commented May 14, 2026

Which issue(s) this PR fixes:

Issue # longhorn/longhorn#11510

What this PR does / why we need it:

The linear dm created with the initiator will block the encrypted dm device each other in v2 volumes.
Then the linear dm device can not be reloaded, removed, and block the initiator to be re-created when the engine crashes unexpectedly.

Special notes for your reviewer:

Additional documentation or context

https://ci.longhorn.io/job/private/job/longhorn-e2e-test/7870/
https://ci.longhorn.io/job/private/job/longhorn-e2e-test/7872/
https://ci.longhorn.io/job/private/job/longhorn-e2e-test/7873/

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses an issue in v2 volumes where an encrypted device-mapper (DM) mapping can prevent the linear DM mapping from being removed/reloaded, which can then block initiator recreation after unexpected engine crashes.

Changes:

  • Extend util.DmsetupRemove to accept multiple DM devices in one call.
  • Update initiator teardown logic to attempt removing a -encrypted DM device alongside the linear DM device.
  • Update the dmsetup CLI wrapper to use the new DmsetupRemove([]string, ...) signature.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
pkg/util/dmsetup.go Changes DmsetupRemove to accept a list of device identifiers.
pkg/initiator/initiator.go Adds logic to detect and remove an -encrypted DM device along with the linear DM device; adds not-found error detection.
app/cmd/dmsetup/dmsetup.go Updates CLI remove command to pass a slice to DmsetupRemove.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/initiator/initiator.go
Comment thread pkg/initiator/initiator.go Outdated
Comment thread pkg/initiator/initiator.go Outdated
Comment thread pkg/util/dmsetup.go
Comment thread pkg/initiator/initiator.go Outdated
@mantissahz mantissahz marked this pull request as draft May 14, 2026 13:28
@mantissahz mantissahz force-pushed the issue12842 branch 4 times, most recently from 611dfb0 to 454408a Compare May 14, 2026 16:49
ref: longhorn/longhorn 12842

Signed-off-by: James Lu <james.lu@suse.com>
@derekbit
Copy link
Copy Markdown
Member

Which issue(s) this PR fixes:

Issue # longhorn/longhorn#12842

Wrong ticket number?

What this PR does / why we need it:

The linear dm created with the initiator will block the encrypted dm device each other in v2 volumes. Then the linear dm device can not be reloaded, removed, and block the initiator to be re-created when the engine crashes unexpectedly.

Special notes for your reviewer:

Additional documentation or context

ci.longhorn.io/job/private/job/longhorn-e2e-test/7870 ci.longhorn.io/job/private/job/longhorn-e2e-test/7872 ci.longhorn.io/job/private/job/longhorn-e2e-test/7873

@mantissahz
Copy link
Copy Markdown
Author

Which issue(s) this PR fixes:
Issue # longhorn/longhorn#12842
Wrong ticket number?

Yes, it should be longhorn/longhorn@11510

@mantissahz mantissahz marked this pull request as ready for review May 18, 2026 00:27
Comment thread pkg/types/types.go
ShallowCopyStateError = "error"

ExecuteTimeout = 180 * time.Second
ExecuteShortTimeout = 5 * time.Second
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it enough? Why not use ExecuteTimeout?

Comment on lines +1232 to +1236
i.logger.WithError(err).Debugf("Failed to remove linear dm device: %v", dmDevPath)
if isRemoveDmDevNotFoundError(err) {
return nil
}
return err
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
i.logger.WithError(err).Debugf("Failed to remove linear dm device: %v", dmDevPath)
if isRemoveDmDevNotFoundError(err) {
return nil
}
return err
if isRemoveDmDevNotFoundError(err) {
return nil
}
i.logger.WithError(err).Errorf("Failed to remove linear dm device %v", dmDevPath)
return err

Comment thread pkg/initiator/initiator.go
@mantissahz
Copy link
Copy Markdown
Author

Try to fix this issue at the volume controller by this PR longhorn/longhorn-manager#4736

@mantissahz mantissahz closed this May 18, 2026
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.

3 participants