cli: reject missing -kernel up front instead of hanging in SBI [#50]#148
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c9ee169fc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex please review again! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c9ee169fc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Lfan-ke
changed the title
Lfan-ke
force-pushed
the
feat/50-missing-kernel-error
branch
2 times, most recently
from
77497ef to
9366625
Compare
|
@codex review this pr thoroughly again. |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…o#50) Running \`machina -nographic\` (or any invocation that lands in the bundled RustSBI without a guest payload) silently sat inside the firmware forever. To a first-time user this looks like a freeze: the README documents Ctrl-A, X to escape, but a contributor who just typed \`cargo run\` to verify the build had no way to know. Implement the issue's preferred host-side check: after the CLI is fully validated (machine name, per-machine option rejections), detect "no -kernel and no external firmware" and exit 1 with a clear message: machina: no kernel specified. Please use -kernel <path/to/kernel.elf> to provide a guest payload. Without -kernel, machina would sit inside SBI firmware with nothing to run. The check intentionally allows -bios <ext-firmware> without -kernel because a self-contained firmware (U-Boot, custom OpenSBI) can drive its own boot. -bios none and -bios builtin still require -kernel. Tests in tests/src/cli_kernel.rs: - missing_kernel_with_default_bios_errors_with_diagnostic - missing_kernel_with_bios_none_errors - missing_kernel_with_bios_builtin_errors Existing cli_kernel, tools and machine-help tests still pass. Signed-off-by: Leo Cheng <chengkelfan@qq.com>
Lfan-ke
force-pushed
the
feat/50-missing-kernel-error
branch
from
9366625 to
6381cdb
Compare
2 participants
Resolves #50.
What
Implement the issue author's preferred fix (Alternative): detect
`no -kernel and no external firmware` during CLI validation
and exit 1 with a clear diagnostic, instead of silently sitting
inside RustSBI with nothing to run.
Behaviour table
The check is placed in `main()` after machine-name and
per-machine option validation so existing rejections (unknown
machine, LoongArch `-S`/`-gdb`/`-monitor` rejections) still
surface first when both apply.
Diagnostic
```
machina: no kernel specified.
Please use -kernel <path/to/kernel.elf> to provide a guest payload.
Without -kernel, machina would sit inside SBI firmware with nothing
to run.
```
Tests
`tests/src/cli_kernel.rs`:
All existing cli_kernel, tools and machine-help tests continue
to pass.
Test plan