From 36713056968fb9106ec0da6c0d964293f0425e99 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Fri, 2 Aug 2019 15:00:45 -0700 Subject: devices: virtio: add Error type for descriptors Add an error type to describe descriptor Errors in more detail. This lets us return a more accurate error in a later CL in this chain by adding a VolatileMemoryError variant. BUG=chromium:990546 TEST=./build_test Change-Id: I08680d0cb64bfc3667bac7b2ad8a8bc0e78e8058 Signed-off-by: Daniel Verkamp Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1733988 Tested-by: kokoro Reviewed-by: Zach Reizner --- devices/src/virtio/descriptor_utils.rs | 59 +++++++++++++++++++++++++++------- devices/src/virtio/mod.rs | 1 + devices/src/virtio/net.rs | 6 ++-- 3 files changed, 52 insertions(+), 14 deletions(-) diff --git a/devices/src/virtio/descriptor_utils.rs b/devices/src/virtio/descriptor_utils.rs index 141c4bf..9d18a63 100644 --- a/devices/src/virtio/descriptor_utils.rs +++ b/devices/src/virtio/descriptor_utils.rs @@ -3,15 +3,38 @@ // found in the LICENSE file. use std::cmp; +use std::fmt::{self, Display}; use std::io; use std::os::unix::io::AsRawFd; +use std::result; use data_model::DataInit; -use sys_util::guest_memory::{Error, Result}; +use sys_util::guest_memory::Error as GuestMemoryError; use sys_util::{GuestAddress, GuestMemory}; use super::DescriptorChain; +#[derive(Debug)] +pub enum Error { + GuestMemoryError(sys_util::GuestMemoryError), + IoError(io::Error), +} + +impl Display for Error { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use self::Error::*; + + match self { + GuestMemoryError(e) => write!(f, "descriptor guest memory error: {}", e), + IoError(e) => write!(f, "descriptor I/O error: {}", e), + } + } +} + +pub type Result = result::Result; + +impl std::error::Error for Error {} + #[derive(PartialEq, Eq)] enum DescriptorFilter { OnlyReadable, @@ -70,7 +93,9 @@ impl<'a> DescriptorChainConsumer<'a> { let addr = current .addr .checked_add(self.offset as u64) - .ok_or_else(|| Error::InvalidGuestAddress(current.addr))?; + .ok_or_else(|| { + Error::GuestMemoryError(GuestMemoryError::InvalidGuestAddress(current.addr)) + })?; let len = cmp::min(count, current.len as usize - self.offset); fnc(addr, len)?; @@ -147,7 +172,7 @@ impl<'a> Reader<'a> { if result.is_ok() { read_count += count; } - result + result.map_err(Error::GuestMemoryError) }, len, ) @@ -162,10 +187,10 @@ impl<'a> Reader<'a> { if count == buf.len() { Ok(()) } else { - Err(Error::ShortRead { + Err(Error::GuestMemoryError(GuestMemoryError::ShortRead { expected: buf.len(), completed: count, - }) + })) } } @@ -181,8 +206,13 @@ impl<'a> Reader<'a> { /// enough data in the descriptor chain buffer. pub fn read_to(&mut self, dst: &dyn AsRawFd, count: usize) -> Result { let mem = self.mem; - self.buffer - .consume(|addr, count| mem.write_from_memory(addr, dst, count), count) + self.buffer.consume( + |addr, count| { + mem.write_from_memory(addr, dst, count) + .map_err(Error::GuestMemoryError) + }, + count, + ) } /// Returns number of bytes available for reading. @@ -233,7 +263,7 @@ impl<'a> Writer<'a> { if result.is_ok() { write_count += count; } - result + result.map_err(Error::GuestMemoryError) }, len, ) @@ -248,10 +278,10 @@ impl<'a> Writer<'a> { if count == buf.len() { Ok(()) } else { - Err(Error::ShortRead { + Err(Error::GuestMemoryError(GuestMemoryError::ShortRead { expected: buf.len(), completed: count, - }) + })) } } @@ -271,8 +301,13 @@ impl<'a> Writer<'a> { /// there isn't enough data in the descriptor chain buffer. pub fn write_from(&mut self, src: &dyn AsRawFd, count: usize) -> Result { let mem = self.mem; - self.buffer - .consume(|addr, count| mem.read_to_memory(addr, src, count), count) + self.buffer.consume( + |addr, count| { + mem.read_to_memory(addr, src, count) + .map_err(Error::GuestMemoryError) + }, + count, + ) } /// Returns number of bytes already written to the descriptor chain buffer. diff --git a/devices/src/virtio/mod.rs b/devices/src/virtio/mod.rs index 7bead30..0970b86 100644 --- a/devices/src/virtio/mod.rs +++ b/devices/src/virtio/mod.rs @@ -27,6 +27,7 @@ pub mod vhost; pub use self::balloon::*; pub use self::block::*; +pub use self::descriptor_utils::Error as DescriptorError; pub use self::descriptor_utils::*; #[cfg(feature = "gpu")] pub use self::gpu::*; diff --git a/devices/src/virtio/net.rs b/devices/src/virtio/net.rs index 73a8499..4875178 100644 --- a/devices/src/virtio/net.rs +++ b/devices/src/virtio/net.rs @@ -19,7 +19,9 @@ use sys_util::{error, warn, EventFd, GuestMemory, PollContext, PollToken}; use virtio_sys::virtio_net::virtio_net_hdr_v1; use virtio_sys::{vhost, virtio_net}; -use super::{Queue, Reader, VirtioDevice, Writer, INTERRUPT_STATUS_USED_RING, TYPE_NET}; +use super::{ + DescriptorError, Queue, Reader, VirtioDevice, Writer, INTERRUPT_STATUS_USED_RING, TYPE_NET, +}; /// The maximum buffer size when segmentation offload is enabled. This /// includes the 12-byte virtio net header. @@ -118,7 +120,7 @@ where match writer.write_all(&self.rx_buf[0..self.rx_count]) { Ok(()) => (), - Err(MemoryError::ShortWrite { .. }) => { + Err(DescriptorError::GuestMemoryError(MemoryError::ShortWrite { .. })) => { warn!( "net: rx: buffer is too small to hold frame of size {}", self.rx_count -- cgit 1.4.1