From c53781bf5f17726c94a1d2cfd7e1b799b4ad92bb Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Wed, 26 Nov 2025 14:00:44 +0100 Subject: [PATCH] misc: clippy: add needless_pass_by_value This is a follow-up of [0]. # Advantages - This saves dozens of unneeded clone()s across the whole code base - Makes it much easier to reason about how parameters are used (often we passed owned Arc/Rc versions without actually needing ownership) # Exceptions For certain code paths, the alternatives would require awkward or overly complex code, and in some cases the functions are the logical owners of the values they take. In those cases, I've added #[allow(clippy::needless_pass_by_value)]. This does not mean that one should not improve this in the future. [0] 6a86c157afde79e734bc35e1065403d532c51f34 Signed-off-by: Philipp Schuster On-behalf-of: SAP philipp.schuster@sap.com --- Cargo.toml | 3 +- arch/src/aarch64/fdt.rs | 9 +- arch/src/aarch64/mod.rs | 4 +- arch/src/riscv64/fdt.rs | 4 +- arch/src/riscv64/mod.rs | 2 +- devices/src/aia.rs | 6 +- devices/src/gic.rs | 6 +- devices/src/ioapic.rs | 2 +- devices/src/tpm.rs | 5 + hypervisor/src/kvm/aarch64/gic/mod.rs | 15 +-- hypervisor/src/kvm/mod.rs | 4 +- hypervisor/src/kvm/riscv64/aia.rs | 5 +- hypervisor/src/mshv/aarch64/gic/mod.rs | 2 +- hypervisor/src/mshv/mod.rs | 2 +- hypervisor/src/vm.rs | 4 +- pci/src/bus.rs | 1 + src/bin/ch-remote.rs | 1 + src/main.rs | 1 + test_infra/src/lib.rs | 9 +- tests/integration.rs | 127 ++++++++++-------- virtio-devices/src/balloon.rs | 6 +- virtio-devices/src/block.rs | 6 +- virtio-devices/src/console.rs | 6 +- virtio-devices/src/epoll_helper.rs | 14 +- virtio-devices/src/iommu.rs | 6 +- virtio-devices/src/mem.rs | 10 +- virtio-devices/src/net.rs | 12 +- virtio-devices/src/pmem.rs | 6 +- virtio-devices/src/rng.rs | 6 +- virtio-devices/src/seccomp_filters.rs | 1 + .../src/transport/pci_common_config.rs | 5 + virtio-devices/src/vhost_user/blk.rs | 4 +- virtio-devices/src/vhost_user/fs.rs | 4 +- virtio-devices/src/vhost_user/mod.rs | 25 ++-- virtio-devices/src/vhost_user/net.rs | 6 +- .../src/vhost_user/vu_common_ctrl.rs | 4 +- virtio-devices/src/vsock/device.rs | 6 +- virtio-devices/src/watchdog.rs | 6 +- vm-device/src/bus.rs | 1 + vmm/src/acpi.rs | 1 + vmm/src/api/http/mod.rs | 1 + vmm/src/api/mod.rs | 1 + vmm/src/config.rs | 1 + vmm/src/console_devices.rs | 16 +-- vmm/src/cpu.rs | 5 +- vmm/src/device_manager.rs | 49 +++---- vmm/src/igvm/igvm_loader.rs | 2 +- vmm/src/igvm/mod.rs | 2 +- vmm/src/lib.rs | 30 ++--- vmm/src/memory_manager.rs | 8 +- vmm/src/seccomp_filters.rs | 1 + vmm/src/sigwinch_listener.rs | 10 +- vmm/src/vm.rs | 31 +++-- 53 files changed, 273 insertions(+), 231 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 155e2fc17..54fc85545 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -174,8 +174,7 @@ assertions_on_result_states = "deny" if_not_else = "deny" manual_string_new = "deny" map_unwrap_or = "deny" -# Very helpful to uncover costly clones, but unfortunately also a rabbit hole. -#needless_pass_by_value = "deny" +needless_pass_by_value = "deny" redundant_else = "deny" semicolon_if_nothing_returned = "deny" undocumented_unsafe_blocks = "deny" diff --git a/arch/src/aarch64/fdt.rs b/arch/src/aarch64/fdt.rs index f466109e8..5ce1bea20 100644 --- a/arch/src/aarch64/fdt.rs +++ b/arch/src/aarch64/fdt.rs @@ -88,6 +88,7 @@ pub enum Error { } type Result = result::Result; +#[derive(Copy, Clone)] pub enum CacheLevel { /// L1 data cache L1D = 0, @@ -207,7 +208,7 @@ pub fn get_cache_shared(cache_level: CacheLevel) -> bool { pub fn create_fdt( guest_mem: &GuestMemoryMmap, cmdline: &str, - vcpu_mpidr: Vec, + vcpu_mpidr: &[u64], vcpu_topology: Option<(u16, u16, u16, u16)>, device_info: &HashMap<(DeviceType, String), T, S>, gic_device: &Arc>, @@ -234,7 +235,7 @@ pub fn create_fdt, guest_mem: &GuestMemoryMmap) -> Result<()> { +pub fn write_fdt_to_memory(fdt_final: &[u8], guest_mem: &GuestMemoryMmap) -> Result<()> { // Write FDT to memory. guest_mem - .write_slice(fdt_final.as_slice(), super::layout::FDT_START) + .write_slice(fdt_final, super::layout::FDT_START) .map_err(Error::WriteFdtToMemory)?; Ok(()) } diff --git a/arch/src/aarch64/mod.rs b/arch/src/aarch64/mod.rs index 910c14998..79c267092 100644 --- a/arch/src/aarch64/mod.rs +++ b/arch/src/aarch64/mod.rs @@ -125,7 +125,7 @@ pub fn arch_memory_regions() -> Vec<(GuestAddress, usize, RegionType)> { pub fn configure_system( guest_mem: &GuestMemoryMmap, cmdline: &str, - vcpu_mpidr: Vec, + vcpu_mpidr: &[u64], vcpu_topology: Option<(u16, u16, u16, u16)>, device_info: &HashMap<(DeviceType, String), T, S>, initrd: &Option, @@ -154,7 +154,7 @@ pub fn configure_system, guest_mem: &GuestMemoryMmap) -> Result<()> { +pub fn write_fdt_to_memory(fdt_final: &[u8], guest_mem: &GuestMemoryMmap) -> Result<()> { // Write FDT to memory. guest_mem - .write_slice(fdt_final.as_slice(), super::layout::FDT_START) + .write_slice(fdt_final, super::layout::FDT_START) .map_err(Error::WriteFdtToMemory)?; Ok(()) } diff --git a/arch/src/riscv64/mod.rs b/arch/src/riscv64/mod.rs index 0ee66db32..8b89b94a5 100644 --- a/arch/src/riscv64/mod.rs +++ b/arch/src/riscv64/mod.rs @@ -187,7 +187,7 @@ pub fn configure_system>, @@ -51,9 +52,8 @@ impl Aia { }) .map_err(Error::CreateInterruptSourceGroup)?; - let vaia = vm - .create_vaia(Aia::create_default_config(vcpu_count as u64)) - .map_err(Error::CreateAia)?; + let config = Aia::create_default_config(vcpu_count as u64); + let vaia = vm.create_vaia(&config).map_err(Error::CreateAia)?; let aia = Aia { interrupt_source_group, diff --git a/devices/src/gic.rs b/devices/src/gic.rs index fd7199d00..65e838402 100644 --- a/devices/src/gic.rs +++ b/devices/src/gic.rs @@ -38,6 +38,7 @@ pub struct Gic { } impl Gic { + #[allow(clippy::needless_pass_by_value)] pub fn new( vcpu_count: u32, interrupt_manager: Arc>, @@ -50,9 +51,8 @@ impl Gic { }) .map_err(Error::CreateInterruptSourceGroup)?; - let vgic = vm - .create_vgic(Gic::create_default_config(vcpu_count as u64)) - .map_err(Error::CreateGic)?; + let config = Gic::create_default_config(vcpu_count as u64); + let vgic = vm.create_vgic(&config).map_err(Error::CreateGic)?; let gic = Gic { interrupt_source_group, diff --git a/devices/src/ioapic.rs b/devices/src/ioapic.rs index 660531eb6..ba05c1ed5 100644 --- a/devices/src/ioapic.rs +++ b/devices/src/ioapic.rs @@ -192,7 +192,7 @@ impl Ioapic { id: String, apic_address: GuestAddress, interrupt_manager: &dyn InterruptManager, - state: Option, + state: Option<&IoapicState>, ) -> Result { let interrupt_source_group = interrupt_manager .create_group(MsiIrqGroupConfig { diff --git a/devices/src/tpm.rs b/devices/src/tpm.rs index 15494e323..86866b2e7 100644 --- a/devices/src/tpm.rs +++ b/devices/src/tpm.rs @@ -36,12 +36,14 @@ enum LocStateFields { TpmRegValidSts, } +#[derive(Copy, Clone)] enum LocStsFields { Granted, BeenSeized, } #[allow(dead_code)] +#[derive(Copy, Clone)] enum IntfIdFields { InterfaceType, InterfaceVersion, @@ -59,6 +61,7 @@ enum IntfIdFields { } #[allow(dead_code)] +#[derive(Copy, Clone)] enum IntfId2Fields { Vid, Did, @@ -70,6 +73,7 @@ enum CtrlStsFields { TpmIdle, } +#[derive(Copy, Clone)] enum CrbRegister { LocState(LocStateFields), LocSts(LocStsFields), @@ -102,6 +106,7 @@ const CRB_LOC_CTRL_REQUEST_ACCESS: u32 = 1 << 0; const CRB_LOC_CTRL_RELINQUISH: u32 = 1 << 1; const CRB_LOC_CTRL_RESET_ESTABLISHMENT_BIT: u32 = 1 << 3; const CRB_LOC_STS: u32 = 0x0C; + const fn get_crb_loc_sts_field(f: LocStsFields) -> (u32, u32, u32) { let (offset, len) = match f { LocStsFields::Granted => (0, 1), diff --git a/hypervisor/src/kvm/aarch64/gic/mod.rs b/hypervisor/src/kvm/aarch64/gic/mod.rs index 2e5523e97..8dcf06078 100644 --- a/hypervisor/src/kvm/aarch64/gic/mod.rs +++ b/hypervisor/src/kvm/aarch64/gic/mod.rs @@ -262,7 +262,7 @@ impl KvmGicV3Its { } /// Method to initialize the GIC device - pub fn new(vm: &dyn Vm, config: VgicConfig) -> Result { + pub fn new(vm: &dyn Vm, config: &VgicConfig) -> Result { // This is inside KVM module let vm = vm.as_any().downcast_ref::().expect("Wrong VM type?"); @@ -509,7 +509,7 @@ mod unit_tests { let hv = crate::new().unwrap(); let vm = hv.create_vm(HypervisorVmConfig::default()).unwrap(); - KvmGicV3Its::new(&*vm, create_test_vgic_config()).unwrap(); + KvmGicV3Its::new(&*vm, &create_test_vgic_config()).unwrap(); } #[test] @@ -517,7 +517,7 @@ mod unit_tests { let hv = crate::new().unwrap(); let vm = hv.create_vm(HypervisorVmConfig::default()).unwrap(); let _ = vm.create_vcpu(0, None).unwrap(); - let gic = KvmGicV3Its::new(&*vm, create_test_vgic_config()).expect("Cannot create gic"); + let gic = KvmGicV3Its::new(&*vm, &create_test_vgic_config()).expect("Cannot create gic"); let state = get_dist_regs(&gic.device).unwrap(); assert_eq!(state.len(), 568); @@ -530,7 +530,7 @@ mod unit_tests { let hv = crate::new().unwrap(); let vm = hv.create_vm(HypervisorVmConfig::default()).unwrap(); let _ = vm.create_vcpu(0, None).unwrap(); - let gic = KvmGicV3Its::new(&*vm, create_test_vgic_config()).expect("Cannot create gic"); + let gic = KvmGicV3Its::new(&*vm, &create_test_vgic_config()).expect("Cannot create gic"); let gicr_typer = vec![123]; let state = get_redist_regs(&gic.device, &gicr_typer).unwrap(); @@ -545,7 +545,7 @@ mod unit_tests { let hv = crate::new().unwrap(); let vm = hv.create_vm(HypervisorVmConfig::default()).unwrap(); let _ = vm.create_vcpu(0, None).unwrap(); - let gic = KvmGicV3Its::new(&*vm, create_test_vgic_config()).expect("Cannot create gic"); + let gic = KvmGicV3Its::new(&*vm, &create_test_vgic_config()).expect("Cannot create gic"); let gicr_typer = vec![123]; let state = get_icc_regs(&gic.device, &gicr_typer).unwrap(); @@ -560,9 +560,8 @@ mod unit_tests { let hv = crate::new().unwrap(); let vm = hv.create_vm(HypervisorVmConfig::default()).unwrap(); let _ = vm.create_vcpu(0, None).unwrap(); - let gic = vm - .create_vgic(create_test_vgic_config()) - .expect("Cannot create gic"); + let vgic_config = create_test_vgic_config(); + let gic = vm.create_vgic(&vgic_config).expect("Cannot create gic"); gic.lock().unwrap().save_data_tables().unwrap(); } diff --git a/hypervisor/src/kvm/mod.rs b/hypervisor/src/kvm/mod.rs index ce6e13829..74ee19a30 100644 --- a/hypervisor/src/kvm/mod.rs +++ b/hypervisor/src/kvm/mod.rs @@ -579,7 +579,7 @@ impl vm::Vm for KvmVm { /// /// Creates a virtual GIC device. /// - fn create_vgic(&self, config: VgicConfig) -> vm::Result>> { + fn create_vgic(&self, config: &VgicConfig) -> vm::Result>> { let gic_device = KvmGicV3Its::new(self, config) .map_err(|e| vm::HypervisorVmError::CreateVgic(anyhow!("Vgic error {e:?}")))?; Ok(Arc::new(Mutex::new(gic_device))) @@ -589,7 +589,7 @@ impl vm::Vm for KvmVm { /// /// Creates a virtual AIA device. /// - fn create_vaia(&self, config: VaiaConfig) -> vm::Result>> { + fn create_vaia(&self, config: &VaiaConfig) -> vm::Result>> { let aia_device = KvmAiaImsics::new(self, config) .map_err(|e| vm::HypervisorVmError::CreateVaia(anyhow!("Vaia error {e:?}")))?; Ok(Arc::new(Mutex::new(aia_device))) diff --git a/hypervisor/src/kvm/riscv64/aia.rs b/hypervisor/src/kvm/riscv64/aia.rs index 9eb6e5f39..67857c6ae 100644 --- a/hypervisor/src/kvm/riscv64/aia.rs +++ b/hypervisor/src/kvm/riscv64/aia.rs @@ -180,7 +180,7 @@ impl KvmAiaImsics { } /// Method to initialize the AIA device - pub fn new(vm: &dyn Vm, config: VaiaConfig) -> Result { + pub fn new(vm: &dyn Vm, config: &VaiaConfig) -> Result { // This is inside KVM module let vm = vm.as_any().downcast_ref::().expect("Wrong VM type?"); @@ -270,6 +270,7 @@ mod unit_tests { let vm = hv.create_vm(HypervisorVmConfig::default()).unwrap(); let _vcpu = vm.create_vcpu(0, None).unwrap(); - assert!(KvmAiaImsics::new(&*vm, create_test_vaia_config()).is_ok()); + let vaia_config = create_test_vaia_config(); + assert!(KvmAiaImsics::new(&*vm, &vaia_config).is_ok()); } } diff --git a/hypervisor/src/mshv/aarch64/gic/mod.rs b/hypervisor/src/mshv/aarch64/gic/mod.rs index 631c5d4f0..47dab6ff3 100644 --- a/hypervisor/src/mshv/aarch64/gic/mod.rs +++ b/hypervisor/src/mshv/aarch64/gic/mod.rs @@ -56,7 +56,7 @@ impl From for GicState { impl MshvGicV2M { /// Create a new GICv2m device - pub fn new(_vm: &dyn Vm, config: VgicConfig) -> Result { + pub fn new(_vm: &dyn Vm, config: &VgicConfig) -> Result { let gic_device = MshvGicV2M { dist_addr: config.dist_addr, dist_size: config.dist_size, diff --git a/hypervisor/src/mshv/mod.rs b/hypervisor/src/mshv/mod.rs index a7de46080..d4ececd8e 100644 --- a/hypervisor/src/mshv/mod.rs +++ b/hypervisor/src/mshv/mod.rs @@ -2193,7 +2193,7 @@ impl vm::Vm for MshvVm { } #[cfg(target_arch = "aarch64")] - fn create_vgic(&self, config: VgicConfig) -> vm::Result>> { + fn create_vgic(&self, config: &VgicConfig) -> vm::Result>> { let gic_device = MshvGicV2M::new(self, config) .map_err(|e| vm::HypervisorVmError::CreateVgic(anyhow!("Vgic error {e:?}")))?; diff --git a/hypervisor/src/vm.rs b/hypervisor/src/vm.rs index 3b43c453a..5b56923b1 100644 --- a/hypervisor/src/vm.rs +++ b/hypervisor/src/vm.rs @@ -318,9 +318,9 @@ pub trait Vm: Send + Sync + Any { /// Creates a new KVM vCPU file descriptor and maps the memory corresponding fn create_vcpu(&self, id: u32, vm_ops: Option>) -> Result>; #[cfg(target_arch = "aarch64")] - fn create_vgic(&self, config: VgicConfig) -> Result>>; + fn create_vgic(&self, config: &VgicConfig) -> Result>>; #[cfg(target_arch = "riscv64")] - fn create_vaia(&self, config: VaiaConfig) -> Result>>; + fn create_vaia(&self, config: &VaiaConfig) -> Result>>; /// Registers an event to be signaled whenever a certain address is written to. fn register_ioevent( diff --git a/pci/src/bus.rs b/pci/src/bus.rs index 041324d96..eaae23a4d 100644 --- a/pci/src/bus.rs +++ b/pci/src/bus.rs @@ -133,6 +133,7 @@ impl PciBus { } } + #[allow(clippy::needless_pass_by_value)] pub fn register_mapping( &self, dev: Arc, diff --git a/src/bin/ch-remote.rs b/src/bin/ch-remote.rs index 39bfaeecd..aa95ce58a 100644 --- a/src/bin/ch-remote.rs +++ b/src/bin/ch-remote.rs @@ -1137,6 +1137,7 @@ fn main() { if let Err(top_error) = target_api.do_command(&matches) { // Helper to join strings with a newline. + #[allow(clippy::needless_pass_by_value)] fn join_strs(mut acc: String, next: String) -> String { if !acc.is_empty() { acc.push('\n'); diff --git a/src/main.rs b/src/main.rs index 192ac7488..a5ff044f9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -486,6 +486,7 @@ fn create_app(default_vcpus: String, default_memory: String, default_rng: String .args(args) } +#[allow(clippy::needless_pass_by_value)] fn start_vmm(cmd_arguments: ArgMatches) -> Result, Error> { let log_level = match cmd_arguments.get_count("v") { 0 => LevelFilter::Warn, diff --git a/test_infra/src/lib.rs b/test_infra/src/lib.rs index 2f68a5cc5..96cfe3659 100644 --- a/test_infra/src/lib.rs +++ b/test_infra/src/lib.rs @@ -504,6 +504,7 @@ pub fn rate_limited_copy, Q: AsRef>(from: P, to: Q) -> io:: Err(io::Error::last_os_error()) } +#[allow(clippy::needless_pass_by_value)] pub fn handle_child_output( r: Result<(), std::boxed::Box>, output: &std::process::Output, @@ -804,7 +805,7 @@ pub fn check_lines_count(input: &str, line_count: usize) -> bool { } } -pub fn check_matched_lines_count(input: &str, keywords: Vec<&str>, line_count: usize) -> bool { +pub fn check_matched_lines_count(input: &str, keywords: &[&str], line_count: usize) -> bool { let mut matches = String::new(); for line in input.lines() { if keywords.iter().all(|k| line.contains(k)) { @@ -1047,7 +1048,7 @@ impl Guest { .map_err(Error::WaitForBoot) } - pub fn check_numa_node_cpus(&self, node_id: usize, cpus: Vec) -> Result<(), Error> { + pub fn check_numa_node_cpus(&self, node_id: usize, cpus: &[usize]) -> Result<(), Error> { for cpu in cpus.iter() { let cmd = format!("[ -d \"/sys/devices/system/node/node{node_id}/cpu{cpu}\" ]"); self.ssh_command(cmd.as_str())?; @@ -1072,7 +1073,7 @@ impl Guest { pub fn check_numa_common( &self, mem_ref: Option<&[u32]>, - node_ref: Option<&[Vec]>, + node_ref: Option<&[&[usize]]>, distance_ref: Option<&[&str]>, ) { if let Some(mem_ref) = mem_ref { @@ -1086,7 +1087,7 @@ impl Guest { if let Some(node_ref) = node_ref { // Check each NUMA node has been assigned the right CPUs set. for (i, n) in node_ref.iter().enumerate() { - self.check_numa_node_cpus(i, n.clone()).unwrap(); + self.check_numa_node_cpus(i, n).unwrap(); } } diff --git a/tests/integration.rs b/tests/integration.rs index 845121635..3d2edea21 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -15,7 +15,7 @@ use std::fs::OpenOptions; use std::io::{BufRead, Read, Seek, SeekFrom, Write}; use std::net::TcpListener; use std::os::unix::io::AsRawFd; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::{Child, Command, Stdio}; use std::string::String; use std::sync::mpsc::Receiver; @@ -151,8 +151,8 @@ impl TargetApi { // Start cloud-hypervisor with no VM parameters, only the API server running. // From the API: Create a VM, boot it and check that it looks as expected. -fn _test_api_create_boot(target_api: TargetApi, guest: Guest) { - let mut child = GuestCommand::new(&guest) +fn _test_api_create_boot(target_api: &TargetApi, guest: &Guest) { + let mut child = GuestCommand::new(guest) .args(target_api.guest_args()) .capture_output() .spawn() @@ -196,8 +196,8 @@ fn _test_api_create_boot(target_api: TargetApi, guest: Guest) { // Start cloud-hypervisor with no VM parameters, only the API server running. // From the API: Create a VM, boot it and check it can be shutdown and then // booted again -fn _test_api_shutdown(target_api: TargetApi, guest: Guest) { - let mut child = GuestCommand::new(&guest) +fn _test_api_shutdown(target_api: &TargetApi, guest: &Guest) { + let mut child = GuestCommand::new(guest) .args(target_api.guest_args()) .capture_output() .spawn() @@ -262,8 +262,8 @@ fn _test_api_shutdown(target_api: TargetApi, guest: Guest) { // Start cloud-hypervisor with no VM parameters, only the API server running. // From the API: Create a VM, boot it and check it can be deleted and then recreated // booted again. -fn _test_api_delete(target_api: TargetApi, guest: Guest) { - let mut child = GuestCommand::new(&guest) +fn _test_api_delete(target_api: &TargetApi, guest: &Guest) { + let mut child = GuestCommand::new(guest) .args(target_api.guest_args()) .capture_output() .spawn() @@ -330,8 +330,8 @@ fn _test_api_delete(target_api: TargetApi, guest: Guest) { // From the API: Create a VM, boot it and check that it looks as expected. // Then we pause the VM, check that it's no longer available. // Finally we resume the VM and check that it's available. -fn _test_api_pause_resume(target_api: TargetApi, guest: Guest) { - let mut child = GuestCommand::new(&guest) +fn _test_api_pause_resume(target_api: &TargetApi, guest: &Guest) { + let mut child = GuestCommand::new(guest) .args(target_api.guest_args()) .capture_output() .spawn() @@ -749,10 +749,10 @@ fn setup_ovs_dpdk_guests( ) -> (Child, Child) { setup_ovs_dpdk(); - let clh_path = if !release_binary { - clh_command("cloud-hypervisor") - } else { + let clh_path = if release_binary { cloud_hypervisor_release_path() + } else { + clh_command("cloud-hypervisor") }; let mut child1 = GuestCommand::new_with_binary_path(guest1, &clh_path) @@ -1125,7 +1125,7 @@ fn _test_guest_numa_nodes(acpi: bool) { guest.check_numa_common( Some(&[960_000, 1_920_000, 2_880_000]), - Some(&[vec![0, 1, 2], vec![3, 4], vec![5]]), + Some(&[&[0, 1, 2], &[3, 4], &[5]]), Some(&["10 15 20", "20 10 25", "25 30 10"]), ); @@ -1147,7 +1147,7 @@ fn _test_guest_numa_nodes(acpi: bool) { guest.check_numa_common( Some(&[3_840_000, 3_840_000, 3_840_000]), - Some(&[vec![0, 1, 2, 9], vec![3, 4, 6, 7, 8], vec![5, 10, 11]]), + Some(&[&[0, 1, 2, 9], &[3, 4, 6, 7, 8], &[5, 10, 11]]), None, ); } @@ -2371,20 +2371,21 @@ fn make_guest_panic(guest: &Guest) { // and write data to host(guest write data to ivshmem pci bar2 memory, host read it from // ivshmem backend file). // It also checks the size of the shared memory region. -fn _test_ivshmem(guest: &Guest, ivshmem_file_path: String, file_size: &str) { +fn _test_ivshmem(guest: &Guest, ivshmem_file_path: impl AsRef, file_size: &str) { + let ivshmem_file_path = ivshmem_file_path.as_ref(); let test_message_read = String::from("ivshmem device test data read"); // Modify backend file data before function test let mut file = OpenOptions::new() .read(true) .write(true) - .open(ivshmem_file_path.as_str()) + .open(ivshmem_file_path) .unwrap(); file.seek(SeekFrom::Start(0)).unwrap(); file.write_all(test_message_read.as_bytes()).unwrap(); file.write_all(b"\0").unwrap(); file.flush().unwrap(); - let output = fs::read_to_string(ivshmem_file_path.as_str()).unwrap(); + let output = fs::read_to_string(ivshmem_file_path).unwrap(); let nul_pos = output.as_bytes().iter().position(|&b| b == 0).unwrap(); let c_str = CStr::from_bytes_until_nul(&output.as_bytes()[..=nul_pos]).unwrap(); let file_message = c_str.to_string_lossy().to_string(); @@ -2498,7 +2499,7 @@ EOF let _ = guest.ssh_command("sudo python3 test_write.py").unwrap(); - let output = fs::read_to_string(ivshmem_file_path.as_str()).unwrap(); + let output = fs::read_to_string(ivshmem_file_path).unwrap(); let nul_pos = output.as_bytes().iter().position(|&b| b == 0).unwrap(); let c_str = CStr::from_bytes_until_nul(&output.as_bytes()[..=nul_pos]).unwrap(); let file_message = c_str.to_string_lossy().to_string(); @@ -2515,17 +2516,19 @@ mod common_parallel { #[test] #[cfg(target_arch = "x86_64")] fn test_focal_hypervisor_fw() { - test_simple_launch(fw_path(FwType::RustHypervisorFirmware), FOCAL_IMAGE_NAME); + let path = fw_path(FwType::RustHypervisorFirmware); + test_simple_launch(&path, FOCAL_IMAGE_NAME); } #[test] #[cfg(target_arch = "x86_64")] fn test_focal_ovmf() { - test_simple_launch(fw_path(FwType::Ovmf), FOCAL_IMAGE_NAME); + let path = fw_path(FwType::Ovmf); + test_simple_launch(&path, FOCAL_IMAGE_NAME); } #[cfg(target_arch = "x86_64")] - fn test_simple_launch(fw_path: String, disk_path: &str) { + fn test_simple_launch(fw_path: &str, disk_path: &str) { let disk_config = Box::new(UbuntuDiskConfig::new(disk_path.to_string())); let guest = Guest::new(disk_config); let event_path = temp_event_monitor_path(&guest.tmp_dir); @@ -2533,7 +2536,7 @@ mod common_parallel { let mut child = GuestCommand::new(&guest) .args(["--cpus", "boot=1"]) .args(["--memory", "size=512M"]) - .args(["--kernel", fw_path.as_str()]) + .args(["--kernel", fw_path]) .default_disks() .default_net() .args(["--serial", "tty", "--console", "off"]) @@ -4702,7 +4705,7 @@ mod common_parallel { // does not have this command line tag. assert!(check_matched_lines_count( guest.ssh_command_l2_1("cat /proc/cmdline").unwrap().trim(), - vec!["VFIOTAG"], + &["VFIOTAG"], 1 )); @@ -4710,7 +4713,7 @@ mod common_parallel { // the L2 VM. assert!(check_matched_lines_count( guest.ssh_command_l2_2("cat /proc/cmdline").unwrap().trim(), - vec!["VFIOTAG"], + &["VFIOTAG"], 1 )); @@ -4726,7 +4729,7 @@ mod common_parallel { // Check both if /dev/vdc exists and if the block size is 16M in L2 VM assert!(check_matched_lines_count( guest.ssh_command_l2_1("lsblk").unwrap().trim(), - vec!["vdc", "16M"], + &["vdc", "16M"], 1 )); @@ -4748,7 +4751,7 @@ mod common_parallel { .unwrap(); assert!(check_matched_lines_count( vfio_hotplug_output.trim(), - vec!["{\"id\":\"vfio123\",\"bdf\":\"0000:00:08.0\"}"], + &["{\"id\":\"vfio123\",\"bdf\":\"0000:00:08.0\"}"], 1 )); @@ -4759,7 +4762,7 @@ mod common_parallel { // VM, so this is our way to validate hotplug works for VFIO PCI. assert!(check_matched_lines_count( guest.ssh_command_l2_3("cat /proc/cmdline").unwrap().trim(), - vec!["VFIOTAG"], + &["VFIOTAG"], 1 )); @@ -4873,7 +4876,8 @@ mod common_parallel { let disk_config = UbuntuDiskConfig::new(JAMMY_IMAGE_NAME.to_string()); let guest = Guest::new(Box::new(disk_config)); - _test_api_shutdown(TargetApi::new_http_api(&guest.tmp_dir), guest); + let target_api = TargetApi::new_http_api(&guest.tmp_dir); + _test_api_shutdown(&target_api, &guest); } #[test] @@ -4881,7 +4885,8 @@ mod common_parallel { let disk_config = UbuntuDiskConfig::new(JAMMY_IMAGE_NAME.to_string()); let guest = Guest::new(Box::new(disk_config)); - _test_api_delete(TargetApi::new_http_api(&guest.tmp_dir), guest); + let target_api = TargetApi::new_http_api(&guest.tmp_dir); + _test_api_delete(&target_api, &guest); } #[test] @@ -4889,7 +4894,8 @@ mod common_parallel { let disk_config = UbuntuDiskConfig::new(JAMMY_IMAGE_NAME.to_string()); let guest = Guest::new(Box::new(disk_config)); - _test_api_pause_resume(TargetApi::new_http_api(&guest.tmp_dir), guest); + let target_api = TargetApi::new_http_api(&guest.tmp_dir); + _test_api_pause_resume(&target_api, &guest); } #[test] @@ -4897,7 +4903,8 @@ mod common_parallel { let disk_config = UbuntuDiskConfig::new(JAMMY_IMAGE_NAME.to_string()); let guest = Guest::new(Box::new(disk_config)); - _test_api_create_boot(TargetApi::new_http_api(&guest.tmp_dir), guest); + let target_api = TargetApi::new_http_api(&guest.tmp_dir); + _test_api_create_boot(&target_api, &guest); } #[test] @@ -7541,7 +7548,8 @@ mod dbus_api { let disk_config = UbuntuDiskConfig::new(JAMMY_IMAGE_NAME.to_string()); let guest = Guest::new(Box::new(disk_config)); - _test_api_create_boot(TargetApi::new_dbus_api(&guest.tmp_dir), guest); + let target_api = TargetApi::new_dbus_api(&guest.tmp_dir); + _test_api_create_boot(&target_api, &guest); } #[test] @@ -7549,7 +7557,8 @@ mod dbus_api { let disk_config = UbuntuDiskConfig::new(JAMMY_IMAGE_NAME.to_string()); let guest = Guest::new(Box::new(disk_config)); - _test_api_shutdown(TargetApi::new_dbus_api(&guest.tmp_dir), guest); + let target_api = TargetApi::new_dbus_api(&guest.tmp_dir); + _test_api_shutdown(&target_api, &guest); } #[test] @@ -7557,7 +7566,8 @@ mod dbus_api { let disk_config = UbuntuDiskConfig::new(JAMMY_IMAGE_NAME.to_string()); let guest = Guest::new(Box::new(disk_config)); - _test_api_delete(TargetApi::new_dbus_api(&guest.tmp_dir), guest); + let target_api = TargetApi::new_dbus_api(&guest.tmp_dir); + _test_api_delete(&target_api, &guest); } #[test] @@ -7565,7 +7575,8 @@ mod dbus_api { let disk_config = UbuntuDiskConfig::new(JAMMY_IMAGE_NAME.to_string()); let guest = Guest::new(Box::new(disk_config)); - _test_api_pause_resume(TargetApi::new_dbus_api(&guest.tmp_dir), guest); + let target_api = TargetApi::new_dbus_api(&guest.tmp_dir); + _test_api_pause_resume(&target_api, &guest); } } @@ -7693,7 +7704,7 @@ mod ivshmem { } // Check ivshmem device in src guest. - _test_ivshmem(&guest, ivshmem_file_path.clone(), file_size); + _test_ivshmem(&guest, &ivshmem_file_path, file_size); // Allow some normal time to elapse to check we don't get spurious reboots thread::sleep(std::time::Duration::new(40, 0)); @@ -7748,7 +7759,7 @@ mod ivshmem { guest.check_devices_common(None, Some(&console_text), Some(&pmem_path)); // Check ivshmem device - _test_ivshmem(&guest, ivshmem_file_path, file_size); + _test_ivshmem(&guest, &ivshmem_file_path, file_size); }); // Clean-up the destination VM and make sure it terminated correctly @@ -7810,7 +7821,7 @@ mod ivshmem { let r = std::panic::catch_unwind(|| { guest.wait_vm_boot(None).unwrap(); - _test_ivshmem(&guest, ivshmem_file_path, file_size); + _test_ivshmem(&guest, &ivshmem_file_path, file_size); }); kill_child(&mut child); let output = child.wait_with_output().unwrap(); @@ -7958,7 +7969,7 @@ mod ivshmem { // Check the number of vCPUs assert_eq!(guest.get_cpu_count().unwrap_or_default(), 2); guest.check_devices_common(Some(&socket), Some(&console_text), None); - _test_ivshmem(&guest, ivshmem_file_path, file_size); + _test_ivshmem(&guest, &ivshmem_file_path, file_size); }); // Shutdown the target VM and check console output kill_child(&mut child); @@ -8278,9 +8289,7 @@ mod common_sequential { // Perform same checks to validate VM has been properly restored assert_eq!(guest.get_cpu_count().unwrap_or_default(), 4); let total_memory = guest.get_total_memory().unwrap_or_default(); - if !use_hotplug { - assert!(total_memory > 1_920_000); - } else { + if use_hotplug { assert!(total_memory > 4_800_000); assert!(total_memory < 5_760_000); // Deflate balloon to restore entire RAM to the VM @@ -8293,6 +8302,8 @@ mod common_sequential { let total_memory = guest.get_total_memory().unwrap_or_default(); assert!(total_memory > 4_800_000); assert!(total_memory < 5_760_000); + } else { + assert!(total_memory > 1_920_000); } guest.check_devices_common(Some(&socket), Some(&console_text), None); @@ -10050,10 +10061,10 @@ mod live_migration { let pmem_path = String::from("/dev/pmem0"); // Start the source VM - let src_vm_path = if !upgrade_test { - clh_command("cloud-hypervisor") - } else { + let src_vm_path = if upgrade_test { cloud_hypervisor_release_path() + } else { + clh_command("cloud-hypervisor") }; let src_api_socket = temp_api_path(&guest.tmp_dir); let mut src_vm_cmd = GuestCommand::new_with_binary_path(&guest, &src_vm_path); @@ -10214,10 +10225,10 @@ mod live_migration { let pmem_path = String::from("/dev/pmem0"); // Start the source VM - let src_vm_path = if !upgrade_test { - clh_command("cloud-hypervisor") - } else { + let src_vm_path = if upgrade_test { cloud_hypervisor_release_path() + } else { + clh_command("cloud-hypervisor") }; let src_api_socket = temp_api_path(&guest.tmp_dir); let mut src_vm_cmd = GuestCommand::new_with_binary_path(&guest, &src_vm_path); @@ -10415,10 +10426,10 @@ mod live_migration { let pmem_path = String::from("/dev/pmem0"); // Start the source VM - let src_vm_path = if !upgrade_test { - clh_command("cloud-hypervisor") - } else { + let src_vm_path = if upgrade_test { cloud_hypervisor_release_path() + } else { + clh_command("cloud-hypervisor") }; let src_api_socket = temp_api_path(&guest.tmp_dir); let mut src_vm_cmd = GuestCommand::new_with_binary_path(&guest, &src_vm_path); @@ -10467,7 +10478,7 @@ mod live_migration { { guest.check_numa_common( Some(&[960_000, 960_000, 1_920_000]), - Some(&[vec![0, 1, 2], vec![3, 4], vec![5]]), + Some(&[&[0, 1, 2], &[3, 4], &[5]]), Some(&["10 15 20", "20 10 25", "25 30 10"]), ); @@ -10563,7 +10574,7 @@ mod live_migration { { guest.check_numa_common( Some(&[960_000, 960_000, 1_920_000]), - Some(&[vec![0, 1, 2], vec![3, 4], vec![5]]), + Some(&[&[0, 1, 2], &[3, 4], &[5]]), Some(&["10 15 20", "20 10 25", "25 30 10"]), ); } @@ -10574,7 +10585,7 @@ mod live_migration { { guest.check_numa_common( Some(&[1_920_000, 1_920_000, 2_880_000]), - Some(&[vec![0, 1, 2], vec![3, 4], vec![5]]), + Some(&[&[0, 1, 2], &[3, 4], &[5]]), Some(&["10 15 20", "20 10 25", "25 30 10"]), ); @@ -10592,7 +10603,7 @@ mod live_migration { guest.check_numa_common( Some(&[3_840_000, 3_840_000, 3_840_000]), - Some(&[vec![0, 1, 2, 9], vec![3, 4, 6, 7, 8], vec![5, 10, 11]]), + Some(&[&[0, 1, 2, 9], &[3, 4, 6, 7, 8], &[5, 10, 11]]), None, ); } @@ -10640,10 +10651,10 @@ mod live_migration { let pmem_path = String::from("/dev/pmem0"); // Start the source VM - let src_vm_path = if !upgrade_test { - clh_command("cloud-hypervisor") - } else { + let src_vm_path = if upgrade_test { cloud_hypervisor_release_path() + } else { + clh_command("cloud-hypervisor") }; let src_api_socket = temp_api_path(&guest.tmp_dir); let mut src_vm_cmd = GuestCommand::new_with_binary_path(&guest, &src_vm_path); diff --git a/virtio-devices/src/balloon.rs b/virtio-devices/src/balloon.rs index ca690ff3f..3db683261 100644 --- a/virtio-devices/src/balloon.rs +++ b/virtio-devices/src/balloon.rs @@ -342,8 +342,8 @@ impl BalloonEpollHandler { fn run( &mut self, - paused: Arc, - paused_sync: Arc, + paused: &AtomicBool, + paused_sync: &Barrier, ) -> result::Result<(), EpollHelperError> { let mut helper = EpollHelper::new(&self.kill_evt, &self.pause_evt)?; helper.add_event(self.inflate_queue_evt.as_raw_fd(), INFLATE_QUEUE_EVENT)?; @@ -639,7 +639,7 @@ impl VirtioDevice for Balloon { Thread::VirtioBalloon, &mut epoll_threads, &self.exit_evt, - move || handler.run(paused, paused_sync.unwrap()), + move || handler.run(&paused, paused_sync.as_ref().unwrap()), )?; self.common.epoll_threads = Some(epoll_threads); diff --git a/virtio-devices/src/block.rs b/virtio-devices/src/block.rs index 898b13616..70ec6f9de 100644 --- a/virtio-devices/src/block.rs +++ b/virtio-devices/src/block.rs @@ -531,8 +531,8 @@ impl BlockEpollHandler { fn run( &mut self, - paused: Arc, - paused_sync: Arc, + paused: &AtomicBool, + paused_sync: &Barrier, ) -> result::Result<(), EpollHelperError> { let mut helper = EpollHelper::new(&self.kill_evt, &self.pause_evt)?; helper.add_event(self.queue_evt.as_raw_fd(), QUEUE_AVAIL_EVENT)?; @@ -987,7 +987,7 @@ impl VirtioDevice for Block { Thread::VirtioBlock, &mut epoll_threads, &self.exit_evt, - move || handler.run(paused, paused_sync.unwrap()), + move || handler.run(&paused, paused_sync.as_ref().unwrap()), )?; } diff --git a/virtio-devices/src/console.rs b/virtio-devices/src/console.rs index a036a5461..c8a9f08a0 100644 --- a/virtio-devices/src/console.rs +++ b/virtio-devices/src/console.rs @@ -284,8 +284,8 @@ impl ConsoleEpollHandler { fn run( &mut self, - paused: Arc, - paused_sync: Arc, + paused: &AtomicBool, + paused_sync: &Barrier, ) -> result::Result<(), EpollHelperError> { let mut helper = EpollHelper::new(&self.kill_evt, &self.pause_evt)?; helper.add_event(self.input_queue_evt.as_raw_fd(), INPUT_QUEUE_EVENT)?; @@ -752,7 +752,7 @@ impl VirtioDevice for Console { Thread::VirtioConsole, &mut epoll_threads, &self.exit_evt, - move || handler.run(paused, paused_sync.unwrap()), + move || handler.run(&paused, paused_sync.as_ref().unwrap()), )?; self.common.epoll_threads = Some(epoll_threads); diff --git a/virtio-devices/src/epoll_helper.rs b/virtio-devices/src/epoll_helper.rs index c20c1190a..dc062c8d3 100644 --- a/virtio-devices/src/epoll_helper.rs +++ b/virtio-devices/src/epoll_helper.rs @@ -10,8 +10,8 @@ use std::fs::File; use std::os::unix::io::{AsRawFd, FromRawFd, RawFd}; +use std::sync::Barrier; use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::{Arc, Barrier}; use std::thread; use log::info; @@ -153,8 +153,8 @@ impl EpollHelper { pub fn run( &mut self, - paused: Arc, - paused_sync: Arc, + paused: &AtomicBool, + paused_sync: &Barrier, handler: &mut dyn EpollHelperHandler, ) -> std::result::Result<(), EpollHelperError> { self.run_with_timeout(paused, paused_sync, handler, -1, false) @@ -163,8 +163,8 @@ impl EpollHelper { #[cfg(not(fuzzing))] pub fn run_with_timeout( &mut self, - paused: Arc, - paused_sync: Arc, + paused: &AtomicBool, + paused_sync: &Barrier, handler: &mut dyn EpollHelperHandler, timeout: i32, enable_event_list: bool, @@ -250,8 +250,8 @@ impl EpollHelper { // and return when no epoll events are active pub fn run_with_timeout( &mut self, - paused: Arc, - paused_sync: Arc, + paused: &AtomicBool, + paused_sync: &Barrier, handler: &mut dyn EpollHelperHandler, _timeout: i32, _enable_event_list: bool, diff --git a/virtio-devices/src/iommu.rs b/virtio-devices/src/iommu.rs index 1318e575f..f4812b04f 100644 --- a/virtio-devices/src/iommu.rs +++ b/virtio-devices/src/iommu.rs @@ -727,8 +727,8 @@ impl IommuEpollHandler { fn run( &mut self, - paused: Arc, - paused_sync: Arc, + paused: &AtomicBool, + paused_sync: &Barrier, ) -> result::Result<(), EpollHelperError> { let mut helper = EpollHelper::new(&self.kill_evt, &self.pause_evt)?; helper.add_event(self.request_queue_evt.as_raw_fd(), REQUEST_Q_EVENT)?; @@ -1110,7 +1110,7 @@ impl VirtioDevice for Iommu { Thread::VirtioIommu, &mut epoll_threads, &self.exit_evt, - move || handler.run(paused, paused_sync.unwrap()), + move || handler.run(&paused, paused_sync.as_ref().unwrap()), )?; self.common.epoll_threads = Some(epoll_threads); diff --git a/virtio-devices/src/mem.rs b/virtio-devices/src/mem.rs index 3890928a5..936fdbe42 100644 --- a/virtio-devices/src/mem.rs +++ b/virtio-devices/src/mem.rs @@ -669,8 +669,8 @@ impl MemEpollHandler { fn run( &mut self, - paused: Arc, - paused_sync: Arc, + paused: &AtomicBool, + paused_sync: &Barrier, ) -> result::Result<(), EpollHelperError> { let mut helper = EpollHelper::new(&self.kill_evt, &self.pause_evt)?; helper.add_event(self.queue_evt.as_raw_fd(), QUEUE_AVAIL_EVENT)?; @@ -879,13 +879,13 @@ impl Mem { pub fn remove_dma_mapping_handler( &mut self, - source: VirtioMemMappingSource, + source: &VirtioMemMappingSource, ) -> result::Result<(), Error> { let handler = self .dma_mapping_handlers .lock() .unwrap() - .remove(&source) + .remove(source) .ok_or(Error::InvalidDmaMappingHandler)?; let config = self.config.lock().unwrap(); @@ -1003,7 +1003,7 @@ impl VirtioDevice for Mem { Thread::VirtioMem, &mut epoll_threads, &self.exit_evt, - move || handler.run(paused, paused_sync.unwrap()), + move || handler.run(&paused, paused_sync.as_ref().unwrap()), )?; self.common.epoll_threads = Some(epoll_threads); diff --git a/virtio-devices/src/net.rs b/virtio-devices/src/net.rs index 71faf8549..8eee66134 100644 --- a/virtio-devices/src/net.rs +++ b/virtio-devices/src/net.rs @@ -75,8 +75,8 @@ impl NetCtrlEpollHandler { pub fn run_ctrl( &mut self, - paused: Arc, - paused_sync: Arc, + paused: &AtomicBool, + paused_sync: &Barrier, ) -> std::result::Result<(), EpollHelperError> { let mut helper = EpollHelper::new(&self.kill_evt, &self.pause_evt)?; helper.add_event(self.queue_evt.as_raw_fd(), CTRL_QUEUE_EVENT)?; @@ -266,8 +266,8 @@ impl NetEpollHandler { fn run( &mut self, - paused: Arc, - paused_sync: Arc, + paused: &AtomicBool, + paused_sync: &Barrier, ) -> result::Result<(), EpollHelperError> { let mut helper = EpollHelper::new(&self.kill_evt, &self.pause_evt)?; helper.add_event(self.queue_evt_pair.0.as_raw_fd(), RX_QUEUE_EVENT)?; @@ -736,7 +736,7 @@ impl VirtioDevice for Net { Thread::VirtioNetCtl, &mut epoll_threads, &self.exit_evt, - move || ctrl_handler.run_ctrl(paused, paused_sync.unwrap()), + move || ctrl_handler.run_ctrl(&paused, paused_sync.as_ref().unwrap()), )?; self.ctrl_queue_epoll_thread = Some(epoll_threads.remove(0)); } @@ -814,7 +814,7 @@ impl VirtioDevice for Net { Thread::VirtioNet, &mut epoll_threads, &self.exit_evt, - move || handler.run(paused, paused_sync.unwrap()), + move || handler.run(&paused, paused_sync.as_ref().unwrap()), )?; } diff --git a/virtio-devices/src/pmem.rs b/virtio-devices/src/pmem.rs index da3722e25..549b62fd9 100644 --- a/virtio-devices/src/pmem.rs +++ b/virtio-devices/src/pmem.rs @@ -217,8 +217,8 @@ impl PmemEpollHandler { fn run( &mut self, - paused: Arc, - paused_sync: Arc, + paused: &AtomicBool, + paused_sync: &Barrier, ) -> result::Result<(), EpollHelperError> { let mut helper = EpollHelper::new(&self.kill_evt, &self.pause_evt)?; helper.add_event(self.queue_evt.as_raw_fd(), QUEUE_AVAIL_EVENT)?; @@ -414,7 +414,7 @@ impl VirtioDevice for Pmem { Thread::VirtioPmem, &mut epoll_threads, &self.exit_evt, - move || handler.run(paused, paused_sync.unwrap()), + move || handler.run(&paused, paused_sync.as_ref().unwrap()), )?; self.common.epoll_threads = Some(epoll_threads); diff --git a/virtio-devices/src/rng.rs b/virtio-devices/src/rng.rs index aff511b0a..2f980d4d8 100644 --- a/virtio-devices/src/rng.rs +++ b/virtio-devices/src/rng.rs @@ -104,8 +104,8 @@ impl RngEpollHandler { fn run( &mut self, - paused: Arc, - paused_sync: Arc, + paused: &AtomicBool, + paused_sync: &Barrier, ) -> result::Result<(), EpollHelperError> { let mut helper = EpollHelper::new(&self.kill_evt, &self.pause_evt)?; helper.add_event(self.queue_evt.as_raw_fd(), QUEUE_AVAIL_EVENT)?; @@ -281,7 +281,7 @@ impl VirtioDevice for Rng { Thread::VirtioRng, &mut epoll_threads, &self.exit_evt, - move || handler.run(paused, paused_sync.unwrap()), + move || handler.run(&paused, paused_sync.as_ref().unwrap()), )?; self.common.epoll_threads = Some(epoll_threads); diff --git a/virtio-devices/src/seccomp_filters.rs b/virtio-devices/src/seccomp_filters.rs index 26d1445f6..9c8cc3e7d 100644 --- a/virtio-devices/src/seccomp_filters.rs +++ b/virtio-devices/src/seccomp_filters.rs @@ -11,6 +11,7 @@ use seccompiler::{ SeccompFilter, SeccompRule, }; +#[derive(Clone, Copy)] pub enum Thread { VirtioBalloon, VirtioBlock, diff --git a/virtio-devices/src/transport/pci_common_config.rs b/virtio-devices/src/transport/pci_common_config.rs index 1a576ad5a..5a7b5f57a 100644 --- a/virtio-devices/src/transport/pci_common_config.rs +++ b/virtio-devices/src/transport/pci_common_config.rs @@ -82,6 +82,8 @@ pub struct VirtioPciCommonConfigState { const VRING_DESC_ELEMENT_SIZE: usize = 16; const VRING_AVAIL_ELEMENT_SIZE: usize = 2; const VRING_USED_ELEMENT_SIZE: usize = 8; + +#[derive(Copy, Clone)] pub enum VringType { Desc, Avail, @@ -191,6 +193,7 @@ impl VirtioPciCommonConfig { } } + #[allow(clippy::needless_pass_by_value)] pub fn write( &mut self, offset: u64, @@ -297,6 +300,7 @@ impl VirtioPciCommonConfig { } } + #[allow(clippy::needless_pass_by_value)] fn read_common_config_dword(&self, offset: u64, device: Arc>) -> u32 { debug!("read_common_config_dword: offset 0x{offset:x}"); match offset { @@ -319,6 +323,7 @@ impl VirtioPciCommonConfig { } } + #[allow(clippy::needless_pass_by_value)] fn write_common_config_dword( &mut self, offset: u64, diff --git a/virtio-devices/src/vhost_user/blk.rs b/virtio-devices/src/vhost_user/blk.rs index 10bc87408..6576013e8 100644 --- a/virtio-devices/src/vhost_user/blk.rs +++ b/virtio-devices/src/vhost_user/blk.rs @@ -297,7 +297,7 @@ impl VirtioDevice for Blk { let mut handler = self.vu_common.activate( mem, - queues, + &queues, interrupt_cb, self.common.acked_features, backend_req_handler, @@ -316,7 +316,7 @@ impl VirtioDevice for Blk { Thread::VirtioVhostBlock, &mut epoll_threads, &self.exit_evt, - move || handler.run(paused, paused_sync.unwrap()), + move || handler.run(&paused, paused_sync.as_ref().unwrap()), )?; self.epoll_thread = Some(epoll_threads.remove(0)); diff --git a/virtio-devices/src/vhost_user/fs.rs b/virtio-devices/src/vhost_user/fs.rs index 726d88367..d0005af90 100644 --- a/virtio-devices/src/vhost_user/fs.rs +++ b/virtio-devices/src/vhost_user/fs.rs @@ -277,7 +277,7 @@ impl VirtioDevice for Fs { let mut handler = self.vu_common.activate( mem, - queues, + &queues, interrupt_cb, self.common.acked_features, backend_req_handler, @@ -295,7 +295,7 @@ impl VirtioDevice for Fs { Thread::VirtioVhostFs, &mut epoll_threads, &self.exit_evt, - move || handler.run(paused, paused_sync.unwrap()), + move || handler.run(&paused, paused_sync.as_ref().unwrap()), )?; self.epoll_thread = Some(epoll_threads.remove(0)); diff --git a/virtio-devices/src/vhost_user/mod.rs b/virtio-devices/src/vhost_user/mod.rs index 764b0d1c0..05233a0be 100644 --- a/virtio-devices/src/vhost_user/mod.rs +++ b/virtio-devices/src/vhost_user/mod.rs @@ -183,8 +183,8 @@ pub struct VhostUserEpollHandler { impl VhostUserEpollHandler { pub fn run( &mut self, - paused: Arc, - paused_sync: Arc, + paused: &AtomicBool, + paused_sync: &Barrier, ) -> std::result::Result<(), EpollHelperError> { let mut helper = EpollHelper::new(&self.kill_evt, &self.pause_evt)?; helper.add_event_custom( @@ -221,14 +221,16 @@ impl VhostUserEpollHandler { ))) })?; + let queues = self + .queues + .iter() + .map(|(i, q, e)| (*i, vm_virtio::clone_queue(q), e.try_clone().unwrap())) + .collect::>(); // Initialize the backend vhost_user .reinitialize_vhost_user( self.mem.memory().deref(), - self.queues - .iter() - .map(|(i, q, e)| (*i, vm_virtio::clone_queue(q), e.try_clone().unwrap())) - .collect(), + &queues, self.virtio_interrupt.as_ref(), self.acked_features, self.acked_protocol_features, @@ -305,7 +307,7 @@ impl VhostUserCommon { pub fn activate( &mut self, mem: GuestMemoryAtomic, - queues: Vec<(usize, Queue, EventFd)>, + queues: &[(usize, Queue, EventFd)], interrupt_cb: Arc, acked_features: u64, backend_req_handler: Option>, @@ -325,14 +327,15 @@ impl VhostUserCommon { return Err(ActivateError::BadActivate); } let vu = self.vu.as_ref().unwrap(); + let queues = queues + .iter() + .map(|(i, q, e)| (*i, vm_virtio::clone_queue(q), e.try_clone().unwrap())) + .collect::>(); vu.lock() .unwrap() .setup_vhost_user( &mem.memory(), - queues - .iter() - .map(|(i, q, e)| (*i, vm_virtio::clone_queue(q), e.try_clone().unwrap())) - .collect(), + &queues, interrupt_cb.as_ref(), acked_features, &backend_req_handler, diff --git a/virtio-devices/src/vhost_user/net.rs b/virtio-devices/src/vhost_user/net.rs index fb79d52d8..e25e8ef11 100644 --- a/virtio-devices/src/vhost_user/net.rs +++ b/virtio-devices/src/vhost_user/net.rs @@ -336,7 +336,7 @@ impl VirtioDevice for Net { Thread::VirtioVhostNetCtl, &mut epoll_threads, &self.exit_evt, - move || ctrl_handler.run_ctrl(paused, paused_sync.unwrap()), + move || ctrl_handler.run_ctrl(&paused, paused_sync.as_ref().unwrap()), )?; self.ctrl_queue_epoll_thread = Some(epoll_threads.remove(0)); } @@ -353,7 +353,7 @@ impl VirtioDevice for Net { let mut handler = self.vu_common.activate( mem, - queues, + &queues, interrupt_cb, backend_acked_features, backend_req_handler, @@ -371,7 +371,7 @@ impl VirtioDevice for Net { Thread::VirtioVhostNet, &mut epoll_threads, &self.exit_evt, - move || handler.run(paused, paused_sync.unwrap()), + move || handler.run(&paused, paused_sync.as_ref().unwrap()), )?; self.epoll_thread = Some(epoll_threads.remove(0)); diff --git a/virtio-devices/src/vhost_user/vu_common_ctrl.rs b/virtio-devices/src/vhost_user/vu_common_ctrl.rs index 76a792c87..264635149 100644 --- a/virtio-devices/src/vhost_user/vu_common_ctrl.rs +++ b/virtio-devices/src/vhost_user/vu_common_ctrl.rs @@ -156,7 +156,7 @@ impl VhostUserHandle { pub fn setup_vhost_user( &mut self, mem: &GuestMemoryMmap, - queues: Vec<(usize, Queue, EventFd)>, + queues: &[(usize, Queue, EventFd)], virtio_interrupt: &dyn VirtioInterrupt, acked_features: u64, backend_req_handler: &Option>, @@ -340,7 +340,7 @@ impl VhostUserHandle { pub fn reinitialize_vhost_user( &mut self, mem: &GuestMemoryMmap, - queues: Vec<(usize, Queue, EventFd)>, + queues: &[(usize, Queue, EventFd)], virtio_interrupt: &dyn VirtioInterrupt, acked_features: u64, acked_protocol_features: u64, diff --git a/virtio-devices/src/vsock/device.rs b/virtio-devices/src/vsock/device.rs index 63cfb3a67..506e2dcba 100644 --- a/virtio-devices/src/vsock/device.rs +++ b/virtio-devices/src/vsock/device.rs @@ -201,8 +201,8 @@ where fn run( &mut self, - paused: Arc, - paused_sync: Arc, + paused: &AtomicBool, + paused_sync: &Barrier, ) -> result::Result<(), EpollHelperError> { let mut helper = EpollHelper::new(&self.kill_evt, &self.pause_evt)?; helper.add_event(self.queue_evts[0].as_raw_fd(), RX_QUEUE_EVENT)?; @@ -463,7 +463,7 @@ where Thread::VirtioVsock, &mut epoll_threads, &self.exit_evt, - move || handler.run(paused, paused_sync.unwrap()), + move || handler.run(&paused, paused_sync.as_ref().unwrap()), )?; self.common.epoll_threads = Some(epoll_threads); diff --git a/virtio-devices/src/watchdog.rs b/virtio-devices/src/watchdog.rs index 2f76cae37..6b9f7cc0a 100644 --- a/virtio-devices/src/watchdog.rs +++ b/virtio-devices/src/watchdog.rs @@ -121,8 +121,8 @@ impl WatchdogEpollHandler { fn run( &mut self, - paused: Arc, - paused_sync: Arc, + paused: &AtomicBool, + paused_sync: &Barrier, ) -> result::Result<(), EpollHelperError> { let mut helper = EpollHelper::new(&self.kill_evt, &self.pause_evt)?; helper.add_event(self.queue_evt.as_raw_fd(), QUEUE_AVAIL_EVENT)?; @@ -369,7 +369,7 @@ impl VirtioDevice for Watchdog { Thread::VirtioWatchdog, &mut epoll_threads, &self.exit_evt, - move || handler.run(paused, paused_sync.unwrap()), + move || handler.run(&paused, paused_sync.as_ref().unwrap()), )?; self.common.epoll_threads = Some(epoll_threads); diff --git a/vm-device/src/bus.rs b/vm-device/src/bus.rs index 92916a65b..a029f0cc9 100644 --- a/vm-device/src/bus.rs +++ b/vm-device/src/bus.rs @@ -147,6 +147,7 @@ impl Bus { None } + #[allow(clippy::needless_pass_by_value)] pub fn insert(&self, device: Arc, base: u64, len: u64) -> Result<()> { if len == 0 { return Err(Error::ZeroSizedRange); diff --git a/vmm/src/acpi.rs b/vmm/src/acpi.rs index 56513a6d0..7fca8435f 100644 --- a/vmm/src/acpi.rs +++ b/vmm/src/acpi.rs @@ -105,6 +105,7 @@ struct ProcessorGiccAffinity { } bitflags! { + #[derive(Copy, Clone)] pub struct MemAffinityFlags: u32 { const NOFLAGS = 0; const ENABLE = 0b1; diff --git a/vmm/src/api/http/mod.rs b/vmm/src/api/http/mod.rs index a35c9aca0..9ba679a80 100644 --- a/vmm/src/api/http/mod.rs +++ b/vmm/src/api/http/mod.rs @@ -76,6 +76,7 @@ const HTTP_ROOT: &str = "/api/v1"; /// The error message contained in the response is supposed to be user-facing, /// thus insightful and helpful while balancing technical accuracy and /// simplicity. +#[allow(clippy::needless_pass_by_value)] pub fn error_response(error: HttpError, status: StatusCode) -> Response { let mut response = Response::new(Version::Http11, status); diff --git a/vmm/src/api/mod.rs b/vmm/src/api/mod.rs index 694eb961b..a0b090542 100644 --- a/vmm/src/api/mod.rs +++ b/vmm/src/api/mod.rs @@ -369,6 +369,7 @@ pub trait RequestHandler { pub type ApiRequest = Box Result + Send + 'static>; +#[allow(clippy::needless_pass_by_value)] fn get_response( action: &Action, api_evt: EventFd, diff --git a/vmm/src/config.rs b/vmm/src/config.rs index 6d5dcf5da..3c99eb26f 100644 --- a/vmm/src/config.rs +++ b/vmm/src/config.rs @@ -811,6 +811,7 @@ impl PlatformConfig { } impl MemoryConfig { + #[allow(clippy::needless_pass_by_value)] pub fn parse(memory: &str, memory_zones: Option>) -> Result { let mut parser = OptionParser::new(); parser diff --git a/vmm/src/console_devices.rs b/vmm/src/console_devices.rs index 9f8d18ae7..70bcabae5 100644 --- a/vmm/src/console_devices.rs +++ b/vmm/src/console_devices.rs @@ -76,7 +76,7 @@ pub struct ConsoleInfo { fn modify_mode( fd: RawFd, f: F, - original_termios_opt: Arc>>, + original_termios_opt: &Mutex>, ) -> vmm_sys_util::errno::Result<()> { // SAFETY: safe because we check the return value of isatty. if unsafe { isatty(fd) } != 1 { @@ -109,7 +109,7 @@ fn modify_mode( fn set_raw_mode( f: &dyn AsRawFd, - original_termios_opt: Arc>>, + original_termios_opt: &Mutex>, ) -> ConsoleDeviceResult<()> { modify_mode( f.as_raw_fd(), @@ -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.clone())?; + set_raw_mode(&sub_fd.as_raw_fd(), &vmm.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.clone())?; + set_raw_mode(&sub_fd.as_raw_fd(), &vmm.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.clone())?; + set_raw_mode(&sub_fd.as_raw_fd(), &vmm.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.clone())?; + set_raw_mode(&out, &vmm.original_termios_opt)?; ConsoleOutput::Tty(Arc::new(out)) } ConsoleOutputMode::Socket => { diff --git a/vmm/src/cpu.rs b/vmm/src/cpu.rs index b3d7f71ef..e501b5c33 100644 --- a/vmm/src/cpu.rs +++ b/vmm/src/cpu.rs @@ -949,7 +949,7 @@ impl CpuManager { pub fn configure_vcpu( &self, - vcpu: Arc>, + vcpu: &Mutex, boot_setup: Option<(EntryPoint, &GuestMemoryAtomic)>, ) -> Result<()> { let mut vcpu = vcpu.lock().unwrap(); @@ -1432,7 +1432,7 @@ impl CpuManager { cmp::Ordering::Greater => { let vcpus = self.create_vcpus(desired_vcpus, None)?; for vcpu in vcpus { - self.configure_vcpu(vcpu, None)?; + self.configure_vcpu(&vcpu, None)?; } self.activate_vcpus(desired_vcpus, true, None)?; Ok(true) @@ -1543,6 +1543,7 @@ impl CpuManager { }) } + #[allow(clippy::needless_pass_by_value)] pub fn create_madt(&self, #[cfg(target_arch = "aarch64")] vgic: Arc>) -> Sdt { use crate::acpi; // This is also checked in the commandline parsing. diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 245be2c85..93d863c52 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -15,7 +15,7 @@ use std::io::{self, IsTerminal, Seek, SeekFrom, stdout}; use std::num::Wrapping; use std::os::unix::fs::OpenOptionsExt; use std::os::unix::io::{AsRawFd, FromRawFd}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::result; use std::sync::{Arc, Mutex}; #[cfg(not(target_arch = "riscv64"))] @@ -1114,7 +1114,7 @@ fn create_mmio_allocators( start: u64, end: u64, num_pci_segments: u16, - weights: Vec, + weights: &[u32], alignment: u64, ) -> Vec>> { let total_weight: u32 = weights.iter().sum(); @@ -1193,7 +1193,7 @@ impl DeviceManager { start_of_mmio32_area, end_of_mmio32_area, num_pci_segments, - mmio32_aperture_weights, + &mmio32_aperture_weights, 4 << 10, ); @@ -1213,7 +1213,7 @@ impl DeviceManager { start_of_mmio64_area, end_of_mmio64_area, num_pci_segments, - mmio64_aperture_weights, + &mmio64_aperture_weights, 4 << 30, ); @@ -1400,6 +1400,7 @@ impl DeviceManager { self.add_interrupt_controller() } + #[allow(clippy::needless_pass_by_value)] pub fn create_devices( &mut self, console_info: Option, @@ -1470,7 +1471,7 @@ impl DeviceManager { #[cfg(not(target_arch = "riscv64"))] if let Some(tpm) = self.config.clone().lock().unwrap().tpm.as_ref() { - let tpm_dev = self.add_tpm_device(tpm.socket.clone())?; + let tpm_dev = self.add_tpm_device(&tpm.socket)?; self.bus_devices .push(Arc::clone(&tpm_dev) as Arc); } @@ -1644,7 +1645,7 @@ impl DeviceManager { let dev_id = self.add_virtio_pci_device( handle.virtio_device, &mapping, - handle.id, + &handle.id, handle.pci_segment, handle.dma_handler, )?; @@ -1675,7 +1676,7 @@ impl DeviceManager { } if let Some(iommu_device) = iommu_device { - let dev_id = self.add_virtio_pci_device(iommu_device, &None, iommu_id, 0, None)?; + let dev_id = self.add_virtio_pci_device(iommu_device, &None, &iommu_id, 0, None)?; self.iommu_attached_devices = Some((dev_id, iommu_attached_devices)); } } @@ -1790,14 +1791,15 @@ impl DeviceManager { ) -> DeviceManagerResult>> { let id = String::from(IOAPIC_DEVICE_NAME); + let state = state_from_id(self.snapshot.as_ref(), id.as_str()) + .map_err(DeviceManagerError::RestoreGetState)?; // Create IOAPIC let interrupt_controller = Arc::new(Mutex::new( ioapic::Ioapic::new( id.clone(), APIC_START, self.msi_interrupt_manager.as_ref(), - state_from_id(self.snapshot.as_ref(), id.as_str()) - .map_err(DeviceManagerError::RestoreGetState)?, + state.as_ref(), ) .map_err(DeviceManagerError::CreateInterruptController)?, )); @@ -2486,7 +2488,7 @@ impl DeviceManager { #[cfg(not(target_arch = "riscv64"))] fn add_tpm_device( &mut self, - tpm_path: PathBuf, + tpm_path: &Path, ) -> DeviceManagerResult>> { // Create TPM Device let tpm = devices::tpm::Tpm::new(tpm_path.to_str().unwrap()).map_err(|e| { @@ -4057,7 +4059,7 @@ impl DeviceManager { &mut self, virtio_device: Arc>, iommu_mapping: &Option>, - virtio_device_id: String, + virtio_device_id: &str, pci_segment_id: u16, dma_handler: Option>, ) -> DeviceManagerResult { @@ -4065,13 +4067,13 @@ impl DeviceManager { // Add the new virtio-pci node to the device tree. let mut node = device_node!(id); - node.children = vec![virtio_device_id.clone()]; + node.children = vec![virtio_device_id.to_string()]; let (pci_segment_id, pci_device_bdf, resources) = self.pci_resources(&id, pci_segment_id)?; // Update the existing virtio node by setting the parent. - if let Some(node) = self.device_tree.lock().unwrap().get_mut(&virtio_device_id) { + if let Some(node) = self.device_tree.lock().unwrap().get_mut(virtio_device_id) { node.parent = Some(id.clone()); } else { return Err(DeviceManagerError::MissingNode); @@ -4472,15 +4474,15 @@ impl DeviceManager { }) } - pub fn remove_device(&mut self, id: String) -> DeviceManagerResult<()> { + pub fn remove_device(&mut self, id: &str) -> DeviceManagerResult<()> { // The node can be directly a PCI node in case the 'id' refers to a // VFIO device or a virtio-pci one. // In case the 'id' refers to a virtio device, we must find the PCI // node by looking at the parent. let device_tree = self.device_tree.lock().unwrap(); let node = device_tree - .get(&id) - .ok_or(DeviceManagerError::UnknownDeviceId(id.clone()))?; + .get(id) + .ok_or_else(|| DeviceManagerError::UnknownDeviceId(id.to_string()))?; // Release advisory locks by dropping all references. // Linux automatically releases all locks of that file if the last open FD is closed. @@ -4545,7 +4547,7 @@ impl DeviceManager { let nets = config.net.as_deref_mut().unwrap(); let net_dev_cfg = nets .iter_mut() - .find(|net| net.id.as_ref() == Some(&id)) + .find(|net| net.id.as_deref() == Some(id)) // unwrap: the device could not have been removed without an ID .unwrap(); let fds = net_dev_cfg.fds.take().unwrap_or(Vec::new()); @@ -4692,12 +4694,11 @@ impl DeviceManager { if remove_dma_handler { for virtio_mem_device in self.virtio_mem_devices.iter() { + let source = VirtioMemMappingSource::Device(pci_device_bdf.into()); virtio_mem_device .lock() .unwrap() - .remove_dma_mapping_handler(VirtioMemMappingSource::Device( - pci_device_bdf.into(), - )) + .remove_dma_mapping_handler(&source) .map_err(DeviceManagerError::RemoveDmaMappingHandlerVirtioMem)?; } } @@ -4804,7 +4805,7 @@ impl DeviceManager { let bdf = self.add_virtio_pci_device( handle.virtio_device, &mapping, - handle.id.clone(), + &handle.id, handle.pci_segment, handle.dma_handler, )?; @@ -5532,7 +5533,7 @@ mod unit_tests { #[test] fn test_create_mmio_allocators() { - let res = create_mmio_allocators(0x100000, 0x400000, 1, vec![1], 4 << 10); + let res = create_mmio_allocators(0x100000, 0x400000, 1, &[1], 4 << 10); assert_eq!(res.len(), 1); assert_eq!( res[0].lock().unwrap().base(), @@ -5543,7 +5544,7 @@ mod unit_tests { vm_memory::GuestAddress(0x3fffff) ); - let res = create_mmio_allocators(0x100000, 0x400000, 2, vec![1, 1], 4 << 10); + let res = create_mmio_allocators(0x100000, 0x400000, 2, &[1, 1], 4 << 10); assert_eq!(res.len(), 2); assert_eq!( res[0].lock().unwrap().base(), @@ -5562,7 +5563,7 @@ mod unit_tests { vm_memory::GuestAddress(0x3fffff) ); - let res = create_mmio_allocators(0x100000, 0x400000, 2, vec![2, 1], 4 << 10); + let res = create_mmio_allocators(0x100000, 0x400000, 2, &[2, 1], 4 << 10); assert_eq!(res.len(), 2); assert_eq!( res[0].lock().unwrap().base(), diff --git a/vmm/src/igvm/igvm_loader.rs b/vmm/src/igvm/igvm_loader.rs index e53dc698f..32aeb0f71 100644 --- a/vmm/src/igvm/igvm_loader.rs +++ b/vmm/src/igvm/igvm_loader.rs @@ -131,7 +131,7 @@ fn import_parameter( /// Right now it only supports SNP based isolation. /// We can boot legacy VM with an igvm file without /// any isolation. -/// +#[allow(clippy::needless_pass_by_value)] pub fn load_igvm( mut file: &std::fs::File, memory_manager: Arc>, diff --git a/vmm/src/igvm/mod.rs b/vmm/src/igvm/mod.rs index 06686b7a0..62c32d4e8 100644 --- a/vmm/src/igvm/mod.rs +++ b/vmm/src/igvm/mod.rs @@ -74,7 +74,7 @@ pub enum BootPageAcceptance { /// The startup memory type used to notify a well behaved host that memory should be present before attempting to /// start the guest. #[allow(dead_code)] -#[derive(Debug, PartialEq, Eq)] +#[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum StartupMemoryType { /// The range is normal memory. Ram, diff --git a/vmm/src/lib.rs b/vmm/src/lib.rs index 1c1eca79b..84db6a14c 100644 --- a/vmm/src/lib.rs +++ b/vmm/src/lib.rs @@ -11,7 +11,6 @@ use std::os::unix::io::{AsRawFd, FromRawFd, RawFd}; use std::os::unix::net::{UnixListener, UnixStream}; use std::panic::AssertUnwindSafe; use std::path::PathBuf; -use std::rc::Rc; use std::sync::mpsc::{Receiver, RecvError, SendError, Sender}; use std::sync::{Arc, Mutex}; #[cfg(not(target_arch = "riscv64"))] @@ -545,9 +544,9 @@ pub fn start_vmm_thread( vmm.setup_signal_handler(landlock_enable)?; vmm.control_loop( - Rc::new(api_receiver), + &api_receiver, #[cfg(feature = "guest_debug")] - Rc::new(gdb_receiver), + &gdb_receiver, ) }) .map_err(Error::VmmThreadSpawn)? @@ -674,7 +673,7 @@ impl Vmm { fn signal_handler( mut signals: Signals, - original_termios_opt: Arc>>, + original_termios_opt: &Mutex>, exit_evt: &EventFd, ) { for sig in &Self::HANDLED_SIGNALS { @@ -747,7 +746,7 @@ impl Vmm { } std::panic::catch_unwind(AssertUnwindSafe(|| { - Vmm::signal_handler(signals, original_termios_opt, &exit_evt); + Vmm::signal_handler(signals, original_termios_opt.as_ref(), &exit_evt); })) .map_err(|_| { error!("vmm signal_handler thread panicked"); @@ -862,7 +861,7 @@ impl Vmm { .unwrap() .landlock_enable { - apply_landlock(self.vm_config.as_ref().unwrap().clone()).map_err(|e| { + apply_landlock(self.vm_config.as_ref().unwrap().as_ref()).map_err(|e| { MigratableError::MigrateReceive(anyhow!("Error applying landlock: {e:?}")) })?; } @@ -1097,12 +1096,13 @@ impl Vmm { Ok(true) } + #[allow(clippy::needless_pass_by_value)] fn send_migration( vm: &mut Vm, #[cfg(all(feature = "kvm", target_arch = "x86_64"))] hypervisor: Arc< dyn hypervisor::Hypervisor, >, - send_data_migration: VmSendMigrationData, + send_data_migration: &VmSendMigrationData, ) -> result::Result<(), MigratableError> { // Set up the socket connection let mut socket = Self::send_migration_socket(&send_data_migration.destination_url)?; @@ -1348,7 +1348,7 @@ impl Vmm { .unwrap() .landlock_enable { - apply_landlock(self.vm_config.as_ref().unwrap().clone()) + apply_landlock(self.vm_config.as_ref().unwrap().as_ref()) .map_err(VmError::ApplyLandlock)?; } @@ -1362,8 +1362,8 @@ impl Vmm { fn control_loop( &mut self, - api_receiver: Rc>, - #[cfg(feature = "guest_debug")] gdb_receiver: Rc>, + api_receiver: &Receiver, + #[cfg(feature = "guest_debug")] gdb_receiver: &Receiver, ) -> Result<()> { const EPOLL_EVENTS_LEN: usize = 100; @@ -1468,7 +1468,7 @@ impl Vmm { } } -fn apply_landlock(vm_config: Arc>) -> result::Result<(), LandlockError> { +fn apply_landlock(vm_config: &Mutex) -> result::Result<(), LandlockError> { vm_config.lock().unwrap().apply_landlock()?; Ok(()) } @@ -1490,7 +1490,7 @@ impl RequestHandler for Vmm { .unwrap() .landlock_enable { - apply_landlock(self.vm_config.as_ref().unwrap().clone()) + apply_landlock(self.vm_config.as_ref().unwrap().as_ref()) .map_err(VmError::ApplyLandlock)?; } Ok(()) @@ -1834,7 +1834,7 @@ impl RequestHandler for Vmm { self.vm_config.as_ref().ok_or(VmError::VmNotCreated)?; if let Some(ref mut vm) = self.vm { - vm.resize_zone(id, desired_ram) + vm.resize_zone(&id, desired_ram) .inspect_err(|e| error!("Error when resizing zone: {e:?}"))?; Ok(()) } else { @@ -1913,7 +1913,7 @@ impl RequestHandler for Vmm { fn vm_remove_device(&mut self, id: String) -> result::Result<(), VmError> { if let Some(ref mut vm) = self.vm { - vm.remove_device(id) + vm.remove_device(&id) .inspect_err(|e| error!("Error when removing device from the VM: {e:?}"))?; Ok(()) } else if let Some(ref config) = self.vm_config { @@ -2271,7 +2271,7 @@ impl RequestHandler for Vmm { vm, #[cfg(all(feature = "kvm", target_arch = "x86_64"))] self.hypervisor.clone(), - send_data_migration.clone(), + &send_data_migration, ) .map_err(|migration_err| { error!("Migration failed: {migration_err:?}"); diff --git a/vmm/src/memory_manager.rs b/vmm/src/memory_manager.rs index 8f4ff649b..24eec5d41 100644 --- a/vmm/src/memory_manager.rs +++ b/vmm/src/memory_manager.rs @@ -721,7 +721,7 @@ impl MemoryManager { fn fill_saved_regions( &mut self, file_path: PathBuf, - saved_regions: MemoryRangeTable, + saved_regions: &MemoryRangeTable, ) -> Result<(), Error> { if saved_regions.is_empty() { return Ok(()); @@ -1268,7 +1268,7 @@ impl MemoryManager { mm.lock() .unwrap() - .fill_saved_regions(memory_file_path, mem_snapshot.memory_ranges)?; + .fill_saved_regions(memory_file_path, &mem_snapshot.memory_ranges)?; Ok(mm) } else { @@ -1291,7 +1291,7 @@ impl MemoryManager { addr: *mut u8, len: u64, mode: u32, - nodemask: Vec, + nodemask: &[u64], maxnode: u64, flags: u32, ) -> Result<(), io::Error> { @@ -1438,7 +1438,7 @@ impl MemoryManager { // MPOL_BIND is the selected mode as it specifies a strict policy // that restricts memory allocation to the nodes specified in the // nodemask. - Self::mbind(addr, len, mode, nodemask, maxnode, flags) + Self::mbind(addr, len, mode, &nodemask, maxnode, flags) .map_err(Error::ApplyNumaPolicy)?; } diff --git a/vmm/src/seccomp_filters.rs b/vmm/src/seccomp_filters.rs index c4a166380..ba9f1f455 100644 --- a/vmm/src/seccomp_filters.rs +++ b/vmm/src/seccomp_filters.rs @@ -25,6 +25,7 @@ use vhost::vhost_kern::vhost_binding::{ VHOST_VDPA_SET_STATUS, VHOST_VDPA_SET_VRING_ENABLE, VHOST_VDPA_SUSPEND, }; +#[derive(Copy, Clone)] pub enum Thread { HttpApi, #[cfg(feature = "dbus_api")] diff --git a/vmm/src/sigwinch_listener.rs b/vmm/src/sigwinch_listener.rs index 970e82fcf..4cbfeddf5 100644 --- a/vmm/src/sigwinch_listener.rs +++ b/vmm/src/sigwinch_listener.rs @@ -19,7 +19,7 @@ use libc::{ poll, pollfd, setsid, sigemptyset, siginfo_t, signal, sigprocmask, syscall, tcgetpgrp, tcsetpgrp, }; -use seccompiler::{BpfProgram, SeccompAction, apply_filter}; +use seccompiler::{BpfProgramRef, SeccompAction, apply_filter}; use vmm_sys_util::signal::register_signal_handler; use crate::clone3::{CLONE_CLEAR_SIGHAND, clone_args, clone3}; @@ -162,7 +162,7 @@ fn set_foreground_process_group(tty: &File) -> io::Result<()> { Ok(()) } -fn sigwinch_listener_main(seccomp_filter: BpfProgram, tx: File, tty: File) -> ! { +fn sigwinch_listener_main(seccomp_filter: BpfProgramRef, tx: File, tty: File) -> ! { // SAFETY: any references to these file descriptors are // unreachable, because this function never returns. unsafe { @@ -174,7 +174,7 @@ fn sigwinch_listener_main(seccomp_filter: BpfProgram, tx: File, tty: File) -> ! unblock_all_signals().unwrap(); if !seccomp_filter.is_empty() { - apply_filter(&seccomp_filter).unwrap(); + apply_filter(seccomp_filter).unwrap(); } register_signal_handler(SIGWINCH, sigwinch_handler).unwrap(); @@ -242,7 +242,7 @@ unsafe fn clone_clear_sighand() -> io::Result { Ok(r.try_into().unwrap()) } -pub fn start_sigwinch_listener(seccomp_filter: BpfProgram, tty_sub: File) -> io::Result { +pub fn start_sigwinch_listener(seccomp_filter: BpfProgramRef, tty_sub: File) -> io::Result { let mut pipe = [-1; 2]; // SAFETY: FFI call with valid arguments if unsafe { pipe2(pipe.as_mut_ptr(), O_CLOEXEC) } == -1 { @@ -275,7 +275,7 @@ pub fn listen_for_sigwinch_on_tty( let seccomp_filter = get_seccomp_filter(seccomp_action, Thread::PtyForeground, hypervisor_type).unwrap(); - let console_resize_pipe = start_sigwinch_listener(seccomp_filter, pty_sub)?; + let console_resize_pipe = start_sigwinch_listener(&seccomp_filter, pty_sub)?; Ok(console_resize_pipe) } diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index fcd6fbfbe..ec455c6ba 100644 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -528,6 +528,7 @@ pub struct Vm { impl Vm { pub const HANDLED_SIGNALS: [i32; 1] = [SIGWINCH]; + #[allow(clippy::needless_pass_by_value)] #[allow(clippy::too_many_arguments)] pub fn new_from_memory_manager( config: Arc>, @@ -557,7 +558,7 @@ impl Vm { // Create NUMA nodes based on NumaConfig. let numa_nodes = - Self::create_numa_nodes(config.lock().unwrap().numa.clone(), &memory_manager)?; + Self::create_numa_nodes(config.lock().unwrap().numa.as_deref(), &memory_manager)?; #[cfg(feature = "tdx")] let tdx_enabled = config.lock().unwrap().is_tdx_enabled(); @@ -915,7 +916,7 @@ impl Vm { } fn create_numa_nodes( - configs: Option>, + configs: Option<&[NumaConfig]>, memory_manager: &Arc>, ) -> Result { let mm = memory_manager.lock().unwrap(); @@ -1148,6 +1149,7 @@ impl Vm { Ok(cmdline) } + #[allow(clippy::needless_pass_by_value)] #[cfg(any(target_arch = "aarch64", target_arch = "riscv64"))] fn load_firmware( mut firmware: &File, @@ -1162,6 +1164,7 @@ impl Vm { }) } + #[allow(clippy::needless_pass_by_value)] #[cfg(any(target_arch = "aarch64", target_arch = "riscv64"))] fn load_kernel( mut kernel: File, @@ -1197,6 +1200,7 @@ impl Vm { } #[cfg(feature = "igvm")] + #[allow(clippy::needless_pass_by_value)] fn load_igvm( igvm: File, memory_manager: Arc>, @@ -1231,6 +1235,7 @@ impl Vm { /// /// For x86_64, the boot path is the same. #[cfg(target_arch = "x86_64")] + #[allow(clippy::needless_pass_by_value)] fn load_kernel( mut kernel: File, cmdline: Option, @@ -1324,6 +1329,7 @@ impl Vm { } } + #[allow(clippy::needless_pass_by_value)] #[cfg(any(target_arch = "aarch64", target_arch = "riscv64"))] fn load_payload( payload: &PayloadConfig, @@ -1521,7 +1527,7 @@ impl Vm { arch::configure_system( &mem, cmdline.as_cstring().unwrap().to_str().unwrap(), - vcpu_mpidrs, + &vcpu_mpidrs, vcpu_topology, device_info, &initramfs_config, @@ -1722,7 +1728,7 @@ impl Vm { Ok(()) } - pub fn resize_zone(&mut self, id: String, desired_memory: u64) -> Result<()> { + pub fn resize_zone(&mut self, id: &str, desired_memory: u64) -> Result<()> { let memory_config = &mut self.config.lock().unwrap().memory; if let Some(zones) = &mut memory_config.zones { @@ -1733,7 +1739,7 @@ impl Vm { self.memory_manager .lock() .unwrap() - .resize_zone(&id, desired_memory - zone.size) + .resize_zone(id, desired_memory - zone.size) .map_err(Error::MemoryManager)?; // We update the memory zone config regardless of the // actual 'resize-zone' operation result (happened or @@ -1805,16 +1811,16 @@ impl Vm { Ok(pci_device_info) } - pub fn remove_device(&mut self, id: String) -> Result<()> { + pub fn remove_device(&mut self, id: &str) -> Result<()> { self.device_manager .lock() .unwrap() - .remove_device(id.clone()) + .remove_device(id) .map_err(Error::DeviceManager)?; // Update VmConfig by removing the device. This is important to // ensure the device would not be created in case of a reboot. - self.config.lock().unwrap().remove_device(&id); + self.config.lock().unwrap().remove_device(id); self.device_manager .lock() @@ -2409,7 +2415,7 @@ impl Vm { self.cpu_manager .lock() .unwrap() - .configure_vcpu(vcpu.clone(), boot_setup) + .configure_vcpu(&vcpu, boot_setup) .map_err(Error::CpuManager)?; #[cfg(target_arch = "aarch64")] @@ -3562,13 +3568,12 @@ mod unit_tests { let hv = hypervisor::new().unwrap(); let vm = hv.create_vm(HypervisorVmConfig::default()).unwrap(); - let gic = vm - .create_vgic(Gic::create_default_config(1)) - .expect("Cannot create gic"); + let vgic_config = Gic::create_default_config(1); + let gic = vm.create_vgic(&vgic_config).expect("Cannot create gic"); create_fdt( &mem, "console=tty0", - vec![0], + &[0], Some((0, 0, 0, 0)), &dev_info, &gic,