Skip to content

refactor: Simplify VMM code to perform API action on device #5387

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Aug 19, 2025
Merged

refactor: Simplify VMM code to perform API action on device #5387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
09f8045
refactor: generate virtio device ids with bindgen
Manciukic Aug 8, 2025
eecbdb9
refactor: reduce boilerplate to execute fn on virtio device
Manciukic Aug 12, 2025
f798957
refactor(balloon): remove unused error variants
Manciukic Aug 12, 2025
755ba29
fix(balloon): improve error message in case of too much requested memory
Manciukic Aug 14, 2025
b1db933
refactor(balloon): derive Copy for BalloonStats
Manciukic Aug 18, 2025
f272086
refactor: add trait function const_device_type to use in Sized contexts
Manciukic Aug 18, 2025
3e99b29
Merge branch 'main' into refactor/with-virtio-device-fn
Manciukic Aug 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions src/vmm/src/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -656,9 +656,9 @@ pub(crate) mod tests {
use super::*;
use crate::device_manager::tests::default_device_manager;
use crate::devices::virtio::block::CacheType;
use crate::devices::virtio::generated::virtio_ids;
use crate::devices::virtio::rng::device::ENTROPY_DEV_ID;
use crate::devices::virtio::vsock::{TYPE_VSOCK, VSOCK_DEV_ID};
use crate::devices::virtio::{TYPE_BALLOON, TYPE_BLOCK, TYPE_RNG};
use crate::devices::virtio::vsock::VSOCK_DEV_ID;
use crate::mmds::data_store::{Mmds, MmdsVersion};
use crate::mmds::ns::MmdsNetworkStack;
use crate::utils::mib_to_bytes;
Expand Down Expand Up @@ -855,7 +855,7 @@ pub(crate) mod tests {

assert!(
vmm.device_manager
.get_virtio_device(TYPE_VSOCK, &vsock_dev_id)
.get_virtio_device(virtio_ids::VIRTIO_ID_VSOCK, &vsock_dev_id)
.is_some()
);
}
Expand All @@ -880,7 +880,7 @@ pub(crate) mod tests {

assert!(
vmm.device_manager
.get_virtio_device(TYPE_RNG, ENTROPY_DEV_ID)
.get_virtio_device(virtio_ids::VIRTIO_ID_RNG, ENTROPY_DEV_ID)
.is_some()
);
}
Expand Down Expand Up @@ -914,7 +914,7 @@ pub(crate) mod tests {

assert!(
vmm.device_manager
.get_virtio_device(TYPE_BALLOON, BALLOON_DEV_ID)
.get_virtio_device(virtio_ids::VIRTIO_ID_BALLOON, BALLOON_DEV_ID)
.is_some()
);
}
Expand Down Expand Up @@ -965,7 +965,7 @@ pub(crate) mod tests {
assert!(cmdline_contains(&cmdline, "root=/dev/vda ro"));
assert!(
vmm.device_manager
.get_virtio_device(TYPE_BLOCK, drive_id.as_str())
.get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, drive_id.as_str())
.is_some()
);
}
Expand All @@ -986,7 +986,7 @@ pub(crate) mod tests {
assert!(cmdline_contains(&cmdline, "root=PARTUUID=0eaa91a0-01 rw"));
assert!(
vmm.device_manager
.get_virtio_device(TYPE_BLOCK, drive_id.as_str())
.get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, drive_id.as_str())
.is_some()
);
}
Expand All @@ -1008,7 +1008,7 @@ pub(crate) mod tests {
assert!(!cmdline_contains(&cmdline, "root=/dev/vda"));
assert!(
vmm.device_manager
.get_virtio_device(TYPE_BLOCK, drive_id.as_str())
.get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, drive_id.as_str())
.is_some()
);
}
Expand Down Expand Up @@ -1045,17 +1045,17 @@ pub(crate) mod tests {
assert!(cmdline_contains(&cmdline, "root=PARTUUID=0eaa91a0-01 rw"));
assert!(
vmm.device_manager
.get_virtio_device(TYPE_BLOCK, "root")
.get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, "root")
.is_some()
);
assert!(
vmm.device_manager
.get_virtio_device(TYPE_BLOCK, "secondary")
.get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, "secondary")
.is_some()
);
assert!(
vmm.device_manager
.get_virtio_device(TYPE_BLOCK, "third")
.get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, "third")
.is_some()
);

