From 417bc6cd03142c1c4219857989b75ac5c56aa47e Mon Sep 17 00:00:00 2001 From: Kornel <kornel@cloudflare.com> Date: Fri, 16 Feb 2024 15:45:05 +0000 Subject: [PATCH] refactor(virtio): Simplify read_config From: Kornel <kornel@cloudflare.com> `io::Write` has higher overhead and unnecessary error handling. Slices can be copied without it. Signed-off-by: Kornel <kornel@cloudflare.com> Signed-off-by: Patrick Roy <roypat@amazon.co.uk> Co-authored-by: Patrick Roy <roypat@amazon.co.uk> --- src/vmm/src/devices/virtio/balloon/device.rs | 21 ++++++------------- .../devices/virtio/block/vhost_user/device.rs | 18 +++++----------- src/vmm/src/devices/virtio/net/device.rs | 20 ++++++------------ 3 files changed, 17 insertions(+), 42 deletions(-) diff --git a/src/vmm/src/devices/virtio/balloon/device.rs b/src/vmm/src/devices/virtio/balloon/device.rs index 9a69312729f..77a13902cc2 100644 --- a/src/vmm/src/devices/virtio/balloon/device.rs +++ b/src/vmm/src/devices/virtio/balloon/device.rs @@ -1,11 +1,10 @@ // Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -use std::io::Write; +use std::fmt; use std::sync::atomic::AtomicU32; use std::sync::Arc; use std::time::Duration; -use std::{cmp, fmt}; use log::error; use serde::Serialize; @@ -593,20 +592,12 @@ impl VirtioDevice for Balloon { self.irq_trigger.irq_status.clone() } - fn read_config(&self, offset: u64, mut data: &mut [u8]) { - let config_space_bytes = self.config_space.as_slice(); - let config_len = config_space_bytes.len() as u64; - if offset >= config_len { + fn read_config(&self, offset: u64, data: &mut [u8]) { + if let Some(config_space_bytes) = self.config_space.as_slice().get(u64_to_usize(offset)..) { + let len = config_space_bytes.len().min(data.len()); + data[..len].copy_from_slice(&config_space_bytes[..len]); + } else { error!("Failed to read config space"); - return; - } - - if let Some(end) = offset.checked_add(data.len() as u64) { - // This write can't fail, offset and end are checked against config_len. - data.write_all( - &config_space_bytes[u64_to_usize(offset)..u64_to_usize(cmp::min(end, config_len))], - ) - .unwrap(); } } diff --git a/src/vmm/src/devices/virtio/block/vhost_user/device.rs b/src/vmm/src/devices/virtio/block/vhost_user/device.rs index a9cb8523f0f..2304262c26e 100644 --- a/src/vmm/src/devices/virtio/block/vhost_user/device.rs +++ b/src/vmm/src/devices/virtio/block/vhost_user/device.rs @@ -4,8 +4,6 @@ // Portions Copyright 2019 Intel Corporation. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -use std::cmp; -use std::io::Write; use std::sync::atomic::AtomicU32; use std::sync::Arc; @@ -322,19 +320,13 @@ impl<T: VhostUserHandleBackend + Send + 'static> VirtioDevice for VhostUserBlock self.irq_trigger.irq_status.clone() } - fn read_config(&self, offset: u64, mut data: &mut [u8]) { - let config_len = self.config_space.len() as u64; - if offset >= config_len { + fn read_config(&self, offset: u64, data: &mut [u8]) { + if let Some(config_space_bytes) = self.config_space.as_slice().get(u64_to_usize(offset)..) { + let len = config_space_bytes.len().min(data.len()); + data[..len].copy_from_slice(&config_space_bytes[..len]); + } else { error!("Failed to read config space"); self.metrics.cfg_fails.inc(); - return; - } - if let Some(end) = offset.checked_add(data.len() as u64) { - // This write can't fail, offset and end are checked against config_len. - data.write_all( - &self.config_space[u64_to_usize(offset)..u64_to_usize(cmp::min(end, config_len))], - ) - .unwrap(); } } diff --git a/src/vmm/src/devices/virtio/net/device.rs b/src/vmm/src/devices/virtio/net/device.rs index 430ce87cd3a..f79692012e5 100755 --- a/src/vmm/src/devices/virtio/net/device.rs +++ b/src/vmm/src/devices/virtio/net/device.rs @@ -7,11 +7,10 @@ #[cfg(not(test))] use std::io::Read; -use std::io::Write; +use std::mem; use std::net::Ipv4Addr; use std::sync::atomic::AtomicU32; use std::sync::{Arc, Mutex}; -use std::{cmp, mem}; use libc::EAGAIN; use log::{error, warn}; @@ -832,20 +831,13 @@ impl VirtioDevice for Net { self.irq_trigger.irq_status.clone() } - fn read_config(&self, offset: u64, mut data: &mut [u8]) { - let config_space_bytes = self.config_space.as_slice(); - let config_len = config_space_bytes.len() as u64; - if offset >= config_len { + fn read_config(&self, offset: u64, data: &mut [u8]) { + if let Some(config_space_bytes) = self.config_space.as_slice().get(u64_to_usize(offset)..) { + let len = config_space_bytes.len().min(data.len()); + data[..len].copy_from_slice(&config_space_bytes[..len]); + } else { error!("Failed to read config space"); self.metrics.cfg_fails.inc(); - return; - } - if let Some(end) = offset.checked_add(data.len() as u64) { - // This write can't fail, offset and end are checked against config_len. - data.write_all( - &config_space_bytes[u64_to_usize(offset)..u64_to_usize(cmp::min(end, config_len))], - ) - .unwrap(); } }