dsg/cloud-hypervisor
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

vmm: remove lock for VmState

Browse source 
The lock doesn't make any sense. There is no shared ownership. All
accesses are already synchronized by accesses on a higher level.

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 2026-02-12 19:03:07 +01:00 • committed by Rob Bradford
parent 9babad0114
commit 03ef7d1991
2 changed files with 22 additions and 44 deletions
Download patch file Download diff file Expand all files Collapse all files

4
vmm/src/lib.rs
View file

@ -1875,7 +1875,7 @@ impl RequestHandler for Vmm {
match &self.vm_config {
Some(vm_config) => {
let state = match &self.vm {
Some(vm) => vm.get_state()?,
Some(vm) => vm.get_state(),
None => VmState::Created,
};
let config = vm_config.lock().unwrap().clone();
@ -2347,7 +2347,7 @@ impl RequestHandler for Vmm {
return e;
}
if vm.get_state().unwrap() == VmState::Paused
if vm.get_state() == VmState::Paused
&& let Err(e) = vm.resume()
{
return e;

62
vmm/src/vm.rs
View file

@ -19,7 +19,7 @@ use std::mem::size_of;
use std::num::Wrapping;
use std::ops::Deref;
use std::os::unix::net::UnixStream;
use std::sync::{Arc, Mutex, RwLock};
use std::sync::{Arc, Mutex};
#[cfg(not(target_arch = "riscv64"))]
use std::time::Instant;
use std::{cmp, result, str, thread};
@ -150,9 +150,6 @@ pub enum Error {
#[error("Cannot enable interrupt controller")]
EnableInterruptController(#[source] interrupt_controller::Error),
#[error("VM state is poisoned")]
PoisonedState,
#[error("Error from device manager")]
DeviceManager(#[source] DeviceManagerError),
@ -511,7 +508,7 @@ pub struct Vm {
threads: Vec<thread::JoinHandle<()>>,
device_manager: Arc<Mutex<DeviceManager>>,
config: Arc<Mutex<VmConfig>>,
state: RwLock<VmState>,
state: VmState,
cpu_manager: Arc<Mutex<cpu::CpuManager>>,
memory_manager: Arc<Mutex<MemoryManager>>,
#[cfg_attr(any(not(feature = "kvm"), target_arch = "aarch64"), allow(dead_code))]
@ -875,7 +872,7 @@ impl Vm {
None
};
let vm_state = if snapshot.is_some() {
let state = if snapshot.is_some() {
VmState::Paused
} else {
VmState::Created
@ -888,7 +885,7 @@ impl Vm {
device_manager,
config,
threads: Vec::with_capacity(1),
state: RwLock::new(vm_state),
state,
cpu_manager,
memory_manager,
vm,
@ -1669,10 +1666,9 @@ impl Vm {
}
pub fn shutdown(&mut self) -> Result<()> {
let mut state = self.state.try_write().map_err(|_| Error::PoisonedState)?;
let new_state = VmState::Shutdown;
state.valid_transition(new_state)?;
self.state.valid_transition(new_state)?;
// Wake up the DeviceManager threads so they will get terminated cleanly
self.device_manager
@ -1691,7 +1687,7 @@ impl Vm {
for thread in self.threads.drain(..) {
thread.join().map_err(Error::ThreadCleanup)?;
}
*state = new_state;
self.state = new_state;
Ok(())
}
@ -2378,7 +2374,7 @@ impl Vm {
pub fn boot(&mut self) -> Result<()> {
trace_scoped!("Vm::boot");
let current_state = self.get_state()?;
let current_state = self.state;
if current_state == VmState::Paused {
return self.resume().map_err(Error::Resume);
}
@ -2562,8 +2558,7 @@ impl Vm {
.start_boot_vcpus(new_state == VmState::BreakPoint)
.map_err(Error::CpuManager)?;
let mut state = self.state.try_write().map_err(|_| Error::PoisonedState)?;
*state = new_state;
self.state = new_state;
Ok(())
}
@ -2593,12 +2588,9 @@ impl Vm {
Arc::clone(&self.config)
}
/// Get the VM state. Returns an error if the state is poisoned.
pub fn get_state(&self) -> Result<VmState> {
/// Get the VM state.
pub fn get_state(&self) -> VmState {
self.state
.try_read()
.map_err(|_| Error::PoisonedState)
.map(|state| *state)
}
/// Gets the actual size of the balloon.
@ -2850,13 +2842,8 @@ impl Vm {
impl Pausable for Vm {
fn pause(&mut self) -> std::result::Result<(), MigratableError> {
event!("vm", "pausing");
let mut state = self
.state
.try_write()
.map_err(|e| MigratableError::Pause(anyhow!("Could not get VM state: {e}")))?;
let new_state = VmState::Paused;
state
self.state
.valid_transition(new_state)
.map_err(|e| MigratableError::Pause(anyhow!("Invalid transition: {e:?}")))?;
@ -2883,7 +2870,7 @@ impl Pausable for Vm {
.pause()
.map_err(|e| MigratableError::Pause(anyhow!("Could not pause the VM: {e}")))?;
*state = new_state;
self.state = new_state;
event!("vm", "paused");
Ok(())
@ -2891,14 +2878,10 @@ impl Pausable for Vm {
fn resume(&mut self) -> std::result::Result<(), MigratableError> {
event!("vm", "resuming");
let current_state = self.get_state().unwrap();
let mut state = self
.state
.try_write()
.map_err(|e| MigratableError::Resume(anyhow!("Could not get VM state: {e}")))?;
let current_state = self.get_state();
let new_state = VmState::Running;
state
self.state
.valid_transition(new_state)
.map_err(|e| MigratableError::Resume(anyhow!("Invalid transition: {e:?}")))?;
@ -2921,7 +2904,7 @@ impl Pausable for Vm {
self.device_manager.lock().unwrap().resume()?;
// And we're back to the Running state.
*state = new_state;
self.state = new_state;
event!("vm", "resumed");
Ok(())
}
@ -2953,8 +2936,7 @@ impl Snapshottable for Vm {
}
}
let current_state = self.get_state().unwrap();
if current_state != VmState::Paused {
if self.get_state() != VmState::Paused {
return Err(MigratableError::Snapshot(anyhow!(
"Trying to snapshot while VM is running"
)));
@ -3116,20 +3098,16 @@ impl Debuggable for Vm {
}
fn debug_pause(&mut self) -> std::result::Result<(), DebuggableError> {
if *self.state.read().unwrap() == VmState::Running {
if self.state == VmState::Running {
self.pause().map_err(DebuggableError::Pause)?;
}
let mut state = self
.state
.try_write()
.map_err(|_| DebuggableError::PoisonedState)?;
*state = VmState::BreakPoint;
self.state = VmState::BreakPoint;
Ok(())
}
fn debug_resume(&mut self) -> std::result::Result<(), DebuggableError> {
if *self.state.read().unwrap() == VmState::BreakPoint {
if self.state == VmState::BreakPoint {
self.resume().map_err(DebuggableError::Pause)?;
}
@ -3209,7 +3187,7 @@ impl GuestDebuggable for Vm {
}
}
match self.get_state().unwrap() {
match self.get_state() {
VmState::Running => {
self.pause().map_err(GuestDebuggableError::Pause)?;
resume = true;