From 6bda6541be7b3052da6d74d2dd6908ca7e6326d9 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Wed, 10 Dec 2025 06:48:50 +0100 Subject: [PATCH] vmm: cleanup &Mutex parameters In [0] we refactored some Arc> parameters to &Mutex> 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 On-behalf-of: SAP philipp.schuster@sap.com --- vmm/src/console_devices.rs | 20 ++++++++++---------- vmm/src/cpu.rs | 7 +++---- vmm/src/lib.rs | 15 ++++++++------- vmm/src/vm.rs | 7 +++---- 4 files changed, 24 insertions(+), 25 deletions(-) diff --git a/vmm/src/console_devices.rs b/vmm/src/console_devices.rs index 70bcabae5..76655d6c1 100644 --- a/vmm/src/console_devices.rs +++ b/vmm/src/console_devices.rs @@ -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( fd: RawFd, f: F, - original_termios_opt: &Mutex>, + original_termios_opt: &mut Option, ) -> 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( 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( fn set_raw_mode( f: &dyn AsRawFd, - original_termios_opt: &Mutex>, + original_termios_opt: &mut Option, ) -> ConsoleDeviceResult<()> { modify_mode( f.as_raw_fd(), @@ -179,6 +178,7 @@ fn dup_stdout() -> vmm_sys_util::errno::Result { pub(crate) fn pre_create_console_devices(vmm: &mut Vmm) -> ConsoleDeviceResult { 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 { 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 { @@ -239,7 +239,7 @@ pub(crate) fn pre_create_console_devices(vmm: &mut Vmm) -> ConsoleDeviceResult { 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 ConsoleDeviceResult { 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 => { diff --git a/vmm/src/cpu.rs b/vmm/src/cpu.rs index e7c8a699f..ea030fb0c 100644 --- a/vmm/src/cpu.rs +++ b/vmm/src/cpu.rs @@ -922,11 +922,9 @@ impl CpuManager { pub fn configure_vcpu( &self, - vcpu: &Mutex, + vcpu: &mut Vcpu, boot_setup: Option<(EntryPoint, &GuestMemoryAtomic)>, ) -> 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) diff --git a/vmm/src/lib.rs b/vmm/src/lib.rs index f34585a68..cb98fd1e0 100644 --- a/vmm/src/lib.rs +++ b/vmm/src/lib.rs @@ -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) -> 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(()) } diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index 79dca313c..721c490b8 100644 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -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)?; }