monitor: implement system_powerdown MMP command (closes #88)#116
monitor: implement system_powerdown MMP command (closes #88)#116mvanhorn wants to merge 1 commit intogevico:mainfrom
Conversation
The QMP system_powerdown command shuts down the VM. machina has no separate ACPI shutdown path, so dispatch reuses the existing quit() path on MonitorService -- semantically equivalent for the embedded monitor and consistent with how the existing "quit" command reaches the shutdown signal. Add a regression test in tests/src/monitor.rs that mirrors the existing test_mmp_quit, asserting the empty success envelope and that is_quit_requested() flips to true after dispatch. Closes gevico#88 Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
|
@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: 9f9ba1fe3f
ℹ️ 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".
| // QMP system_powerdown shuts down the VM. machina has no | ||
| // separate ACPI shutdown path, so reuse quit() which signals | ||
| // the run loop to stop -- semantically equivalent here. | ||
| s.quit(); |
There was a problem hiding this comment.
dispatch() currently maps system_powerdown directly to quit(), which forces the VM to terminate immediately instead of requesting an in-guest powerdown flow. In QMP, system_powerdown is a soft guest request that may be ignored or delayed, so conflating it with quit can cause management clients that expect graceful shutdown semantics to trigger abrupt exits (and possible guest data loss) once they switch from handling CommandNotFound to using this command.
Useful? React with 👍 / 👎.
2 participants
Summary
Closes #88
The MMP dispatcher accepted
qmp_capabilities,query-status,stop,cont,quit,query-cpus-fast, andsystem_reset(deferred).system_powerdownreturnedCommandNotFound, which prevented QMP clients from issuing the standard graceful-shutdown command against machina.Changes
monitor/src/mmp.rs: add a"system_powerdown"arm todispatch()that reuses the existingMonitorService::quit()path. machina has no separate ACPI shutdown path, so reusing the run-loop quit signal is the semantically equivalent behavior the issue called out.monitor/src/mmp.rs: extendhandle_connection()to terminate the TCP session when it seessystem_powerdown(previously onlyquitdid this) so a client that issuessystem_powerdownover TCP no longer leaves the connection stuck and the server returning toincoming()despite quit being requested.tests/src/monitor.rs: addtest_mmp_system_powerdown, mirroring the existingtest_mmp_quit. Asserts the empty success envelope andis_quit_requested()flips after dispatch.Verification
Per the workshop description, this is a 4-line dispatch addition; the diff is 18 added lines including the regression test.
cargo build -p machina-monitoris clean.make clippyandmake testcould not be run on this machine (the JIT crateaccel/requires a host x86-64 target viaextern "sysv64"); CI on the gevico/machina x86-64 runners will exercise the full quality gates.Issue ref: #88