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

StaticHeader::setup Improve error reporting #1443

Closed
Tracked by #1564
tasleson opened this issue Feb 25, 2019 · 1 comment · May be fixed by #1562
Closed
Tracked by #1564

StaticHeader::setup Improve error reporting #1443

tasleson opened this issue Feb 25, 2019 · 1 comment · May be fixed by #1562
Assignees
Labels
sub-issue issue that has a parent

Comments

@tasleson
Copy link
Contributor

tasleson commented Feb 25, 2019

Note: Taken from #1440 (comment)

fn setup<F>(f: &mut F) -> StratisResult<Option<StaticHeader>>
    where
        F: Read + Seek + SyncAll,

This returns an error in two situations:

  • If the metadata seems to indicate that the device is a Stratis deivce, but no well-formed signature could be found.
  • If a well-formed signature was found but an error occurred during the repair of a second, ill-formed, old, or unreadable signature.

I think it is right to return an error in the first case, but I think the second should possibly be handled differently. I think a good option is to enrich the return type to

StratisResult<Option<StaticHeader>, Option<some error thing if repair fails>>

Additionally, taken from: #1440 (comment)

let err_str =  "Appeared to be a Stratis device, but no valid sigblock found";
Err(StratisError::Engine(ErrorEnum::Invalid, err_str.into()))

Actually, better to include both errors, and improve the error messages in the errors returned from sigblock_from_buf.

@mulkieran
Copy link
Member

Closing in favor of #2133, as that separates the action of reading and interpreting the signature buffers from the action of choosing and if necessary repairing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sub-issue issue that has a parent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants