Skip to content

Commit df38923

Browse files
phip1611arctic-alpaca
authored andcommitted
vmm: keep virtio activation alive in migration
Live migration can deadlock if the guest triggers a virtio device activation while the migration worker owns the VM. The failure shows up when starting live migrations during boot and firmware startup, where the guest can reset and reinitialize virtio devices while precopy is running. In the failing case, the source log shows a pending virtio activation that never completes: 8.115833s _virtio-pci-net_0: Needs activation; returning barrier 8.115854s vmm/src/vm.rs:464 -- Waiting for barrier 24.875452s Entering downtime phase 24.875481s stopping vcpu throttling thread ... vCPU thread did not respond in 10ms to signal - retrying vCPU thread did not respond in 20ms to signal - retrying ... thread 'throttle-vcpu' (1029) panicked ... Pause(Error signalling vCPUs: Timeout when waiting for signal to be acknowledged) The vCPU blocks on the activation barrier and never reaches the normal pause checkpoint. Later, migration enters downtime and stops the vCPU throttle thread. In the failing case, that thread is still inside a CpuManager::pause() call, which waits for every vCPU to acknowledge the signal. The blocked vCPU never does, so the pause times out. Fix this by storing the DeviceManager inside VmOwnership::Migration. This keeps just enough state on the VMM thread to drain pending virtio activations while the migration worker owns the Vm. The barrier logic stays unchanged. The VMM now releases the same activation barrier during migration that it already released before migration started. This keeps the guest from getting stuck in the activation wait and lets the later pause succeed. Co-authored-by: Leander Kohler <leander.kohler@cyberus-technology.de> On-behalf-of: SAP leander.kohler@sap.com Signed-off-by: Leander Kohler <leander.kohler@cyberus-technology.de> On-behalf-of: SAP philipp.schuster@sap.com Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de> (cherry picked from commit 4d727c4) Signed-off-by: Julian Schindel <julian.schindel@cyberus-technology.de>
1 parent 6e061a2 commit df38923

2 files changed

Lines changed: 35 additions & 7 deletions

File tree

‎vmm/src/lib.rs‎

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use std::panic::AssertUnwindSafe;
2020
use std::path::PathBuf;
2121
use std::sync::atomic::{AtomicBool, Ordering};
2222
use std::sync::mpsc::{Receiver, RecvError, SendError, Sender};
23-
use std::sync::{Arc, Mutex};
23+
use std::sync::{Arc, Mutex, Weak};
2424
use std::time::Duration;
2525
#[cfg(not(target_arch = "riscv64"))]
2626
use std::time::Instant;
@@ -70,6 +70,7 @@ use crate::config::{MemoryRestoreMode, RestoreConfig, add_to_config};
7070
use crate::coredump::GuestDebuggable;
7171
#[cfg(feature = "kvm")]
7272
use crate::cpu::IS_IN_SHUTDOWN;
73+
use crate::device_manager::DeviceManager;
7374
use crate::landlock::Landlock;
7475
use crate::memory_manager::MemoryManager;
7576
use crate::migration::transport::{
@@ -643,13 +644,18 @@ pub struct VmmThreadHandle {
643644

644645
/// Models the current ownership and associated state of the VM from the
645646
/// perspective of the VMM.
646-
pub enum VmOwnership {
647+
enum VmOwnership {
647648
Owned(Vm),
648649
/// The VM is temporarily owned by an ongoing migration worker.
650+
///
651+
/// We deliberately do not use shared access to the VM to prevent a whole
652+
/// class of race conditions.
649653
Migration {
650654
migration_worker_handle: MigrationWorkerHandle,
651655
/// Snapshot returned while the VMM cannot inspect the worker-owned VM.
652656
vm_info_response: VmInfoResponse,
657+
/// Access to VM state needed during migration.
658+
device_manager: Weak<Mutex<DeviceManager>>,
653659
},
654660
None,
655661
}
@@ -2225,12 +2231,27 @@ impl Vmm {
22252231
}
22262232
}
22272233
EpollDispatch::ActivateVirtioDevices => {
2228-
// TODO: Future follow-up must resolve virtio activation handling while migrating.
22292234
let count = self.activate_evt.read().map_err(Error::EventFdRead)?;
2230-
if let VmOwnership::Owned(ref vm) = self.vm {
2231-
info!("Trying to activate pending virtio devices: count = {count}");
2232-
vm.activate_virtio_devices()
2233-
.map_err(Error::ActivateVirtioDevices)?;
2235+
info!("Trying to activate pending virtio devices: count = {count}");
2236+
match &self.vm {
2237+
VmOwnership::Owned(vm) => {
2238+
vm.activate_virtio_devices()
2239+
.map_err(Error::ActivateVirtioDevices)?;
2240+
}
2241+
VmOwnership::Migration { device_manager, .. } => {
2242+
// If the VM (and thus the device manager) were
2243+
// dropped at this point, we'd have a serious
2244+
// programming bug.
2245+
let device_manager = device_manager
2246+
.upgrade()
2247+
.expect("DeviceManager should remain alive during a migration");
2248+
let device_manager = device_manager.lock().unwrap();
2249+
device_manager
2250+
.activate_virtio_devices()
2251+
.map_err(VmError::ActivateVirtioDevices)
2252+
.map_err(Error::ActivateVirtioDevices)?;
2253+
}
2254+
VmOwnership::None => {}
22342255
}
22352256
}
22362257
EpollDispatch::Api => {
@@ -3323,6 +3344,8 @@ impl RequestHandler for Vmm {
33233344
.take_owned_or(VmError::VmNotRunning)
33243345
.expect("should have VM ownership as we just checked it");
33253346

3347+
let device_manager = Arc::downgrade(vm.device_manager());
3348+
33263349
match MigrationWorker::spawn(
33273350
vm,
33283351
check_migration_evt,
@@ -3336,6 +3359,7 @@ impl RequestHandler for Vmm {
33363359
self.vm = VmOwnership::Migration {
33373360
migration_worker_handle: handle,
33383361
vm_info_response: vm_info_snapshot,
3362+
device_manager,
33393363
};
33403364
Ok(())
33413365
}

‎vmm/src/vm.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3485,6 +3485,10 @@ impl Vm {
34853485
.unwrap()
34863486
.post_migration_announce();
34873487
}
3488+
3489+
pub fn device_manager(&self) -> &Arc<Mutex<DeviceManager>> {
3490+
&self.device_manager
3491+
}
34883492
}
34893493

34903494
impl Pausable for Vm {

0 commit comments

Comments
 (0)