-
Notifications
You must be signed in to change notification settings - Fork 419
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
vmm: http: graceful shutdown of the http api thread #6248
vmm: http: graceful shutdown of the http api thread #6248
Conversation
20e8a0b
to
98e8753
Compare
@alex-matei the CI configuration has changed so please rebase your work on the "main" branch |
98e8753
to
6f904f5
Compare
@rbradford Done, rebased on top of main. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good thanks!
vmm/src/lib.rs
Outdated
@@ -455,25 +456,27 @@ pub fn start_vmm_thread( | |||
None => None, | |||
}; | |||
|
|||
if let Some(http_path) = http_path { | |||
api::start_http_path_thread( | |||
let http_thread = if let Some(http_path) = http_path { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I would have named this http_api_thread_handle
so you don't need to alias it when you pass it to the function below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, I changed to http_api_handle.
vmm/src/api/http/mod.rs
Outdated
// Retrieve seccomp filter for API thread | ||
let api_seccomp_filter = get_seccomp_filter(seccomp_action, Thread::HttpApi, hypervisor_type) | ||
.map_err(VmmError::CreateSeccompFilter)?; | ||
|
||
thread::Builder::new() | ||
let api_shutdown_fd = EventFd::new(libc::EFD_NONBLOCK).map_err(VmmError::EventFdCreate)?; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Drop this line of whitespace
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
This commit ensures that the HttpApi thread flushes all the responses before the application shuts down. Without this step, in case of a VmmShutdown request the application might terminate before the thread sends a response. Fixes: cloud-hypervisor#6247 Signed-off-by: Alexandru Matei <alexandru.matei@uipath.com>
6f904f5
to
70eb129
Compare
This commit ensures that the HttpApi thread flushes all the responses before the application shuts down. Without this step, in case of a VmmShutdown request the application might terminate before the thread sends a response.
Fixes: #6247