vmm: remove unused mutex in api

This patch removes locks in VmCreate request and VmInfo response
since we needn't use a lock here and should ensure that internal
implementation is transparent to the runtime.

Signed-off-by: Songqian Li <sionli@tencent.com>
This commit is contained in:
Songqian Li 2024-09-18 20:06:22 +08:00 committed by Rob Bradford
parent 7eb70730d1
commit cc9899e09d
6 changed files with 29 additions and 32 deletions

View File

@ -9,7 +9,6 @@ use once_cell::sync::Lazy;
use std::os::unix::io::AsRawFd; use std::os::unix::io::AsRawFd;
use std::path::PathBuf; use std::path::PathBuf;
use std::sync::mpsc::{channel, Receiver}; use std::sync::mpsc::{channel, Receiver};
use std::sync::{Arc, Mutex};
use std::thread; use std::thread;
use vm_migration::MigratableError; use vm_migration::MigratableError;
use vmm::api::{ use vmm::api::{
@ -81,7 +80,7 @@ fn generate_request(bytes: &[u8]) -> Option<Request> {
struct StubApiRequestHandler; struct StubApiRequestHandler;
impl RequestHandler for StubApiRequestHandler { impl RequestHandler for StubApiRequestHandler {
fn vm_create(&mut self, _: Arc<Mutex<VmConfig>>) -> Result<(), VmError> { fn vm_create(&mut self, _: Box<VmConfig>) -> Result<(), VmError> {
Ok(()) Ok(())
} }
@ -120,7 +119,7 @@ impl RequestHandler for StubApiRequestHandler {
fn vm_info(&self) -> Result<VmInfoResponse, VmError> { fn vm_info(&self) -> Result<VmInfoResponse, VmError> {
Ok(VmInfoResponse { Ok(VmInfoResponse {
config: Arc::new(Mutex::new(VmConfig { config: Box::new(VmConfig {
cpus: CpusConfig { cpus: CpusConfig {
boot_vcpus: 1, boot_vcpus: 1,
max_vcpus: 1, max_vcpus: 1,
@ -194,7 +193,7 @@ impl RequestHandler for StubApiRequestHandler {
preserved_fds: None, preserved_fds: None,
landlock_enable: false, landlock_enable: false,
landlock_rules: None, landlock_rules: None,
})), }),
state: VmState::Running, state: VmState::Running,
memory_actual_size: 0, memory_actual_size: 0,
device_tree: None, device_tree: None,

View File

@ -15,7 +15,7 @@ use signal_hook::consts::SIGSYS;
use std::fs::File; use std::fs::File;
use std::os::unix::io::{AsRawFd, FromRawFd, RawFd}; use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
use std::sync::mpsc::channel; use std::sync::mpsc::channel;
use std::sync::{Arc, Mutex}; use std::sync::Mutex;
use std::{env, io}; use std::{env, io};
use thiserror::Error; use thiserror::Error;
#[cfg(feature = "dbus_api")] #[cfg(feature = "dbus_api")]
@ -787,7 +787,7 @@ fn start_vmm(cmd_arguments: ArgMatches) -> Result<Option<String>, Error> {
.send( .send(
api_evt.try_clone().unwrap(), api_evt.try_clone().unwrap(),
api_request_sender, api_request_sender,
Arc::new(Mutex::new(vm_config)), Box::new(vm_config),
) )
.map_err(Error::VmCreate)?; .map_err(Error::VmCreate)?;
vmm::api::VmBoot vmm::api::VmBoot

View File

@ -20,7 +20,7 @@ use hypervisor::HypervisorType;
use seccompiler::{apply_filter, SeccompAction}; use seccompiler::{apply_filter, SeccompAction};
use std::panic::AssertUnwindSafe; use std::panic::AssertUnwindSafe;
use std::sync::mpsc::Sender; use std::sync::mpsc::Sender;
use std::sync::{Arc, Mutex}; use std::sync::Arc;
use std::thread; use std::thread;
use vmm_sys_util::eventfd::EventFd; use vmm_sys_util::eventfd::EventFd;
use zbus::fdo::{self, Result}; use zbus::fdo::{self, Result};
@ -201,7 +201,7 @@ impl DBusApi {
let api_sender = self.clone_api_sender().await; let api_sender = self.clone_api_sender().await;
let api_notifier = self.clone_api_notifier()?; let api_notifier = self.clone_api_notifier()?;
let mut vm_config: VmConfig = serde_json::from_str(&vm_config).map_err(api_error)?; let mut vm_config: Box<VmConfig> = serde_json::from_str(&vm_config).map_err(api_error)?;
if let Some(ref mut nets) = vm_config.net { if let Some(ref mut nets) = vm_config.net {
if nets.iter().any(|net| net.fds.is_some()) { if nets.iter().any(|net| net.fds.is_some()) {
@ -212,11 +212,9 @@ impl DBusApi {
} }
} }
blocking::unblock(move || { blocking::unblock(move || VmCreate.send(api_notifier, api_sender, vm_config))
VmCreate.send(api_notifier, api_sender, Arc::new(Mutex::new(vm_config))) .await
}) .map_err(api_error)?;
.await
.map_err(api_error)?;
Ok(()) Ok(())
} }

View File

@ -18,7 +18,6 @@ use micro_http::{Body, Method, Request, Response, StatusCode, Version};
use std::fs::File; use std::fs::File;
use std::os::unix::io::IntoRawFd; use std::os::unix::io::IntoRawFd;
use std::sync::mpsc::Sender; use std::sync::mpsc::Sender;
use std::sync::{Arc, Mutex};
use vmm_sys_util::eventfd::EventFd; use vmm_sys_util::eventfd::EventFd;
// /api/v1/vm.create handler // /api/v1/vm.create handler
@ -36,7 +35,7 @@ impl EndpointHandler for VmCreate {
match &req.body { match &req.body {
Some(body) => { Some(body) => {
// Deserialize into a VmConfig // Deserialize into a VmConfig
let mut vm_config: VmConfig = match serde_json::from_slice(body.raw()) let mut vm_config: Box<VmConfig> = match serde_json::from_slice(body.raw())
.map_err(HttpError::SerdeJsonDeserialize) .map_err(HttpError::SerdeJsonDeserialize)
{ {
Ok(config) => config, Ok(config) => config,
@ -53,7 +52,7 @@ impl EndpointHandler for VmCreate {
} }
match crate::api::VmCreate match crate::api::VmCreate
.send(api_notifier, api_sender, Arc::new(Mutex::new(vm_config))) .send(api_notifier, api_sender, vm_config)
.map_err(HttpError::ApiError) .map_err(HttpError::ApiError)
{ {
Ok(_) => Response::new(Version::Http11, StatusCode::NoContent), Ok(_) => Response::new(Version::Http11, StatusCode::NoContent),

View File

@ -51,7 +51,6 @@ use serde::{Deserialize, Serialize};
use std::fmt::Display; use std::fmt::Display;
use std::io; use std::io;
use std::sync::mpsc::{channel, RecvError, SendError, Sender}; use std::sync::mpsc::{channel, RecvError, SendError, Sender};
use std::sync::{Arc, Mutex};
use vm_migration::MigratableError; use vm_migration::MigratableError;
use vmm_sys_util::eventfd::EventFd; use vmm_sys_util::eventfd::EventFd;
@ -210,10 +209,10 @@ impl Display for ApiError {
#[derive(Clone, Deserialize, Serialize)] #[derive(Clone, Deserialize, Serialize)]
pub struct VmInfoResponse { pub struct VmInfoResponse {
pub config: Arc<Mutex<VmConfig>>, pub config: Box<VmConfig>,
pub state: VmState, pub state: VmState,
pub memory_actual_size: u64, pub memory_actual_size: u64,
pub device_tree: Option<Arc<Mutex<DeviceTree>>>, pub device_tree: Option<DeviceTree>,
} }
#[derive(Clone, Deserialize, Serialize)] #[derive(Clone, Deserialize, Serialize)]
@ -287,7 +286,7 @@ pub enum ApiResponsePayload {
pub type ApiResponse = Result<ApiResponsePayload, ApiError>; pub type ApiResponse = Result<ApiResponsePayload, ApiError>;
pub trait RequestHandler { pub trait RequestHandler {
fn vm_create(&mut self, config: Arc<Mutex<VmConfig>>) -> Result<(), VmError>; fn vm_create(&mut self, config: Box<VmConfig>) -> Result<(), VmError>;
fn vm_boot(&mut self) -> Result<(), VmError>; fn vm_boot(&mut self) -> Result<(), VmError>;
@ -827,7 +826,7 @@ impl ApiAction for VmCounters {
pub struct VmCreate; pub struct VmCreate;
impl ApiAction for VmCreate { impl ApiAction for VmCreate {
type RequestBody = Arc<Mutex<VmConfig>>; type RequestBody = Box<VmConfig>;
type ResponseBody = (); type ResponseBody = ();
fn request( fn request(

View File

@ -1239,11 +1239,11 @@ fn apply_landlock(vm_config: Arc<Mutex<VmConfig>>) -> result::Result<(), Landloc
} }
impl RequestHandler for Vmm { impl RequestHandler for Vmm {
fn vm_create(&mut self, config: Arc<Mutex<VmConfig>>) -> result::Result<(), VmError> { fn vm_create(&mut self, config: Box<VmConfig>) -> result::Result<(), VmError> {
// We only store the passed VM config. // We only store the passed VM config.
// The VM will be created when being asked to boot it. // The VM will be created when being asked to boot it.
if self.vm_config.is_none() { if self.vm_config.is_none() {
self.vm_config = Some(config); self.vm_config = Some(Arc::new(Mutex::new(*config)));
self.console_info = self.console_info =
Some(pre_create_console_devices(self).map_err(VmError::CreateConsoleDevices)?); Some(pre_create_console_devices(self).map_err(VmError::CreateConsoleDevices)?);
@ -1554,23 +1554,25 @@ impl RequestHandler for Vmm {
fn vm_info(&self) -> result::Result<VmInfoResponse, VmError> { fn vm_info(&self) -> result::Result<VmInfoResponse, VmError> {
match &self.vm_config { match &self.vm_config {
Some(config) => { Some(vm_config) => {
let state = match &self.vm { let state = match &self.vm {
Some(vm) => vm.get_state()?, Some(vm) => vm.get_state()?,
None => VmState::Created, None => VmState::Created,
}; };
let config = vm_config.lock().unwrap().clone();
let config = Arc::clone(config); let mut memory_actual_size = config.memory.total_size();
let mut memory_actual_size = config.lock().unwrap().memory.total_size();
if let Some(vm) = &self.vm { if let Some(vm) = &self.vm {
memory_actual_size -= vm.balloon_size(); memory_actual_size -= vm.balloon_size();
} }
let device_tree = self.vm.as_ref().map(|vm| vm.device_tree()); let device_tree = self
.vm
.as_ref()
.map(|vm| vm.device_tree().lock().unwrap().clone());
Ok(VmInfoResponse { Ok(VmInfoResponse {
config, config: Box::new(config),
state, state,
memory_actual_size, memory_actual_size,
device_tree, device_tree,
@ -2164,8 +2166,8 @@ mod unit_tests {
.unwrap() .unwrap()
} }
fn create_dummy_vm_config() -> Arc<Mutex<VmConfig>> { fn create_dummy_vm_config() -> Box<VmConfig> {
Arc::new(Mutex::new(VmConfig { Box::new(VmConfig {
cpus: CpusConfig { cpus: CpusConfig {
boot_vcpus: 1, boot_vcpus: 1,
max_vcpus: 1, max_vcpus: 1,
@ -2242,7 +2244,7 @@ mod unit_tests {
preserved_fds: None, preserved_fds: None,
landlock_enable: false, landlock_enable: false,
landlock_rules: None, landlock_rules: None,
})) })
} }
#[test] #[test]