diff --git a/src/vmm/src/device_manager/persist.rs b/src/vmm/src/device_manager/persist.rs index d951af089eb..bf2938189b4 100644 --- a/src/vmm/src/device_manager/persist.rs +++ b/src/vmm/src/device_manager/persist.rs @@ -37,7 +37,6 @@ use crate::devices::virtio::vsock::persist::{ VsockConstructorArgs, VsockState, VsockUdsConstructorArgs, }; use crate::devices::virtio::vsock::{Vsock, VsockUnixBackend}; -use crate::logger::warn; use crate::mmds::data_store::MmdsVersion; use crate::resources::VmResources; use crate::snapshot::Persist; @@ -256,20 +255,17 @@ impl<'a> Persist<'a> for MMIODeviceManager { // Both virtio-block and vhost-user-block share same device type. VirtioDeviceType::Block => { let block = locked_device.as_mut_any().downcast_mut::().unwrap(); - if block.is_vhost_user() { - warn!( - "Skipping vhost-user-block device. VhostUserBlock does not support \ - snapshotting yet" - ); - } else { - let device_state = block.save(); - states.block_devices.push(VirtioDeviceState { - device_id, - device_state, - transport_state, - device_info, - }); - } + assert!( + !block.is_vhost_user(), + "vhost-user-block does not support snapshotting yet" + ); + let device_state = block.save(); + states.block_devices.push(VirtioDeviceState { + device_id, + device_state, + transport_state, + device_info, + }); } VirtioDeviceType::Net => { let net = locked_device.as_mut_any().downcast_mut::().unwrap(); diff --git a/src/vmm/src/lib.rs b/src/vmm/src/lib.rs index 020d941012e..a2a50010ae0 100644 --- a/src/vmm/src/lib.rs +++ b/src/vmm/src/lib.rs @@ -141,7 +141,7 @@ use crate::devices::virtio::balloon::{ }; use crate::devices::virtio::block::BlockError; use crate::devices::virtio::block::device::Block; -use crate::devices::virtio::device::VirtioDeviceType; +use crate::devices::virtio::device::{VirtioDevice, VirtioDeviceType}; use crate::devices::virtio::mem::device::VirtioMem; use crate::devices::virtio::mem::{VIRTIO_MEM_DEV_ID, VirtioMemError, VirtioMemStatus}; use crate::devices::virtio::net::Net; @@ -449,6 +449,32 @@ impl Vmm { } } + /// Check if the VM has any devices without snapshot support + pub fn check_unsnapshottable_devices(&self) -> Result<(), MicrovmStateError> { + let mut tuples = Vec::new(); + self.device_manager + .for_each_virtio_device(|device_type, device| { + if let VirtioDeviceType::Block = device_type + && let Some(b) = device.as_any().downcast_ref::() + && b.is_vhost_user() + { + tuples.push(("vhost-user-block", b.id().to_owned())); + } + }); + if tuples.is_empty() { + Ok(()) + } else { + let msg = tuples + .iter() + .map(|(t, id)| format!("{t}(id: {id})")) + .collect::>() + .join(","); + Err(MicrovmStateError::NotAllowed(format!( + "Devices without snapshot support are present: {msg}" + ))) + } + } + /// Starts the microVM vcpus. /// /// # Errors @@ -555,7 +581,8 @@ impl Vmm { /// Saves the state of a paused Microvm. pub fn save_state(&mut self, vm_info: &VmInfo) -> Result { - use self::MicrovmStateError::SaveVmState; + self.check_unsnapshottable_devices()?; + // We need to save device state before saving KVM state. // Some devices, (at the time of writing this comment block device with async engine) // might modify the VirtIO transport and send an interrupt to the guest. If we save KVM @@ -567,13 +594,17 @@ impl Vmm { let vm_state = { #[cfg(target_arch = "x86_64")] { - self.vm.save_state().map_err(SaveVmState)? + self.vm + .save_state() + .map_err(MicrovmStateError::SaveVmState)? } #[cfg(target_arch = "aarch64")] { let mpidrs = construct_kvm_mpidrs(&vcpu_states); - self.vm.save_state(&mpidrs).map_err(SaveVmState)? + self.vm + .save_state(&mpidrs) + .map_err(MicrovmStateError::SaveVmState)? } }; diff --git a/tests/integration_tests/functional/test_drive_vhost_user.py b/tests/integration_tests/functional/test_drive_vhost_user.py index 07fcafb715e..ad1acdec72e 100644 --- a/tests/integration_tests/functional/test_drive_vhost_user.py +++ b/tests/integration_tests/functional/test_drive_vhost_user.py @@ -88,7 +88,8 @@ def _check_drives(test_microvm, assert_dict, keys_array): def test_vhost_user_block(uvm_vhost_user_booted_ro): """ This test simply tries to boot a VM with - vhost-user-block as a root device. + vhost-user-block as a root device and then + tries to snapshot it. """ vm = uvm_vhost_user_booted_ro @@ -105,6 +106,14 @@ def test_vhost_user_block(uvm_vhost_user_booted_ro): _check_drives(vm, assert_dict, assert_dict.keys()) vhost_user_block_metrics.validate(vm) + with pytest.raises( + RuntimeError, + match=r"Devices without snapshot support are present: vhost-user-block\(id: rootfs\)", + ): + vm.api.snapshot_create.put( + mem_file_path="memfile", snapshot_path="statefile", snapshot_type="Full" + ) + def test_vhost_user_block_read_write(uvm_vhost_user_booted_rw): """