Expand Down Expand Up @@ -1084,7 +1084,7 @@ pub(crate) mod tests {
assert!(cmdline_contains(&cmdline, "root=/dev/vda rw"));
assert!(
vmm.device_manager
.get_virtio_device(TYPE_BLOCK, drive_id.as_str())
.get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, drive_id.as_str())
.is_some()
);
}
Expand All @@ -1105,7 +1105,7 @@ pub(crate) mod tests {
assert!(cmdline_contains(&cmdline, "root=PARTUUID=0eaa91a0-01 ro"));
assert!(
vmm.device_manager
.get_virtio_device(TYPE_BLOCK, drive_id.as_str())
.get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, drive_id.as_str())
.is_some()
);
}
Expand All @@ -1126,7 +1126,7 @@ pub(crate) mod tests {
assert!(cmdline_contains(&cmdline, "root=/dev/vda rw"));
assert!(
vmm.device_manager
.get_virtio_device(TYPE_BLOCK, drive_id.as_str())
.get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, drive_id.as_str())
.is_some()
);
}
Expand Down
8 changes: 3 additions & 5 deletions src/vmm/src/device_manager/mmio.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ pub(crate) mod tests {
use crate::test_utils::multi_region_mem_raw;
use crate::vstate::kvm::Kvm;
use crate::vstate::memory::{GuestAddress, GuestMemoryMmap};
use crate::{Vm, arch};
use crate::{Vm, arch, impl_device_type};

const QUEUE_SIZES: &[u16] = &[64];

Expand Down Expand Up @@ -522,6 +522,8 @@ pub(crate) mod tests {
}

impl VirtioDevice for DummyDevice {
impl_device_type!(0);

fn avail_features(&self) -> u64 {
0
}
Expand All @@ -532,10 +534,6 @@ pub(crate) mod tests {

fn set_acked_features(&mut self, _: u64) {}

fn device_type(&self) -> u32 {
0
}

fn queues(&self) -> &[Queue] {
&self.queues
}
Expand Down
26 changes: 17 additions & 9 deletions src/vmm/src/device_manager/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ pub enum FindDeviceError {
/// Device not found
DeviceNotFound,
/// Internal Device error: {0}
InternalDeviceError(String),
InternalDeviceError(anyhow::Error),
}

#[derive(Debug)]
Expand Down Expand Up @@ -371,27 +371,35 @@ impl DeviceManager {
}

/// Run fn `f()` for the virtio device matching `virtio_type` and `id`.
pub fn with_virtio_device_with_id<T, F>(
pub fn try_with_virtio_device_with_id<T, F, R, E>(
&self,
virtio_type: u32,
id: &str,
f: F,
) -> Result<(), FindDeviceError>
) -> Result<R, FindDeviceError>
where
T: VirtioDevice + 'static + Debug,
F: FnOnce(&mut T) -> Result<(), String>,
E: std::error::Error + 'static + Send + Sync,
F: FnOnce(&mut T) -> Result<R, E>,
{
if let Some(device) = self.get_virtio_device(virtio_type, id) {
if let Some(device) = self.get_virtio_device(T::const_device_type(), id) {
let mut dev = device.lock().expect("Poisoned lock");
f(dev
.as_mut_any()
.downcast_mut::<T>()
.ok_or(FindDeviceError::InvalidDeviceType)?)
.map_err(FindDeviceError::InternalDeviceError)?;
.map_err(|e| FindDeviceError::InternalDeviceError(e.into()))
} else {
return Err(FindDeviceError::DeviceNotFound);
Err(FindDeviceError::DeviceNotFound)
}
Ok(())
}

/// Run fn `f()` for the virtio device matching `virtio_type` and `id`.
pub fn with_virtio_device_with_id<T, F, R>(&self, id: &str, f: F) -> Result<R, FindDeviceError>
where
T: VirtioDevice + 'static + Debug,
F: FnOnce(&mut T) -> R,
{
self.try_with_virtio_device_with_id(id, |dev: &mut T| Ok::<R, FindDeviceError>(f(dev)))
}
}

Expand Down
14 changes: 7 additions & 7 deletions src/vmm/src/device_manager/pci_mngr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use crate::devices::virtio::balloon::persist::{BalloonConstructorArgs, BalloonSt
use crate::devices::virtio::block::device::Block;
use crate::devices::virtio::block::persist::{BlockConstructorArgs, BlockState};
use crate::devices::virtio::device::VirtioDevice;
use crate::devices::virtio::generated::virtio_ids;
use crate::devices::virtio::net::Net;
use crate::devices::virtio::net::persist::{NetConstructorArgs, NetState};
use crate::devices::virtio::rng::Entropy;
Expand All @@ -29,8 +30,7 @@ use crate::devices::virtio::transport::pci::device::{
use crate::devices::virtio::vsock::persist::{
VsockConstructorArgs, VsockState, VsockUdsConstructorArgs,
};
use crate::devices::virtio::vsock::{TYPE_VSOCK, Vsock, VsockUnixBackend};
use crate::devices::virtio::{TYPE_BALLOON, TYPE_BLOCK, TYPE_NET, TYPE_RNG};
use crate::devices::virtio::vsock::{Vsock, VsockUnixBackend};
use crate::resources::VmResources;
use crate::snapshot::Persist;
use crate::vmm_config::mmds::MmdsConfigError;
Expand Down Expand Up @@ -285,7 +285,7 @@ impl<'a> Persist<'a> for PciDevices {
let pci_device_bdf = transport_state.pci_device_bdf.into();

match locked_virtio_dev.device_type() {
TYPE_BALLOON => {
virtio_ids::VIRTIO_ID_BALLOON => {
let balloon_device = locked_virtio_dev
.as_any()
.downcast_ref::<Balloon>()
Expand All @@ -300,7 +300,7 @@ impl<'a> Persist<'a> for PciDevices {
transport_state,
});
}
TYPE_BLOCK => {
virtio_ids::VIRTIO_ID_BLOCK => {
let block_dev = locked_virtio_dev
.as_mut_any()
.downcast_mut::<Block>()
Expand All @@ -321,7 +321,7 @@ impl<'a> Persist<'a> for PciDevices {
});
}
}
TYPE_NET => {
virtio_ids::VIRTIO_ID_NET => {
let net_dev = locked_virtio_dev
.as_mut_any()
.downcast_mut::<Net>()
Expand All @@ -343,7 +343,7 @@ impl<'a> Persist<'a> for PciDevices {
transport_state,
})
}
TYPE_VSOCK => {
virtio_ids::VIRTIO_ID_VSOCK => {
let vsock_dev = locked_virtio_dev
.as_mut_any()
// Currently, VsockUnixBackend is the only implementation of VsockBackend.
Expand Down Expand Up @@ -374,7 +374,7 @@ impl<'a> Persist<'a> for PciDevices {
transport_state,
});
}
TYPE_RNG => {
virtio_ids::VIRTIO_ID_RNG => {
let rng_dev = locked_virtio_dev
.as_mut_any()
.downcast_mut::<Entropy>()
Expand Down
17 changes: 8 additions & 9 deletions src/vmm/src/device_manager/persist.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ use crate::arch::DeviceType;
use crate::devices::acpi::vmgenid::{VMGenIDState, VMGenIdConstructorArgs, VmGenId, VmGenIdError};
#[cfg(target_arch = "aarch64")]
use crate::devices::legacy::RTCDevice;
use crate::devices::virtio::ActivateError;
use crate::devices::virtio::balloon::persist::{BalloonConstructorArgs, BalloonState};
use crate::devices::virtio::balloon::{Balloon, BalloonError};
use crate::devices::virtio::block::BlockError;
use crate::devices::virtio::block::device::Block;
use crate::devices::virtio::block::persist::{BlockConstructorArgs, BlockState};
use crate::devices::virtio::device::VirtioDevice;
use crate::devices::virtio::generated::virtio_ids;
use crate::devices::virtio::net::Net;
use crate::devices::virtio::net::persist::{
NetConstructorArgs, NetPersistError as NetError, NetState,
Expand All @@ -36,10 +38,7 @@ use crate::devices::virtio::transport::mmio::{IrqTrigger, MmioTransport};
use crate::devices::virtio::vsock::persist::{
VsockConstructorArgs, VsockState, VsockUdsConstructorArgs,
};
use crate::devices::virtio::vsock::{
TYPE_VSOCK, Vsock, VsockError, VsockUnixBackend, VsockUnixBackendError,
};
use crate::devices::virtio::{ActivateError, TYPE_BALLOON, TYPE_BLOCK, TYPE_NET, TYPE_RNG};
use crate::devices::virtio::vsock::{Vsock, VsockError, VsockUnixBackend, VsockUnixBackendError};
use crate::mmds::data_store::MmdsVersion;
use crate::resources::{ResourcesError, VmResources};
use crate::snapshot::Persist;
Expand Down Expand Up @@ -242,7 +241,7 @@ impl<'a> Persist<'a> for MMIODeviceManager {

let mut locked_device = mmio_transport_locked.locked_device();
match locked_device.device_type() {
TYPE_BALLOON => {
virtio_ids::VIRTIO_ID_BALLOON => {
let device_state = locked_device
.as_any()
.downcast_ref::<Balloon>()
Expand All @@ -256,7 +255,7 @@ impl<'a> Persist<'a> for MMIODeviceManager {
});
}
// Both virtio-block and vhost-user-block share same device type.
TYPE_BLOCK => {
virtio_ids::VIRTIO_ID_BLOCK => {
let block = locked_device.as_mut_any().downcast_mut::<Block>().unwrap();
if block.is_vhost_user() {
warn!(
Expand All @@ -274,7 +273,7 @@ impl<'a> Persist<'a> for MMIODeviceManager {
});
}
}
TYPE_NET => {
virtio_ids::VIRTIO_ID_NET => {
let net = locked_device.as_mut_any().downcast_mut::<Net>().unwrap();
if let (Some(mmds_ns), None) = (net.mmds_ns.as_ref(), states.mmds.as_ref()) {
let mmds_guard = mmds_ns.mmds.lock().expect("Poisoned lock");
Expand All @@ -293,7 +292,7 @@ impl<'a> Persist<'a> for MMIODeviceManager {
device_info,
});
}
TYPE_VSOCK => {
virtio_ids::VIRTIO_ID_VSOCK => {
let vsock = locked_device
.as_mut_any()
// Currently, VsockUnixBackend is the only implementation of VsockBackend.
Expand Down Expand Up @@ -322,7 +321,7 @@ impl<'a> Persist<'a> for MMIODeviceManager {
device_info,
});
}
TYPE_RNG => {
virtio_ids::VIRTIO_ID_RNG => {
let entropy = locked_device
.as_mut_any()
.downcast_mut::<Entropy>()
Expand Down
Loading