Improve HTTP parsing#110
Conversation
Luap99
left a comment
Luap99
left a comment
There was a problem hiding this comment.
Not a maintainer of course but seems good to me overall, a lot better than just checking for POST
| }; | ||
|
|
||
| match state { | ||
| "Stop" | "HardStop" => { |
There was a problem hiding this comment.
Is there no hard shut down for libkrun implemented? I guess practically from my tests we could sigkill the process?
in podman so far we have a 90s timeout for a graceful shutdown and then make another request with HardStop, if the VM ever hung in shutdown this will not be able to kill it and we leak the VM.
(anyhow outside of the scope of this PR just found it weird we treat them the same)
There was a problem hiding this comment.
No, we don't have a hard shut down yet. Both SIGTERM and SIGKILL are a bit dangerous as we risk leaving in-flight virtio-blk buffers unwritten.
In 2.0 we'll have a way to orderly exit the VMM without the guest cooperation.
| ) | ||
| } | ||
|
|
||
| fn parse_state_value(json: &str) -> Option<&str> { |
There was a problem hiding this comment.
I guess you want to avoid a json dep? I guess this is not fully spec compliant parsing either but likely gets the job good enough done.
There was a problem hiding this comment.
Yes, but on a second thought, given that we already added httpparse, we could also add a popular JSON crate, like serde.
Instead of doing manual parsing of HTTP requests, rely on a mature crate like httparse to do the job. Also, actually parse the REST endpoint and contents to make sure we're acting on the right request. Fixes: podman-container-tools/podman#29084 Signed-off-by: Sergio Lopez <slp@redhat.com>
This is a patch release to include the fix in HTTP parsing. Signed-off-by: Sergio Lopez <slp@redhat.com>
3 participants
Instead of doing manual parsing of HTTP requests, rely on a mature crate like httparse to do the job. Also, actually parse the REST endpoint and contents to make sure we're acting on the right request.
Fixes: podman-container-tools/podman#29084