From 1091494320d92ee683d2efab6c058897a05b8257 Mon Sep 17 00:00:00 2001 From: Alexandru Matei Date: Thu, 22 Feb 2024 11:41:14 +0200 Subject: [PATCH] vmm: http: graceful shutdown of the http api thread 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 Signed-off-by: Alexandru Matei --- src/main.rs | 7 +++++++ vmm/src/api/http/mod.rs | 37 +++++++++++++++++++++++++++++++------ vmm/src/lib.rs | 17 +++++++++++------ vmm/src/seccomp_filters.rs | 1 + 4 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/main.rs b/src/main.rs index b9e2a1551..5ec09c940 100644 --- a/src/main.rs +++ b/src/main.rs @@ -20,6 +20,7 @@ use std::sync::{Arc, Mutex}; use thiserror::Error; #[cfg(feature = "dbus_api")] use vmm::api::dbus::{dbus_api_graceful_shutdown, DBusApiOptions}; +use vmm::api::http::http_api_graceful_shutdown; use vmm::api::ApiAction; use vmm::config; use vmm_sys_util::eventfd::EventFd; @@ -82,6 +83,8 @@ enum Error { LogFileCreation(std::io::Error), #[error("Error setting up logger: {0}")] LoggerSetup(log::SetLoggerError), + #[error("Failed to gracefully shutdown http api: {0}")] + HttpApiShutdown(#[source] vmm::Error), } struct Logger { @@ -760,6 +763,10 @@ fn start_vmm(cmd_arguments: ArgMatches) -> Result, Error> { .map_err(Error::ThreadJoin)? .map_err(Error::VmmThread)?; + if let Some(api_handle) = vmm_thread_handle.http_api_handle { + http_api_graceful_shutdown(api_handle).map_err(Error::HttpApiShutdown)? + } + #[cfg(feature = "dbus_api")] if let Some(chs) = vmm_thread_handle.dbus_shutdown_chs { dbus_api_graceful_shutdown(chs); diff --git a/vmm/src/api/http/mod.rs b/vmm/src/api/http/mod.rs index 2eac530f7..a34df8a17 100644 --- a/vmm/src/api/http/mod.rs +++ b/vmm/src/api/http/mod.rs @@ -16,7 +16,9 @@ use crate::seccomp_filters::{get_seccomp_filter, Thread}; use crate::{Error as VmmError, Result}; use core::fmt; use hypervisor::HypervisorType; -use micro_http::{Body, HttpServer, MediaType, Method, Request, Response, StatusCode, Version}; +use micro_http::{ + Body, HttpServer, MediaType, Method, Request, Response, ServerError, StatusCode, Version, +}; use once_cell::sync::Lazy; use seccompiler::{apply_filter, SeccompAction}; use serde_json::Error as SerdeError; @@ -33,6 +35,8 @@ use vmm_sys_util::eventfd::EventFd; pub mod http_endpoint; +pub type HttpApiHandle = (thread::JoinHandle>, EventFd); + /// Errors associated with VMM management #[derive(Debug)] pub enum HttpError { @@ -297,12 +301,19 @@ fn start_http_thread( seccomp_action: &SeccompAction, exit_evt: EventFd, hypervisor_type: HypervisorType, -) -> Result>> { +) -> Result { // 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)?; + let api_shutdown_fd_clone = api_shutdown_fd.try_clone().unwrap(); + + server + .add_kill_switch(api_shutdown_fd_clone) + .map_err(VmmError::CreateApiServer)?; + + let thread = thread::Builder::new() .name("http-server".to_string()) .spawn(move || { // Apply seccomp filter for API thread. @@ -329,6 +340,10 @@ fn start_http_thread( } } } + Err(ServerError::ShutdownEvent) => { + server.flush_outgoing_writes(); + return; + } Err(e) => { error!( "HTTP server error on retrieving incoming request. Error: {}", @@ -346,7 +361,9 @@ fn start_http_thread( Ok(()) }) - .map_err(VmmError::HttpThreadSpawn) + .map_err(VmmError::HttpThreadSpawn)?; + + Ok((thread, api_shutdown_fd)) } pub fn start_http_path_thread( @@ -356,12 +373,13 @@ pub fn start_http_path_thread( seccomp_action: &SeccompAction, exit_evt: EventFd, hypervisor_type: HypervisorType, -) -> Result>> { +) -> Result { let socket_path = PathBuf::from(path); let socket_fd = UnixListener::bind(socket_path).map_err(VmmError::CreateApiServerSocket)?; // SAFETY: Valid FD just opened let server = unsafe { HttpServer::new_from_fd(socket_fd.into_raw_fd()) } .map_err(VmmError::CreateApiServer)?; + start_http_thread( server, api_notifier, @@ -379,7 +397,7 @@ pub fn start_http_fd_thread( seccomp_action: &SeccompAction, exit_evt: EventFd, hypervisor_type: HypervisorType, -) -> Result>> { +) -> Result { // SAFETY: Valid FD let server = unsafe { HttpServer::new_from_fd(fd) }.map_err(VmmError::CreateApiServer)?; start_http_thread( @@ -391,3 +409,10 @@ pub fn start_http_fd_thread( hypervisor_type, ) } + +pub fn http_api_graceful_shutdown(http_handle: HttpApiHandle) -> Result<()> { + let (api_thread, api_shutdown_fd) = http_handle; + + api_shutdown_fd.write(1).unwrap(); + api_thread.join().map_err(VmmError::ThreadCleanup)? +} diff --git a/vmm/src/lib.rs b/vmm/src/lib.rs index db90799ff..d55c346ef 100644 --- a/vmm/src/lib.rs +++ b/vmm/src/lib.rs @@ -27,6 +27,7 @@ use crate::vm::{Error as VmError, Vm, VmState}; use anyhow::anyhow; #[cfg(feature = "dbus_api")] use api::dbus::{DBusApiOptions, DBusApiShutdownChannels}; +use api::http::HttpApiHandle; use libc::{tcsetattr, termios, EFD_NONBLOCK, SIGINT, SIGTERM, TCSANOW}; use memory_manager::MemoryManagerSnapshotData; use pci::PciBdf; @@ -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_api_handle = if let Some(http_path) = http_path { + Some(api::start_http_path_thread( http_path, api_event_clone, api_sender, seccomp_action, exit_event, hypervisor_type, - )?; + )?) } else if let Some(http_fd) = http_fd { - api::start_http_fd_thread( + Some(api::start_http_fd_thread( http_fd, api_event_clone, api_sender, seccomp_action, exit_event, hypervisor_type, - )?; - } + )?) + } else { + None + }; #[cfg(feature = "guest_debug")] if let Some(debug_path) = debug_path { @@ -493,6 +496,7 @@ pub fn start_vmm_thread( thread_handle: thread, #[cfg(feature = "dbus_api")] dbus_shutdown_chs, + http_api_handle, }) } @@ -523,6 +527,7 @@ pub struct VmmThreadHandle { pub thread_handle: thread::JoinHandle>, #[cfg(feature = "dbus_api")] pub dbus_shutdown_chs: Option, + pub http_api_handle: Option, } pub struct Vmm { diff --git a/vmm/src/seccomp_filters.rs b/vmm/src/seccomp_filters.rs index 6201f5ea7..da84a8c8c 100644 --- a/vmm/src/seccomp_filters.rs +++ b/vmm/src/seccomp_filters.rs @@ -838,6 +838,7 @@ fn http_api_thread_rules() -> Result)>, BackendError> (libc::SYS_sched_yield, vec![]), (libc::SYS_sigaltstack, vec![]), (libc::SYS_write, vec![]), + (libc::SYS_rt_sigprocmask, vec![]), ]) }