vmm: cleanup &Mutex parameters

In [0] we refactored some Arc<Mutex<T>> parameters to &Mutex<T>> to
satisfy clippy's needless_pass_by_value lint. Nevertheless, this is also
not so idiomatic, so as a follow-up, we put the responsibility to lock
objects to the caller side (only where this is not strictly needed by
the callee).

While on it, I also tried to pass vm_config directly into
pre_create_console_devices() which would clean up some code, but then
we have interleaving mutable and immutable borrows of the Vmm, which
are denied by the borrow checker.

Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
This commit is contained in:
Philipp Schuster
2025-12-10 06:48:50 +01:00
committed by Rob Bradford
parent 4592f37bcf
commit 6bda6541be
4 changed files with 24 additions and 25 deletions

View File

@@ -16,7 +16,7 @@ use std::os::fd::{AsRawFd, FromRawFd, RawFd};
use std::os::unix::fs::OpenOptionsExt;
use std::os::unix::net::UnixListener;
use std::path::PathBuf;
use std::sync::{Arc, Mutex};
use std::sync::Arc;
use std::{io, result};
use libc::{TCSANOW, cfmakeraw, isatty, tcgetattr, tcsetattr, termios};
@@ -76,7 +76,7 @@ pub struct ConsoleInfo {
fn modify_mode<F: FnOnce(&mut termios)>(
fd: RawFd,
f: F,
original_termios_opt: &Mutex<Option<termios>>,
original_termios_opt: &mut Option<termios>,
) -> vmm_sys_util::errno::Result<()> {
// SAFETY: safe because we check the return value of isatty.
if unsafe { isatty(fd) } != 1 {
@@ -91,7 +91,6 @@ fn modify_mode<F: FnOnce(&mut termios)>(
if ret < 0 {
return vmm_sys_util::errno::errno_result();
}
let mut original_termios_opt = original_termios_opt.lock().unwrap();
if original_termios_opt.is_none() {
original_termios_opt.replace(termios);
}
@@ -109,7 +108,7 @@ fn modify_mode<F: FnOnce(&mut termios)>(
fn set_raw_mode(
f: &dyn AsRawFd,
original_termios_opt: &Mutex<Option<termios>>,
original_termios_opt: &mut Option<termios>,
) -> ConsoleDeviceResult<()> {
modify_mode(
f.as_raw_fd(),
@@ -179,6 +178,7 @@ fn dup_stdout() -> vmm_sys_util::errno::Result<File> {
pub(crate) fn pre_create_console_devices(vmm: &mut Vmm) -> ConsoleDeviceResult<ConsoleInfo> {
let vm_config = vmm.vm_config.as_mut().unwrap().clone();
let mut vmconfig = vm_config.lock().unwrap();
let mut original_termios_opt = vmm.original_termios_opt.lock().unwrap();
let console_info = ConsoleInfo {
console_main_fd: match vmconfig.console.mode {
@@ -190,7 +190,7 @@ pub(crate) fn pre_create_console_devices(vmm: &mut Vmm) -> ConsoleDeviceResult<C
ConsoleOutputMode::Pty => {
let (main_fd, sub_fd, path) =
create_pty().map_err(ConsoleDeviceError::CreateConsoleDevice)?;
set_raw_mode(&sub_fd.as_raw_fd(), &vmm.original_termios_opt)?;
set_raw_mode(&sub_fd.as_raw_fd(), &mut original_termios_opt)?;
vmconfig.console.file = Some(path.clone());
vmm.console_resize_pipe = Some(Arc::new(
listen_for_sigwinch_on_tty(
@@ -221,7 +221,7 @@ pub(crate) fn pre_create_console_devices(vmm: &mut Vmm) -> ConsoleDeviceResult<C
}
// Make sure stdout is in raw mode, if it's a terminal.
set_raw_mode(&stdout, &vmm.original_termios_opt)?;
set_raw_mode(&stdout, &mut original_termios_opt)?;
ConsoleOutput::Tty(Arc::new(stdout))
}
ConsoleOutputMode::Socket => {
@@ -239,7 +239,7 @@ pub(crate) fn pre_create_console_devices(vmm: &mut Vmm) -> ConsoleDeviceResult<C
ConsoleOutputMode::Pty => {
let (main_fd, sub_fd, path) =
create_pty().map_err(ConsoleDeviceError::CreateConsoleDevice)?;
set_raw_mode(&sub_fd.as_raw_fd(), &vmm.original_termios_opt)?;
set_raw_mode(&sub_fd.as_raw_fd(), &mut original_termios_opt)?;
vmconfig.serial.file = Some(path.clone());
ConsoleOutput::Pty(Arc::new(main_fd))
}
@@ -255,7 +255,7 @@ pub(crate) fn pre_create_console_devices(vmm: &mut Vmm) -> ConsoleDeviceResult<C
let stdout = dup_stdout().map_err(ConsoleDeviceError::DupFd)?;
// Make sure stdout is in raw mode, if it's a terminal.
set_raw_mode(&stdout, &vmm.original_termios_opt)?;
set_raw_mode(&stdout, &mut original_termios_opt)?;
ConsoleOutput::Tty(Arc::new(stdout))
}
@@ -277,14 +277,14 @@ pub(crate) fn pre_create_console_devices(vmm: &mut Vmm) -> ConsoleDeviceResult<C
ConsoleOutputMode::Pty => {
let (main_fd, sub_fd, path) =
create_pty().map_err(ConsoleDeviceError::CreateConsoleDevice)?;
set_raw_mode(&sub_fd.as_raw_fd(), &vmm.original_termios_opt)?;
set_raw_mode(&sub_fd.as_raw_fd(), &mut original_termios_opt)?;
vmconfig.debug_console.file = Some(path.clone());
ConsoleOutput::Pty(Arc::new(main_fd))
}
ConsoleOutputMode::Tty => {
let out =
dup_stdout().map_err(|e| ConsoleDeviceError::CreateConsoleDevice(e.into()))?;
set_raw_mode(&out, &vmm.original_termios_opt)?;
set_raw_mode(&out, &mut original_termios_opt)?;
ConsoleOutput::Tty(Arc::new(out))
}
ConsoleOutputMode::Socket => {

View File

@@ -922,11 +922,9 @@ impl CpuManager {
pub fn configure_vcpu(
&self,
vcpu: &Mutex<Vcpu>,
vcpu: &mut Vcpu,
boot_setup: Option<(EntryPoint, &GuestMemoryAtomic<GuestMemoryMmap>)>,
) -> Result<()> {
let mut vcpu = vcpu.lock().unwrap();
#[cfg(feature = "sev_snp")]
if self.sev_snp_enabled {
if let Some((kernel_entry_point, _)) = boot_setup {
@@ -1406,7 +1404,8 @@ impl CpuManager {
cmp::Ordering::Greater => {
let vcpus = self.create_vcpus(desired_vcpus, None)?;
for vcpu in vcpus {
self.configure_vcpu(&vcpu, None)?;
let mut vcpu = vcpu.lock().unwrap();
self.configure_vcpu(&mut vcpu, None)?;
}
self.activate_vcpus(desired_vcpus, true, None)?;
Ok(true)

View File

@@ -1014,7 +1014,8 @@ impl Vmm {
.unwrap()
.landlock_enable
{
apply_landlock(self.vm_config.as_ref().unwrap().as_ref()).map_err(|e| {
let mut config = self.vm_config.as_ref().unwrap().lock().unwrap();
apply_landlock(&mut config).map_err(|e| {
MigratableError::MigrateReceive(anyhow!("Error applying landlock: {e:?}"))
})?;
}
@@ -1486,8 +1487,8 @@ impl Vmm {
.unwrap()
.landlock_enable
{
apply_landlock(self.vm_config.as_ref().unwrap().as_ref())
.map_err(VmError::ApplyLandlock)?;
let mut config = self.vm_config.as_ref().unwrap().lock().unwrap();
apply_landlock(&mut config).map_err(VmError::ApplyLandlock)?;
}
// Now we can restore the rest of the VM.
@@ -1606,8 +1607,8 @@ impl Vmm {
}
}
fn apply_landlock(vm_config: &Mutex<VmConfig>) -> result::Result<(), LandlockError> {
vm_config.lock().unwrap().apply_landlock()?;
fn apply_landlock(vm_config: &mut VmConfig) -> result::Result<(), LandlockError> {
vm_config.apply_landlock()?;
Ok(())
}
@@ -1628,8 +1629,8 @@ impl RequestHandler for Vmm {
.as_ref()
.is_some_and(|config| config.lock().unwrap().landlock_enable)
{
apply_landlock(self.vm_config.as_ref().unwrap().as_ref())
.map_err(VmError::ApplyLandlock)?;
let mut config = self.vm_config.as_ref().unwrap().lock().unwrap();
apply_landlock(&mut config).map_err(VmError::ApplyLandlock)?;
}
Ok(())
}

View File

@@ -2390,16 +2390,15 @@ impl Vm {
for vcpu in vcpus {
let guest_memory = &self.memory_manager.lock().as_ref().unwrap().guest_memory();
let boot_setup = entry_point.map(|e| (e, guest_memory));
let mut vcpu = vcpu.lock().unwrap();
self.cpu_manager
.lock()
.unwrap()
.configure_vcpu(&vcpu, boot_setup)
.configure_vcpu(&mut vcpu, boot_setup)
.map_err(Error::CpuManager)?;
#[cfg(target_arch = "aarch64")]
vcpu.lock()
.unwrap()
.set_gic_redistributor_addr(redist_addr[2], redist_addr[3])
vcpu.set_gic_redistributor_addr(redist_addr[2], redist_addr[3])
.map_err(Error::CpuManager)?;
}