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

Consider alternatives to using unwrap() #100

Open
abrestic-rivos opened this issue Sep 16, 2022 · 0 comments
Open

Consider alternatives to using unwrap() #100

abrestic-rivos opened this issue Sep 16, 2022 · 0 comments

Comments

@abrestic-rivos
Copy link
Collaborator

Our usage of unwrap() generally falls into one of two categories:

  1. During startup when initializing Salus and loading the host VM.
  2. At runtime when we've expected we've made the necessary guarantees for an operation to succeed.

For (1), we should probably propagate these errors up to main() and let it expect() or panic!() as appropriate.

For (2), I think we should consider something like a warn!() or bug!() macro that dumps a message to the console and taints or poisons the TSM as an indication that it may be in an inconsistent state, similar to WARN_ON in Linux. TVM's could then opt into (or out of) running on a tainted TSM. While the end result to the user might be the same -- TVMs can't run and the TSM needs to be reset -- it would at least provide a better defined failure state than panicking.

rbradford added a commit to rbradford/salus that referenced this issue Mar 31, 2023
Replace the use of .expect() for errors during launch with the returning
of errors instead and explicit printing of error message.

This necessitates the split of the init and main functions into wrapper
versions that handle the error and a version that does the work.

See: rivosinc#100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
rbradford added a commit to rbradford/salus that referenced this issue Mar 31, 2023
Replace the use of .expect() for errors during launch with the returning
of errors instead and explicit printing of error message.

This necessitates the split of the init and main functions into wrapper
versions that handle the error and a version that does the work.

See: rivosinc#100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
rbradford added a commit to rbradford/salus that referenced this issue Mar 31, 2023
Replace the use of .expect() for errors during launch with the returning
of errors instead and explicit printing of error message.

This necessitates the split of the init and main functions into wrapper
versions that handle the error and a version that does the work.

See: rivosinc#100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
rbradford added a commit to rbradford/salus that referenced this issue Mar 31, 2023
Replace the use of .expect() for errors during launch with the returning
of errors instead and explicit printing of error message.

This necessitates the split of the init and main functions into wrapper
versions that handle the error and a version that does the work.

See: rivosinc#100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
abrestic-rivos pushed a commit that referenced this issue Mar 31, 2023
Replace the use of .expect() for errors during launch with the returning
of errors instead and explicit printing of error message.

This necessitates the split of the init and main functions into wrapper
versions that handle the error and a version that does the work.

See: #100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
rbradford added a commit to rbradford/salus that referenced this issue Mar 31, 2023
Replace with an error which comes in two flavours: one kind for missing
required properties and another for missing child nodes.

With this new error available make use of it in the primary_main()
method.

See: rivosinc#100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
rbradford added a commit to rbradford/salus that referenced this issue Mar 31, 2023
Use this new error in the primary_init() method via the main module's
Error enum

Temporarily use a .expect() in the _secondary_init as it has not been
split as per primary_init().

See: rivosinc#100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
rbradford added a commit to rbradford/salus that referenced this issue Mar 31, 2023
Split secondary_init() into a version that the asm code jumps to that
acts as a wrapper around another function that generated a Result type.

See: rivosinc#100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
rbradford added a commit to rbradford/salus that referenced this issue Mar 31, 2023
Replace with an error which comes in two flavours: one kind for missing
required properties and another for missing child nodes.

With this new error available make use of it in the primary_main()
method.

See: rivosinc#100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
rbradford added a commit to rbradford/salus that referenced this issue Mar 31, 2023
Use this new error in the primary_init() method via the main module's
Error enum

Temporarily use a .expect() in the _secondary_init as it has not been
split as per primary_init().

See: rivosinc#100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
rbradford added a commit to rbradford/salus that referenced this issue Mar 31, 2023
Split secondary_init() into a version that the asm code jumps to that
acts as a wrapper around another function that generated a Result type.

See: rivosinc#100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
abrestic-rivos pushed a commit that referenced this issue Mar 31, 2023
Replace with an error which comes in two flavours: one kind for missing
required properties and another for missing child nodes.

With this new error available make use of it in the primary_main()
method.

See: #100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
abrestic-rivos pushed a commit that referenced this issue Mar 31, 2023
Use this new error in the primary_init() method via the main module's
Error enum

Temporarily use a .expect() in the _secondary_init as it has not been
split as per primary_init().

See: #100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
abrestic-rivos pushed a commit that referenced this issue Mar 31, 2023
Split secondary_init() into a version that the asm code jumps to that
acts as a wrapper around another function that generated a Result type.

See: #100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
rbradford added a commit to rbradford/salus that referenced this issue Apr 3, 2023
Some of the errors implementd fmt::Display so use that in preference
over fmt::Debug when presenting the error messages.

See: rivosinc#100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
rbradford added a commit to rbradford/salus that referenced this issue Apr 3, 2023
This removes the use of .expect() during the allocation of the per CPU
stack memory.

See: rivosinc#100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
rbradford added a commit to rbradford/salus that referenced this issue Apr 3, 2023
Propagate the errors, rather than use .expect() for the creation of the
page allocator for the hypervisor's memory.

See: rivosinc#100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
rbradford added a commit to rbradford/salus that referenced this issue Apr 3, 2023
Propagate the errors, rather than use .expect() for the creation of the
page allocator for the hypervisor's memory.

See: rivosinc#100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
abrestic-rivos pushed a commit that referenced this issue Apr 4, 2023
Some of the errors implementd fmt::Display so use that in preference
over fmt::Debug when presenting the error messages.

See: #100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
abrestic-rivos pushed a commit that referenced this issue Apr 4, 2023
This removes the use of .expect() during the allocation of the per CPU
stack memory.

See: #100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
abrestic-rivos pushed a commit that referenced this issue Apr 4, 2023
Propagate the errors, rather than use .expect() for the creation of the
page allocator for the hypervisor's memory.

See: #100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
rbradford added a commit to rbradford/salus that referenced this issue Apr 5, 2023
Remove some bare .unwrap() calls of results in create_heap()

See: rivosinc#100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
rbradford added a commit to rbradford/salus that referenced this issue Apr 5, 2023
Replace bare .unwrap() calls with versions that generate and propagate
errors up.

See: rivosinc#100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
dgreid pushed a commit that referenced this issue Apr 5, 2023
Remove some bare .unwrap() calls of results in create_heap()

See: #100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
dgreid pushed a commit that referenced this issue Apr 5, 2023
Replace bare .unwrap() calls with versions that generate and propagate
errors up.

See: #100

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
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

No branches or pull requests

1 participant