From 81ab148b4f9af6c2363d19f224c49c62a05a8b56 Mon Sep 17 00:00:00 2001 From: Egor Lazarchuk Date: Mon, 25 Nov 2024 17:56:13 +0000 Subject: [PATCH] chore(clippy): update code with new lints With a new Rust version, new Clippy entered our repository. This commit aligns our codebase with guidance provided by new lints. Signed-off-by: Egor Lazarchuk --- Cargo.toml | 1 + .../src/api_server/request/actions.rs | 3 +- .../src/api_server/request/boot_source.rs | 3 +- .../src/api_server/request/drive.rs | 6 +- .../src/api_server/request/logger.rs | 3 +- .../request/machine_configuration.rs | 6 +- .../src/api_server/request/metrics.rs | 3 +- .../src/api_server/request/mmds.rs | 9 +-- src/firecracker/src/api_server/request/net.rs | 6 +- .../src/api_server/request/vsock.rs | 3 +- src/vmm/src/arch/aarch64/cache_info.rs | 61 +++++++++---------- src/vmm/src/devices/legacy/serial.rs | 2 +- src/vmm/src/devices/virtio/balloon/metrics.rs | 2 +- .../devices/virtio/block/virtio/metrics.rs | 3 +- src/vmm/src/devices/virtio/iovec.rs | 12 ++-- src/vmm/src/devices/virtio/mmio.rs | 4 +- src/vmm/src/devices/virtio/net/device.rs | 3 +- src/vmm/src/devices/virtio/net/metrics.rs | 3 +- src/vmm/src/devices/virtio/queue.rs | 13 ++-- src/vmm/src/devices/virtio/rng/device.rs | 3 +- src/vmm/src/devices/virtio/rng/metrics.rs | 2 +- .../src/devices/virtio/vhost_user_metrics.rs | 3 +- .../src/devices/virtio/vsock/event_handler.rs | 2 +- src/vmm/src/devices/virtio/vsock/metrics.rs | 2 +- src/vmm/src/devices/virtio/vsock/packet.rs | 1 + .../src/devices/virtio/vsock/unix/muxer.rs | 11 ++-- .../devices/virtio/vsock/unix/muxer_rxq.rs | 1 + src/vmm/src/dumbo/pdu/mod.rs | 3 +- src/vmm/src/dumbo/tcp/endpoint.rs | 3 +- src/vmm/src/io_uring/mod.rs | 10 ++- src/vmm/src/lib.rs | 9 +-- src/vmm/src/logger/metrics.rs | 6 +- src/vmm/src/rate_limiter/mod.rs | 14 ++--- src/vmm/src/resources.rs | 4 +- src/vmm/src/rpc_interface.rs | 6 +- src/vmm/src/vmm_config/boot_source.rs | 2 +- 36 files changed, 103 insertions(+), 125 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 914c4cdc87cb..4f8cc3f5eb50 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,6 +9,7 @@ resolver = "2" [workspace.lints.rust] missing_debug_implementations = "warn" +unexpected_cfgs = { level = "warn", check-cfg = ['cfg(kani)'] } [workspace.lints.clippy] ptr_as_ptr = "warn" diff --git a/src/firecracker/src/api_server/request/actions.rs b/src/firecracker/src/api_server/request/actions.rs index 7d7974571a98..a3b3f3f3a884 100644 --- a/src/firecracker/src/api_server/request/actions.rs +++ b/src/firecracker/src/api_server/request/actions.rs @@ -30,9 +30,8 @@ struct ActionBody { pub(crate) fn parse_put_actions(body: &Body) -> Result { METRICS.put_api_requests.actions_count.inc(); - let action_body = serde_json::from_slice::(body.raw()).map_err(|err| { + let action_body = serde_json::from_slice::(body.raw()).inspect_err(|_| { METRICS.put_api_requests.actions_fails.inc(); - err })?; match action_body.action_type { diff --git a/src/firecracker/src/api_server/request/boot_source.rs b/src/firecracker/src/api_server/request/boot_source.rs index 16f3c1b54992..10e161484617 100644 --- a/src/firecracker/src/api_server/request/boot_source.rs +++ b/src/firecracker/src/api_server/request/boot_source.rs @@ -11,9 +11,8 @@ use super::Body; pub(crate) fn parse_put_boot_source(body: &Body) -> Result { METRICS.put_api_requests.boot_source_count.inc(); Ok(ParsedRequest::new_sync(VmmAction::ConfigureBootSource( - serde_json::from_slice::(body.raw()).map_err(|err| { + serde_json::from_slice::(body.raw()).inspect_err(|_| { METRICS.put_api_requests.boot_source_fails.inc(); - err })?, ))) } diff --git a/src/firecracker/src/api_server/request/drive.rs b/src/firecracker/src/api_server/request/drive.rs index a74d3be69ffc..4b2548b53c8b 100644 --- a/src/firecracker/src/api_server/request/drive.rs +++ b/src/firecracker/src/api_server/request/drive.rs @@ -20,9 +20,8 @@ pub(crate) fn parse_put_drive( return Err(RequestError::EmptyID); }; - let device_cfg = serde_json::from_slice::(body.raw()).map_err(|err| { + let device_cfg = serde_json::from_slice::(body.raw()).inspect_err(|_| { METRICS.put_api_requests.drive_fails.inc(); - err })?; if id != device_cfg.drive_id { @@ -51,9 +50,8 @@ pub(crate) fn parse_patch_drive( }; let block_device_update_cfg: BlockDeviceUpdateConfig = - serde_json::from_slice::(body.raw()).map_err(|err| { + serde_json::from_slice::(body.raw()).inspect_err(|_| { METRICS.patch_api_requests.drive_fails.inc(); - err })?; if id != block_device_update_cfg.drive_id { diff --git a/src/firecracker/src/api_server/request/logger.rs b/src/firecracker/src/api_server/request/logger.rs index 6355bf48beb5..cda125ac71cf 100644 --- a/src/firecracker/src/api_server/request/logger.rs +++ b/src/firecracker/src/api_server/request/logger.rs @@ -10,9 +10,8 @@ use super::Body; pub(crate) fn parse_put_logger(body: &Body) -> Result { METRICS.put_api_requests.logger_count.inc(); let res = serde_json::from_slice::(body.raw()); - let config = res.map_err(|err| { + let config = res.inspect_err(|_| { METRICS.put_api_requests.logger_fails.inc(); - err })?; Ok(ParsedRequest::new_sync(VmmAction::ConfigureLogger(config))) } diff --git a/src/firecracker/src/api_server/request/machine_configuration.rs b/src/firecracker/src/api_server/request/machine_configuration.rs index 344d9095b776..871bbda5ecc6 100644 --- a/src/firecracker/src/api_server/request/machine_configuration.rs +++ b/src/firecracker/src/api_server/request/machine_configuration.rs @@ -15,9 +15,8 @@ pub(crate) fn parse_get_machine_config() -> Result pub(crate) fn parse_put_machine_config(body: &Body) -> Result { METRICS.put_api_requests.machine_cfg_count.inc(); - let config = serde_json::from_slice::(body.raw()).map_err(|err| { + let config = serde_json::from_slice::(body.raw()).inspect_err(|_| { METRICS.put_api_requests.machine_cfg_fails.inc(); - err })?; // Check for the presence of deprecated `cpu_template` field. @@ -44,9 +43,8 @@ pub(crate) fn parse_put_machine_config(body: &Body) -> Result Result { METRICS.patch_api_requests.machine_cfg_count.inc(); let config_update = - serde_json::from_slice::(body.raw()).map_err(|err| { + serde_json::from_slice::(body.raw()).inspect_err(|_| { METRICS.patch_api_requests.machine_cfg_fails.inc(); - err })?; if config_update.is_empty() { diff --git a/src/firecracker/src/api_server/request/metrics.rs b/src/firecracker/src/api_server/request/metrics.rs index e6b361a49c1f..054ece194220 100644 --- a/src/firecracker/src/api_server/request/metrics.rs +++ b/src/firecracker/src/api_server/request/metrics.rs @@ -11,9 +11,8 @@ use super::Body; pub(crate) fn parse_put_metrics(body: &Body) -> Result { METRICS.put_api_requests.metrics_count.inc(); Ok(ParsedRequest::new_sync(VmmAction::ConfigureMetrics( - serde_json::from_slice::(body.raw()).map_err(|err| { + serde_json::from_slice::(body.raw()).inspect_err(|_| { METRICS.put_api_requests.metrics_fails.inc(); - err })?, ))) } diff --git a/src/firecracker/src/api_server/request/mmds.rs b/src/firecracker/src/api_server/request/mmds.rs index ced0a671e9ba..2bc96512d3cc 100644 --- a/src/firecracker/src/api_server/request/mmds.rs +++ b/src/firecracker/src/api_server/request/mmds.rs @@ -16,9 +16,8 @@ pub(crate) fn parse_get_mmds() -> Result { } fn parse_put_mmds_config(body: &Body) -> Result { - let config: MmdsConfig = serde_json::from_slice(body.raw()).map_err(|err| { + let config: MmdsConfig = serde_json::from_slice(body.raw()).inspect_err(|_| { METRICS.put_api_requests.mmds_fails.inc(); - err })?; // Construct the `ParsedRequest` object. let version = config.version; @@ -42,9 +41,8 @@ pub(crate) fn parse_put_mmds( METRICS.put_api_requests.mmds_count.inc(); match path_second_token { None => Ok(ParsedRequest::new_sync(VmmAction::PutMMDS( - serde_json::from_slice(body.raw()).map_err(|err| { + serde_json::from_slice(body.raw()).inspect_err(|_| { METRICS.put_api_requests.mmds_fails.inc(); - err })?, ))), Some("config") => parse_put_mmds_config(body), @@ -61,9 +59,8 @@ pub(crate) fn parse_put_mmds( pub(crate) fn parse_patch_mmds(body: &Body) -> Result { METRICS.patch_api_requests.mmds_count.inc(); Ok(ParsedRequest::new_sync(VmmAction::PatchMMDS( - serde_json::from_slice(body.raw()).map_err(|err| { + serde_json::from_slice(body.raw()).inspect_err(|_| { METRICS.patch_api_requests.mmds_fails.inc(); - err })?, ))) } diff --git a/src/firecracker/src/api_server/request/net.rs b/src/firecracker/src/api_server/request/net.rs index 0ab5377b0a1e..5fced98635c1 100644 --- a/src/firecracker/src/api_server/request/net.rs +++ b/src/firecracker/src/api_server/request/net.rs @@ -20,9 +20,8 @@ pub(crate) fn parse_put_net( return Err(RequestError::EmptyID); }; - let netif = serde_json::from_slice::(body.raw()).map_err(|err| { + let netif = serde_json::from_slice::(body.raw()).inspect_err(|_| { METRICS.put_api_requests.network_fails.inc(); - err })?; if id != netif.iface_id.as_str() { METRICS.put_api_requests.network_fails.inc(); @@ -53,9 +52,8 @@ pub(crate) fn parse_patch_net( }; let netif = - serde_json::from_slice::(body.raw()).map_err(|err| { + serde_json::from_slice::(body.raw()).inspect_err(|_| { METRICS.patch_api_requests.network_fails.inc(); - err })?; if id != netif.iface_id { METRICS.patch_api_requests.network_count.inc(); diff --git a/src/firecracker/src/api_server/request/vsock.rs b/src/firecracker/src/api_server/request/vsock.rs index 67bf7b0a9851..acf129d456ca 100644 --- a/src/firecracker/src/api_server/request/vsock.rs +++ b/src/firecracker/src/api_server/request/vsock.rs @@ -10,9 +10,8 @@ use super::Body; pub(crate) fn parse_put_vsock(body: &Body) -> Result { METRICS.put_api_requests.vsock_count.inc(); - let vsock_cfg = serde_json::from_slice::(body.raw()).map_err(|err| { + let vsock_cfg = serde_json::from_slice::(body.raw()).inspect_err(|_| { METRICS.put_api_requests.vsock_fails.inc(); - err })?; // Check for the presence of deprecated `vsock_id` field. diff --git a/src/vmm/src/arch/aarch64/cache_info.rs b/src/vmm/src/arch/aarch64/cache_info.rs index cd61cabeb027..f94fc8f7822c 100644 --- a/src/vmm/src/arch/aarch64/cache_info.rs +++ b/src/vmm/src/arch/aarch64/cache_info.rs @@ -23,8 +23,8 @@ pub(crate) enum CacheInfoError { MissingOptionalAttr(String, CacheEntry), } -struct CacheEngine { - store: Box, +struct CacheEngine { + store: T, } trait CacheStore: std::fmt::Debug { @@ -32,7 +32,7 @@ trait CacheStore: std::fmt::Debug { } #[derive(Debug)] -pub(crate) struct CacheEntry { +pub struct CacheEntry { // Cache Level: 1, 2, 3.. pub level: u8, // Type of cache: Unified, Data, Instruction. @@ -45,17 +45,16 @@ pub(crate) struct CacheEntry { } #[derive(Debug)] -struct HostCacheStore { +pub struct HostCacheStore { cache_dir: PathBuf, } -#[cfg(not(test))] -impl Default for CacheEngine { +impl Default for CacheEngine { fn default() -> Self { CacheEngine { - store: Box::new(HostCacheStore { + store: HostCacheStore { cache_dir: PathBuf::from("/sys/devices/system/cpu/cpu0/cache"), - }), + }, } } } @@ -72,7 +71,7 @@ impl CacheStore for HostCacheStore { } impl CacheEntry { - fn from_index(index: u8, store: &dyn CacheStore) -> Result { + fn from_index(index: u8, store: &impl CacheStore) -> Result { let mut err_str = String::new(); let mut cache: CacheEntry = CacheEntry::default(); @@ -287,10 +286,10 @@ pub(crate) fn read_cache_config( // Also without this mechanism we would be logging the warnings for each level which pollutes // a lot the logs. let mut logged_missing_attr = false; - let engine = CacheEngine::default(); + let engine = CacheEngine::::default(); for index in 0..=MAX_CACHE_LEVEL { - match CacheEntry::from_index(index, engine.store.as_ref()) { + match CacheEntry::from_index(index, &engine.store) { Ok(cache) => { append_cache_level(cache_l1, cache_non_l1, cache); } @@ -326,22 +325,22 @@ mod tests { dummy_fs: HashMap, } - impl Default for CacheEngine { + impl Default for CacheEngine { fn default() -> Self { CacheEngine { - store: Box::new(MockCacheStore { + store: MockCacheStore { dummy_fs: create_default_store(), - }), + }, } } } - impl CacheEngine { + impl CacheEngine { fn new(map: &HashMap) -> Self { CacheEngine { - store: Box::new(MockCacheStore { + store: MockCacheStore { dummy_fs: map.clone(), - }), + }, } } } @@ -425,12 +424,12 @@ mod tests { let mut map1 = default_map.clone(); map1.remove("index0/type"); let engine = CacheEngine::new(&map1); - let res = CacheEntry::from_index(0, engine.store.as_ref()); + let res = CacheEntry::from_index(0, &engine.store); // We did create the level file but we still do not have the type file. assert!(matches!(res.unwrap_err(), CacheInfoError::MissingCacheType)); let engine = CacheEngine::new(&default_map); - let res = CacheEntry::from_index(0, engine.store.as_ref()); + let res = CacheEntry::from_index(0, &engine.store); assert_eq!( format!("{}", res.unwrap_err()), "shared cpu map, coherency line size, size, number of sets", @@ -440,7 +439,7 @@ mod tests { let mut map2 = default_map.clone(); map2.insert("index0/level".to_string(), "d".to_string()); let engine = CacheEngine::new(&map2); - let res = CacheEntry::from_index(0, engine.store.as_ref()); + let res = CacheEntry::from_index(0, &engine.store); assert_eq!( format!("{}", res.unwrap_err()), "Invalid cache configuration found for level: invalid digit found in string" @@ -448,7 +447,7 @@ mod tests { default_map.insert("index0/type".to_string(), "Instructionn".to_string()); let engine = CacheEngine::new(&default_map); - let res = CacheEntry::from_index(0, engine.store.as_ref()); + let res = CacheEntry::from_index(0, &engine.store); assert_eq!( format!("{}", res.unwrap_err()), "Invalid cache configuration found for type: Instructionn" @@ -464,7 +463,7 @@ mod tests { "00000000,00000001".to_string(), ); let engine = CacheEngine::new(&default_map); - let res = CacheEntry::from_index(0, engine.store.as_ref()); + let res = CacheEntry::from_index(0, &engine.store); assert_eq!( format!("{}", res.unwrap_err()), "coherency line size, size, number of sets" @@ -475,7 +474,7 @@ mod tests { "00000000,0000000G".to_string(), ); let engine = CacheEngine::new(&default_map); - let res = CacheEntry::from_index(0, engine.store.as_ref()); + let res = CacheEntry::from_index(0, &engine.store); assert_eq!( format!("{}", res.unwrap_err()), "Invalid cache configuration found for shared_cpu_map: invalid digit found in string" @@ -483,7 +482,7 @@ mod tests { default_map.insert("index0/shared_cpu_map".to_string(), "00000000".to_string()); let engine = CacheEngine::new(&default_map); - let res = CacheEntry::from_index(0, engine.store.as_ref()); + let res = CacheEntry::from_index(0, &engine.store); assert_eq!( format!("{}", res.unwrap_err()), "Invalid cache configuration found for shared_cpu_map: 00000000" @@ -496,7 +495,7 @@ mod tests { default_map.insert("index0/coherency_line_size".to_string(), "64".to_string()); let engine = CacheEngine::new(&default_map); - let res = CacheEntry::from_index(0, engine.store.as_ref()); + let res = CacheEntry::from_index(0, &engine.store); assert_eq!( "shared cpu map, size, number of sets", format!("{}", res.unwrap_err()) @@ -507,7 +506,7 @@ mod tests { "Instruction".to_string(), ); let engine = CacheEngine::new(&default_map); - let res = CacheEntry::from_index(0, engine.store.as_ref()); + let res = CacheEntry::from_index(0, &engine.store); assert_eq!( format!("{}", res.unwrap_err()), "Invalid cache configuration found for coherency_line_size: invalid digit found in \ @@ -521,7 +520,7 @@ mod tests { default_map.insert("index0/size".to_string(), "64K".to_string()); let engine = CacheEngine::new(&default_map); - let res = CacheEntry::from_index(0, engine.store.as_ref()); + let res = CacheEntry::from_index(0, &engine.store); assert_eq!( format!("{}", res.unwrap_err()), "shared cpu map, coherency line size, number of sets", @@ -529,7 +528,7 @@ mod tests { default_map.insert("index0/size".to_string(), "64".to_string()); let engine = CacheEngine::new(&default_map); - let res = CacheEntry::from_index(0, engine.store.as_ref()); + let res = CacheEntry::from_index(0, &engine.store); assert_eq!( format!("{}", res.unwrap_err()), "Invalid cache configuration found for size: 64" @@ -537,7 +536,7 @@ mod tests { default_map.insert("index0/size".to_string(), "64Z".to_string()); let engine = CacheEngine::new(&default_map); - let res = CacheEntry::from_index(0, engine.store.as_ref()); + let res = CacheEntry::from_index(0, &engine.store); assert_eq!( format!("{}", res.unwrap_err()), "Invalid cache configuration found for size: 64Z" @@ -550,7 +549,7 @@ mod tests { default_map.insert("index0/number_of_sets".to_string(), "64".to_string()); let engine = CacheEngine::new(&default_map); - let res = CacheEntry::from_index(0, engine.store.as_ref()); + let res = CacheEntry::from_index(0, &engine.store); assert_eq!( "shared cpu map, coherency line size, size", format!("{}", res.unwrap_err()) @@ -558,7 +557,7 @@ mod tests { default_map.insert("index0/number_of_sets".to_string(), "64K".to_string()); let engine = CacheEngine::new(&default_map); - let res = CacheEntry::from_index(0, engine.store.as_ref()); + let res = CacheEntry::from_index(0, &engine.store); assert_eq!( format!("{}", res.unwrap_err()), "Invalid cache configuration found for number_of_sets: invalid digit found in string" diff --git a/src/vmm/src/devices/legacy/serial.rs b/src/vmm/src/devices/legacy/serial.rs index cd74159dbdcd..278c15a4464d 100644 --- a/src/vmm/src/devices/legacy/serial.rs +++ b/src/vmm/src/devices/legacy/serial.rs @@ -386,7 +386,7 @@ mod tests { ), input: None::, }; - serial.serial.raw_input(&[b'a', b'b', b'c']).unwrap(); + serial.serial.raw_input(b"abc").unwrap(); let invalid_reads_before = metrics.missed_read_count.count(); let mut v = [0x00; 2]; diff --git a/src/vmm/src/devices/virtio/balloon/metrics.rs b/src/vmm/src/devices/virtio/balloon/metrics.rs index da8f7a285208..0b438cae2d4e 100644 --- a/src/vmm/src/devices/virtio/balloon/metrics.rs +++ b/src/vmm/src/devices/virtio/balloon/metrics.rs @@ -31,7 +31,7 @@ //! //! The system implements 1 type of metrics: //! * Shared Incremental Metrics (SharedIncMetrics) - dedicated for the metrics which need a counter -//! (i.e the number of times an API request failed). These metrics are reset upon flush. +//! (i.e the number of times an API request failed). These metrics are reset upon flush. use serde::ser::SerializeMap; use serde::{Serialize, Serializer}; diff --git a/src/vmm/src/devices/virtio/block/virtio/metrics.rs b/src/vmm/src/devices/virtio/block/virtio/metrics.rs index 69a521fde54c..0abe2a589637 100644 --- a/src/vmm/src/devices/virtio/block/virtio/metrics.rs +++ b/src/vmm/src/devices/virtio/block/virtio/metrics.rs @@ -72,7 +72,8 @@ //! //! The system implements 1 type of metrics: //! * Shared Incremental Metrics (SharedIncMetrics) - dedicated for the metrics which need a counter -//! (i.e the number of times an API request failed). These metrics are reset upon flush. +//! (i.e the number of times an API request failed). These metrics are reset upon flush. +//! //! We add BlockDeviceMetrics entries from block::metrics::METRICS into Block device instead of //! Block device having individual separate BlockDeviceMetrics entries because Block device is not //! accessible from signal handlers to flush metrics and block::metrics::METRICS is. diff --git a/src/vmm/src/devices/virtio/iovec.rs b/src/vmm/src/devices/virtio/iovec.rs index 2161d7273ef5..9262dff661bc 100644 --- a/src/vmm/src/devices/virtio/iovec.rs +++ b/src/vmm/src/devices/virtio/iovec.rs @@ -264,10 +264,11 @@ impl IoVecBufferMut { // We use get_slice instead of `get_host_address` here in order to have the whole // range of the descriptor chain checked, i.e. [addr, addr + len) is a valid memory // region in the GuestMemoryMmap. - let slice = mem.get_slice(desc.addr, desc.len as usize).map_err(|err| { - self.vecs.pop_back(nr_iovecs); - err - })?; + let slice = mem + .get_slice(desc.addr, desc.len as usize) + .inspect_err(|_| { + self.vecs.pop_back(nr_iovecs); + })?; // We need to mark the area of guest memory that will be mutated through this // IoVecBufferMut as dirty ahead of time, as we loose access to all // vm-memory related information after converting down to iovecs. @@ -288,9 +289,8 @@ impl IoVecBufferMut { length = length .checked_add(desc.len) .ok_or(IoVecError::OverflowedDescriptor) - .map_err(|err| { + .inspect_err(|_| { self.vecs.pop_back(nr_iovecs); - err })?; next_descriptor = desc.next_descriptor(); diff --git a/src/vmm/src/devices/virtio/mmio.rs b/src/vmm/src/devices/virtio/mmio.rs index f734058c5b1a..63d7a80d84d8 100644 --- a/src/vmm/src/devices/virtio/mmio.rs +++ b/src/vmm/src/devices/virtio/mmio.rs @@ -38,9 +38,9 @@ const MMIO_VERSION: u32 = 2; /// /// 1. Mmio reads and writes must be sent to this device at what is referred to here as MMIO base. /// 1. `Mmio::queue_evts` must be installed at `virtio::NOTIFY_REG_OFFSET` offset from the MMIO -/// base. Each event in the array must be signaled if the index is written at that offset. +/// base. Each event in the array must be signaled if the index is written at that offset. /// 1. `Mmio::interrupt_evt` must signal an interrupt that the guest driver is listening to when it -/// is written to. +/// is written to. /// /// Typically one page (4096 bytes) of MMIO address space is sufficient to handle this transport /// and inner virtio device. diff --git a/src/vmm/src/devices/virtio/net/device.rs b/src/vmm/src/devices/virtio/net/device.rs index 818fed86a304..a17c25d72481 100755 --- a/src/vmm/src/devices/virtio/net/device.rs +++ b/src/vmm/src/devices/virtio/net/device.rs @@ -514,10 +514,9 @@ impl Net { NetError::VnetHeaderMissing })?; - let headers = frame_bytes_from_buf(&headers[..header_len]).map_err(|e| { + let headers = frame_bytes_from_buf(&headers[..header_len]).inspect_err(|_| { error!("VNET headers missing in TX frame"); net_metrics.tx_malformed_frames.inc(); - e })?; if let Some(ns) = mmds_ns { diff --git a/src/vmm/src/devices/virtio/net/metrics.rs b/src/vmm/src/devices/virtio/net/metrics.rs index dfdf69702a7a..60e03f224de5 100644 --- a/src/vmm/src/devices/virtio/net/metrics.rs +++ b/src/vmm/src/devices/virtio/net/metrics.rs @@ -75,7 +75,8 @@ //! //! The system implements 1 types of metrics: //! * Shared Incremental Metrics (SharedIncMetrics) - dedicated for the metrics which need a counter -//! (i.e the number of times an API request failed). These metrics are reset upon flush. +//! (i.e the number of times an API request failed). These metrics are reset upon flush. +//! //! We use net::metrics::METRICS instead of adding an entry of NetDeviceMetrics //! in Net so that metrics are accessible to be flushed even from signal handlers. diff --git a/src/vmm/src/devices/virtio/queue.rs b/src/vmm/src/devices/virtio/queue.rs index 39ccef1f1ee6..7c2c78d2407e 100644 --- a/src/vmm/src/devices/virtio/queue.rs +++ b/src/vmm/src/devices/virtio/queue.rs @@ -172,9 +172,8 @@ impl Iterator for DescriptorIterator { type Item = DescriptorChain; fn next(&mut self) -> Option { - self.0.take().map(|desc| { + self.0.take().inspect(|desc| { self.0 = desc.next_descriptor(); - desc }) } } @@ -560,10 +559,9 @@ impl Queue { // index is bound by the queue size let desc_index = unsafe { self.avail_ring_ring_get(usize::from(idx)) }; - DescriptorChain::checked_new(self.desc_table_ptr, self.actual_size(), desc_index).map( - |dc| { + DescriptorChain::checked_new(self.desc_table_ptr, self.actual_size(), desc_index).inspect( + |_| { self.next_avail += Wrapping(1); - dc }, ) } @@ -575,9 +573,8 @@ impl Queue { } /// Write used element into used_ring ring. - /// - [`ring_index_offset`] is an offset added to - /// the current [`self.next_used`] to obtain actual index - /// into used_ring. + /// - [`ring_index_offset`] is an offset added to the current [`self.next_used`] to obtain + /// actual index into used_ring. pub fn write_used_element( &mut self, ring_index_offset: u16, diff --git a/src/vmm/src/devices/virtio/rng/device.rs b/src/vmm/src/devices/virtio/rng/device.rs index 96513e49b266..dca25999f3e8 100644 --- a/src/vmm/src/devices/virtio/rng/device.rs +++ b/src/vmm/src/devices/virtio/rng/device.rs @@ -119,9 +119,8 @@ impl Entropy { } let mut rand_bytes = vec![0; self.buffer.len() as usize]; - rand::fill(&mut rand_bytes).map_err(|err| { + rand::fill(&mut rand_bytes).inspect_err(|_| { METRICS.host_rng_fails.inc(); - err })?; // It is ok to unwrap here. We are writing `iovec.len()` bytes at offset 0. diff --git a/src/vmm/src/devices/virtio/rng/metrics.rs b/src/vmm/src/devices/virtio/rng/metrics.rs index eb1dcbfacaee..e02f5ce8cc44 100644 --- a/src/vmm/src/devices/virtio/rng/metrics.rs +++ b/src/vmm/src/devices/virtio/rng/metrics.rs @@ -31,7 +31,7 @@ //! //! The system implements 1 type of metrics: //! * Shared Incremental Metrics (SharedIncMetrics) - dedicated for the metrics which need a counter -//! (i.e the number of times an API request failed). These metrics are reset upon flush. +//! (i.e the number of times an API request failed). These metrics are reset upon flush. use serde::ser::SerializeMap; use serde::{Serialize, Serializer}; diff --git a/src/vmm/src/devices/virtio/vhost_user_metrics.rs b/src/vmm/src/devices/virtio/vhost_user_metrics.rs index 73b41e6a86cd..0a3c8cd3a6ab 100644 --- a/src/vmm/src/devices/virtio/vhost_user_metrics.rs +++ b/src/vmm/src/devices/virtio/vhost_user_metrics.rs @@ -64,9 +64,10 @@ //! //! The system implements 2 type of metrics: //! * Shared Incremental Metrics (SharedIncMetrics) - dedicated for the metrics which need a counter -//! (i.e the number of times activating a device failed). These metrics are reset upon flush. +//! (i.e the number of times activating a device failed). These metrics are reset upon flush. //! * Shared Store Metrics (SharedStoreMetrics) - are targeted at keeping a persistent value, it is //! `not` intended to act as a counter (i.e for measure the process start up time for example). +//! //! We add VhostUserDeviceMetrics entries from vhost_user_metrics::METRICS into vhost_user device //! instead of vhost_user device having individual separate VhostUserDeviceMetrics entries because //! vhost_user device is not accessible from signal handlers to flush metrics and diff --git a/src/vmm/src/devices/virtio/vsock/event_handler.rs b/src/vmm/src/devices/virtio/vsock/event_handler.rs index 40e75d1a9f5d..59cf3fe0103c 100755 --- a/src/vmm/src/devices/virtio/vsock/event_handler.rs +++ b/src/vmm/src/devices/virtio/vsock/event_handler.rs @@ -9,7 +9,7 @@ use std::fmt::Debug; /// The vsock object implements the runtime logic of our vsock device: /// 1. Respond to TX queue events by wrapping virtio buffers into `VsockPacket`s, then sending -/// those packets to the `VsockBackend`; +/// those packets to the `VsockBackend`; /// 2. Forward backend FD event notifications to the `VsockBackend`; /// 3. Fetch incoming packets from the `VsockBackend` and place them into the virtio RX queue; /// 4. Whenever we have processed some virtio buffers (either TX or RX), let the driver know by diff --git a/src/vmm/src/devices/virtio/vsock/metrics.rs b/src/vmm/src/devices/virtio/vsock/metrics.rs index 5307302f5ca1..d626d5dca344 100644 --- a/src/vmm/src/devices/virtio/vsock/metrics.rs +++ b/src/vmm/src/devices/virtio/vsock/metrics.rs @@ -34,7 +34,7 @@ //! //! The system implements 1 type of metrics: //! * Shared Incremental Metrics (SharedIncMetrics) - dedicated for the metrics which need a counter -//! (i.e the number of times an API request failed). These metrics are reset upon flush. +//! (i.e the number of times an API request failed). These metrics are reset upon flush. use serde::ser::SerializeMap; use serde::{Serialize, Serializer}; diff --git a/src/vmm/src/devices/virtio/vsock/packet.rs b/src/vmm/src/devices/virtio/vsock/packet.rs index 213b5bb2b343..f19723d25cb0 100644 --- a/src/vmm/src/devices/virtio/vsock/packet.rs +++ b/src/vmm/src/devices/virtio/vsock/packet.rs @@ -7,6 +7,7 @@ //! virtio queue: //! - the packet header; and //! - the packet data/buffer. +//! //! There is a 1:1 relation between descriptor chains and packets: the first (chain head) holds //! the header, and an optional second descriptor holds the data. The second descriptor is only //! present for data packets (VSOCK_OP_RW). diff --git a/src/vmm/src/devices/virtio/vsock/unix/muxer.rs b/src/vmm/src/devices/virtio/vsock/unix/muxer.rs index 4d4b19962371..84273f45aba0 100644 --- a/src/vmm/src/devices/virtio/vsock/unix/muxer.rs +++ b/src/vmm/src/devices/virtio/vsock/unix/muxer.rs @@ -24,11 +24,12 @@ /// destination port to which it wants to connect); /// 3. Some event was triggered for a connected Unix socket, that belongs to a /// `VsockConnection`. -/// The muxer gets notified about all of these events, because, as a `VsockEpollListener` -/// implementor, it gets to register a nested epoll FD into the main VMM epolling loop. All -/// other pollable FDs are then registered under this nested epoll FD. -/// To route all these events to their handlers, the muxer uses another `HashMap` object, -/// mapping `RawFd`s to `EpollListener`s. +/// +/// The muxer gets notified about all of these events, because, as a `VsockEpollListener` +/// implementor, it gets to register a nested epoll FD into the main VMM epolling loop. All +/// other pollable FDs are then registered under this nested epoll FD. +/// To route all these events to their handlers, the muxer uses another `HashMap` object, +/// mapping `RawFd`s to `EpollListener`s. use std::collections::{HashMap, HashSet}; use std::fmt::Debug; use std::io::Read; diff --git a/src/vmm/src/devices/virtio/vsock/unix/muxer_rxq.rs b/src/vmm/src/devices/virtio/vsock/unix/muxer_rxq.rs index 8327882a3fb9..77f7670af7d6 100644 --- a/src/vmm/src/devices/virtio/vsock/unix/muxer_rxq.rs +++ b/src/vmm/src/devices/virtio/vsock/unix/muxer_rxq.rs @@ -67,6 +67,7 @@ impl MuxerRxQ { /// A push will fail when: /// - trying to push a connection key onto an out-of-sync, or full queue; or /// - trying to push an RST onto a queue already full of RSTs. + /// /// RSTs take precedence over connections, because connections can always be queried for /// pending RX data later. Aside from this queue, there is no other storage for RSTs, so /// failing to push one means that we have to drop the packet. diff --git a/src/vmm/src/dumbo/pdu/mod.rs b/src/vmm/src/dumbo/pdu/mod.rs index d700474ed02b..fc6a24760a73 100644 --- a/src/vmm/src/dumbo/pdu/mod.rs +++ b/src/vmm/src/dumbo/pdu/mod.rs @@ -66,8 +66,7 @@ enum ChecksumProto { /// * `bytes` - Raw bytes of a TCP packet or a UDP datagram /// * `src_addr` - IPv4 source address /// * `dst_addr` - IPv4 destination address -/// * `protocol` - **must** be either `PROTOCOL_TCP` or `PROTOCOL_UDP` defined in -/// `ipv4` module +/// * `protocol` - **must** be either `PROTOCOL_TCP` or `PROTOCOL_UDP` defined in `ipv4` module /// /// More details about TCP checksum computation can be found [here]. /// diff --git a/src/vmm/src/dumbo/tcp/endpoint.rs b/src/vmm/src/dumbo/tcp/endpoint.rs index 92058d83ee97..15fb10e10f99 100644 --- a/src/vmm/src/dumbo/tcp/endpoint.rs +++ b/src/vmm/src/dumbo/tcp/endpoint.rs @@ -281,9 +281,8 @@ impl Endpoint { tcp_payload_src, timestamp_cycles(), ) { - Ok(write_result) => write_result.map(|segment| { + Ok(write_result) => write_result.inspect(|segment| { self.response_seq += Wrapping(u32::from(segment.inner().payload_len())); - segment }), Err(_) => { METRICS.mmds.tx_errors.inc(); diff --git a/src/vmm/src/io_uring/mod.rs b/src/vmm/src/io_uring/mod.rs index 3466fd01aa50..f306148044ca 100644 --- a/src/vmm/src/io_uring/mod.rs +++ b/src/vmm/src/io_uring/mod.rs @@ -97,8 +97,8 @@ impl IoUring { /// /// # Arguments /// - /// * `num_entries` - Requested number of entries in the ring. Will be rounded up to the - /// nearest power of two. + /// * `num_entries` - Requested number of entries in the ring. Will be rounded up to the nearest + /// power of two. /// * `files` - Files to be registered for IO. /// * `restrictions` - Vector of [`Restriction`](restriction/enum.Restriction.html)s /// * `eventfd` - Optional eventfd for receiving completion notifications. @@ -175,10 +175,9 @@ impl IoUring { } self.squeue .push(op.into_sqe(&mut self.slab)) - .map(|res| { + .inspect(|_| { // This is safe since self.num_ops < IORING_MAX_CQ_ENTRIES (65536) self.num_ops += 1; - res }) .map_err(|(sqe_err, user_data_key)| -> (IoUringError, T) { ( @@ -207,11 +206,10 @@ impl IoUring { self.cqueue .pop(&mut self.slab) .map(|maybe_cqe| { - maybe_cqe.map(|cqe| { + maybe_cqe.inspect(|_| { // This is safe since the pop-ed CQEs have been previously pushed. However // we use a saturating_sub for extra safety. self.num_ops = self.num_ops.saturating_sub(1); - cqe }) }) .map_err(IoUringError::CQueue) diff --git a/src/vmm/src/lib.rs b/src/vmm/src/lib.rs index c80f004e789e..6a28e14f26b0 100644 --- a/src/vmm/src/lib.rs +++ b/src/vmm/src/lib.rs @@ -368,15 +368,13 @@ impl Vmm { if let Some(stdin) = self.events_observer.as_mut() { // Set raw mode for stdin. - stdin.lock().set_raw_mode().map_err(|err| { + stdin.lock().set_raw_mode().inspect_err(|&err| { warn!("Cannot set raw mode for the terminal. {:?}", err); - err })?; // Set non blocking stdin. - stdin.lock().set_non_block(true).map_err(|err| { + stdin.lock().set_non_block(true).inspect_err(|&err| { warn!("Cannot set non block for the terminal. {:?}", err); - err })?; } @@ -906,9 +904,8 @@ impl Drop for Vmm { self.stop(self.shutdown_exit_code.unwrap_or(FcExitCode::Ok)); if let Some(observer) = self.events_observer.as_mut() { - let res = observer.lock().set_canon_mode().map_err(|err| { + let res = observer.lock().set_canon_mode().inspect_err(|&err| { warn!("Cannot set canonical mode for the terminal. {:?}", err); - err }); if let Err(err) = res { warn!("{}", VmmError::VmmObserverTeardown(err)); diff --git a/src/vmm/src/logger/metrics.rs b/src/vmm/src/logger/metrics.rs index bcde569115e4..dd8fa1a293ce 100644 --- a/src/vmm/src/logger/metrics.rs +++ b/src/vmm/src/logger/metrics.rs @@ -46,10 +46,9 @@ //! //! The system implements 2 types of metrics: //! * Shared Incremental Metrics (SharedIncMetrics) - dedicated for the metrics which need a counter -//! (i.e the number of times an API request failed). These metrics are reset upon flush. +//! (i.e the number of times an API request failed). These metrics are reset upon flush. //! * Shared Store Metrics (SharedStoreMetrics) - are targeted at keeping a persistent value, it is -//! not -//! intended to act as a counter (i.e for measure the process start up time for example). +//! not intended to act as a counter (i.e for measure the process start up time for example). //! //! The current approach for the `SharedIncMetrics` type is to store two values (current and //! previous) and compute the delta between them each time we do a flush (i.e by serialization). @@ -58,6 +57,7 @@ //! to actual writing, so less synchronization effort is required. //! * We don't have to worry at all that much about losing some data if writing fails for a while //! (this could be a concern, I guess). +//! //! If if turns out this approach is not really what we want, it's pretty easy to resort to //! something else, while working behind the same interface. diff --git a/src/vmm/src/rate_limiter/mod.rs b/src/vmm/src/rate_limiter/mod.rs index b4c2a6d34edf..6943f5d8ee40 100644 --- a/src/vmm/src/rate_limiter/mod.rs +++ b/src/vmm/src/rate_limiter/mod.rs @@ -328,15 +328,15 @@ impl RateLimiter { /// # Arguments /// /// * `bytes_total_capacity` - the total capacity of the `TokenType::Bytes` token bucket. - /// * `bytes_one_time_burst` - initial extra credit on top of `bytes_total_capacity`, - /// that does not replenish and which can be used for an initial burst of data. - /// * `bytes_complete_refill_time_ms` - number of milliseconds for the `TokenType::Bytes` - /// token bucket to go from zero Bytes to `bytes_total_capacity` Bytes. + /// * `bytes_one_time_burst` - initial extra credit on top of `bytes_total_capacity`, that does + /// not replenish and which can be used for an initial burst of data. + /// * `bytes_complete_refill_time_ms` - number of milliseconds for the `TokenType::Bytes` token + /// bucket to go from zero Bytes to `bytes_total_capacity` Bytes. /// * `ops_total_capacity` - the total capacity of the `TokenType::Ops` token bucket. - /// * `ops_one_time_burst` - initial extra credit on top of `ops_total_capacity`, - /// that does not replenish and which can be used for an initial burst of data. + /// * `ops_one_time_burst` - initial extra credit on top of `ops_total_capacity`, that does not + /// replenish and which can be used for an initial burst of data. /// * `ops_complete_refill_time_ms` - number of milliseconds for the `TokenType::Ops` token - /// bucket to go from zero Ops to `ops_total_capacity` Ops. + /// bucket to go from zero Ops to `ops_total_capacity` Ops. /// /// If either bytes/ops *size* or *refill_time* are **zero**, the limiter /// is **disabled** for that respective token type. diff --git a/src/vmm/src/resources.rs b/src/vmm/src/resources.rs index 0ad4df8aa191..0cde08a844db 100644 --- a/src/vmm/src/resources.rs +++ b/src/vmm/src/resources.rs @@ -1529,7 +1529,7 @@ mod tests { .as_cstring() .unwrap() .as_bytes_with_nul(), - [cmdline.as_bytes(), &[b'\0']].concat() + [cmdline.as_bytes(), b"\0"].concat() ); assert_ne!( boot_builder.kernel_file.metadata().unwrap().st_ino(), @@ -1554,7 +1554,7 @@ mod tests { .as_cstring() .unwrap() .as_bytes_with_nul(), - [cmdline.as_bytes(), &[b'\0']].concat() + [cmdline.as_bytes(), b"\0"].concat() ); assert_eq!( boot_source_builder.kernel_file.metadata().unwrap().st_ino(), diff --git a/src/vmm/src/rpc_interface.rs b/src/vmm/src/rpc_interface.rs index 566228fd53a2..7f66880edab7 100644 --- a/src/vmm/src/rpc_interface.rs +++ b/src/vmm/src/rpc_interface.rs @@ -572,20 +572,18 @@ impl<'a> PrebootApiController<'a> { load_params, self.vm_resources, ) - .map_err(|err| { + .inspect_err(|_| { // If restore fails, we consider the process is too dirty to recover. self.fatal_error = Some(BuildMicrovmFromRequestsError::Restore); - err })?; // Resume VM if load_params.resume_vm { vmm.lock() .expect("Poisoned lock") .resume_vm() - .map_err(|err| { + .inspect_err(|_| { // If resume fails, we consider the process is too dirty to recover. self.fatal_error = Some(BuildMicrovmFromRequestsError::Resume); - err })?; } // Set the VM diff --git a/src/vmm/src/vmm_config/boot_source.rs b/src/vmm/src/vmm_config/boot_source.rs index c40a0fde014c..1bfed7bad9cb 100644 --- a/src/vmm/src/vmm_config/boot_source.rs +++ b/src/vmm/src/vmm_config/boot_source.rs @@ -119,7 +119,7 @@ pub(crate) mod tests { assert!(boot_cfg.initrd_file.is_none()); assert_eq!( boot_cfg.cmdline.as_cstring().unwrap().as_bytes_with_nul(), - [DEFAULT_KERNEL_CMDLINE.as_bytes(), &[b'\0']].concat() + [DEFAULT_KERNEL_CMDLINE.as_bytes(), b"\0"].concat() ); }