Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metadata rework #3274

Closed
wants to merge 30 commits into from
Closed

Conversation

jbaublitz
Copy link
Member

@jbaublitz jbaublitz force-pushed the metadata-rework branch 2 times, most recently from 832136b to 966514b Compare June 21, 2024 19:09
Copy link
Member

@mulkieran mulkieran left a comment

Choose a reason for hiding this comment

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

One more remark on errors when encountering failures when the problem is bad encryption information...

None
};
log_on_failure!(
device.activate_handle().activate_by_passphrase(
Copy link
Member

Choose a reason for hiding this comment

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

Supposing you get the correct keyslot, but also supply a bad passphrase, the error message is very low-level: Cryptsetup error: IO error occurred: Operation not permitted (os error 1). It might be better to map to something a bit more user-friendly here as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

This error is not specific to this PR and existed previously. I am willing to put up another PR that we can merge separately, but I don't think it belongs here.

Copy link
Member

@mulkieran mulkieran left a comment

Choose a reason for hiding this comment

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

Request about the client side of stratis-min

}

Ok(engine.start_pool(id, unlock_method).await?.is_changed())
Ok(engine
Copy link
Member

Choose a reason for hiding this comment

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

Because of the change here, I think you will also want a change in src/bin/stratis-min/stratis-min.rs specifically around the error message "--prompt and an unlock_method of clevis are mutually exclusive". I think that should no longer be true after this PR, because it is not true on the D-Bus side.

This test code here also illustrates the current behavior: #3635 .

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.

4 participants