disk: add get_len() to eliminate need for Seek

This new trait allows DiskFile implementors to provide the length of the
file directly rather than using SeekFrom::End with seek().

BUG=None
TEST=./build_test
TEST=Boot Termina in crosvm

Change-Id: I9447ebb43dbd5fbb32a3a6b6d2fc969b9406cdbc
Signed-off-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1913961
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

6 files changed, 46 insertions, 34 deletions

diff --git a/devices/src/virtio/block.rs b/devices/src/virtio/block.rs
index 2910e1d..1fc69a0 100644
--- a/devices/src/virtio/block.rs
+++ b/devices/src/virtio/block.rs
@@ -4,7 +4,7 @@
 
 use std::cmp::{max, min};
 use std::fmt::{self, Display};
-use std::io::{self, Seek, SeekFrom, Write};
+use std::io::{self, Write};
 use std::mem::size_of;
 use std::os::unix::io::{AsRawFd, RawFd};
 use std::result;
@@ -344,7 +344,7 @@ impl Worker {
             return DiskControlResult::Err(SysError::new(libc::EIO));
         }
 
-        if let Ok(new_disk_size) = self.disk_image.seek(SeekFrom::End(0)) {
+        if let Ok(new_disk_size) = self.disk_image.get_len() {
             let mut disk_size = self.disk_size.lock();
             *disk_size = new_disk_size;
         }
@@ -484,16 +484,14 @@ fn build_config_space(disk_size: u64, seg_max: u32) -> virtio_blk_config {
 }
 
 impl Block {
-    /// Create a new virtio block device that operates on the given file.
-    ///
-    /// The given file must be seekable and sizable.
+    /// Create a new virtio block device that operates on the given DiskFile.
     pub fn new(
-        mut disk_image: Box<dyn DiskFile>,
+        disk_image: Box<dyn DiskFile>,
         read_only: bool,
         sparse: bool,
         control_socket: Option<DiskControlResponseSocket>,
     ) -> SysResult<Block> {
-        let disk_size = disk_image.seek(SeekFrom::End(0))? as u64;
+        let disk_size = disk_image.get_len()?;
         if disk_size % SECTOR_SIZE != 0 {
             warn!(
                 "Disk size {} is not a multiple of sector size {}; \
diff --git a/disk/src/composite.rs b/disk/src/composite.rs
index da0fe07..bd21b33 100644
--- a/disk/src/composite.rs
+++ b/disk/src/composite.rs
@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 use std::cmp::{max, min};
-use std::convert::TryFrom;
 use std::fmt::{self, Display};
 use std::fs::{File, OpenOptions};
 use std::io::{self, ErrorKind, Read, Seek, SeekFrom};
@@ -14,7 +13,9 @@ use crate::{create_disk_file, DiskFile, ImageType};
 use data_model::VolatileSlice;
 use protos::cdisk_spec;
 use remain::sorted;
-use sys_util::{AsRawFds, FileReadWriteAtVolatile, FileSetLen, FileSync, PunchHole, WriteZeroesAt};
+use sys_util::{
+    AsRawFds, FileGetLen, FileReadWriteAtVolatile, FileSetLen, FileSync, PunchHole, WriteZeroesAt,
+};
 
 #[sorted]
 #[derive(Debug)]
@@ -68,7 +69,6 @@ impl ComponentDiskPart {
 /// and not overlapping.
 pub struct CompositeDiskFile {
     component_disks: Vec<ComponentDiskPart>,
-    cursor_location: u64,
 }
 
 fn ranges_overlap(a: &Range<u64>, b: &Range<u64>) -> bool {
@@ -109,7 +109,6 @@ impl CompositeDiskFile {
         }
         Ok(CompositeDiskFile {
             component_disks: disks,
-            cursor_location: 0,
         })
     }
 
@@ -211,6 +210,12 @@ impl CompositeDiskFile {
     }
 }
 
+impl FileGetLen for CompositeDiskFile {
+    fn get_len(&self) -> io::Result<u64> {
+        Ok(self.length())
+    }
+}
+
 impl FileSetLen for CompositeDiskFile {
     fn set_len(&self, _len: u64) -> io::Result<()> {
         Err(io::Error::new(ErrorKind::Other, "unsupported operation"))
@@ -287,19 +292,6 @@ impl PunchHole for CompositeDiskFile {
     }
 }
 
-impl Seek for CompositeDiskFile {
-    fn seek(&mut self, pos: SeekFrom) -> io::Result<u64> {
-        let cursor_location = match pos {
-            SeekFrom::Start(offset) => Ok(offset),
-            SeekFrom::End(offset) => u64::try_from(self.length() as i64 + offset),
-            SeekFrom::Current(offset) => u64::try_from(self.cursor_location as i64 + offset),
-        }
-        .map_err(|e| io::Error::new(ErrorKind::InvalidData, e))?;
-        self.cursor_location = cursor_location;
-        Ok(cursor_location)
-    }
-}
-
 impl WriteZeroesAt for CompositeDiskFile {
     fn write_zeroes_at(&mut self, offset: u64, length: usize) -> io::Result<usize> {
         let cursor_location = offset;
@@ -349,7 +341,7 @@ mod tests {
     }
 
     #[test]
-    fn seek_to_end() {
+    fn get_len() {
         let file1: File = SharedMemory::new(None).unwrap().into();
         let file2: File = SharedMemory::new(None).unwrap().into();
         let disk_part1 = ComponentDiskPart {
@@ -362,9 +354,9 @@ mod tests {
             offset: 100,
             length: 100,
         };
-        let mut composite = CompositeDiskFile::new(vec![disk_part1, disk_part2]).unwrap();
-        let location = composite.seek(SeekFrom::End(0)).unwrap();
-        assert_eq!(location, 200);
+        let composite = CompositeDiskFile::new(vec![disk_part1, disk_part2]).unwrap();
+        let len = composite.get_len().unwrap();
+        assert_eq!(len, 200);
     }
 
     #[test]
diff --git a/disk/src/disk.rs b/disk/src/disk.rs
index a9c44c6..c520d13 100644
--- a/disk/src/disk.rs
+++ b/disk/src/disk.rs
@@ -11,7 +11,8 @@ use libc::EINVAL;
 use qcow::{QcowFile, QCOW_MAGIC};
 use remain::sorted;
 use sys_util::{
-    AsRawFds, FileReadWriteAtVolatile, FileSetLen, FileSync, PunchHole, SeekHole, WriteZeroesAt,
+    AsRawFds, FileGetLen, FileReadWriteAtVolatile, FileSetLen, FileSync, PunchHole, SeekHole,
+    WriteZeroesAt,
 };
 
 #[cfg(feature = "composite-disk")]
@@ -41,10 +42,10 @@ pub type Result<T> = std::result::Result<T, Error>;
 #[rustfmt::skip] // rustfmt won't wrap the long list of trait bounds.
 pub trait DiskFile:
     FileSetLen
+    + FileGetLen
     + FileSync
     + FileReadWriteAtVolatile
     + PunchHole
-    + Seek
     + WriteZeroesAt
     + Send
     + AsRawFds
@@ -52,10 +53,10 @@ pub trait DiskFile:
 }
 impl<
         D: FileSetLen
+            + FileGetLen
             + FileSync
             + PunchHole
             + FileReadWriteAtVolatile
-            + Seek
             + WriteZeroesAt
             + Send
             + AsRawFds,
diff --git a/qcow/src/qcow.rs b/qcow/src/qcow.rs
index aa923f9..dd8c1a2 100644
--- a/qcow/src/qcow.rs
+++ b/qcow/src/qcow.rs
@@ -10,8 +10,8 @@ use data_model::{VolatileMemory, VolatileSlice};
 use libc::{EINVAL, ENOSPC, ENOTSUP};
 use remain::sorted;
 use sys_util::{
-    error, FileReadWriteAtVolatile, FileReadWriteVolatile, FileSetLen, FileSync, PunchHole,
-    SeekHole, WriteZeroesAt,
+    error, FileGetLen, FileReadWriteAtVolatile, FileReadWriteVolatile, FileSetLen, FileSync,
+    PunchHole, SeekHole, WriteZeroesAt,
 };
 
 use std::cmp::{max, min};
@@ -1568,6 +1568,12 @@ impl FileSetLen for QcowFile {
     }
 }
 
+impl FileGetLen for QcowFile {
+    fn get_len(&self) -> io::Result<u64> {
+        Ok(self.virtual_size())
+    }
+}
+
 impl PunchHole for QcowFile {
     fn punch_hole(&mut self, offset: u64, length: u64) -> std::io::Result<()> {
         let mut remaining = length;
diff --git a/sys_util/src/file_traits.rs b/sys_util/src/file_traits.rs
index 0e4d919..60ba050 100644
--- a/sys_util/src/file_traits.rs
+++ b/sys_util/src/file_traits.rs
@@ -41,6 +41,21 @@ impl FileSetLen for File {
     }
 }
 
+/// A trait for getting the size of a file.
+/// This is equivalent to File's metadata().len() method,
+/// but wrapped in a trait so that it can be implemented for
+/// other types.
+pub trait FileGetLen {
+    /// Get the current length of the file in bytes.
+    fn get_len(&self) -> Result<u64>;
+}
+
+impl FileGetLen for File {
+    fn get_len(&self) -> Result<u64> {
+        Ok(self.metadata()?.len())
+    }
+}
+
 /// A trait similar to `Read` and `Write`, but uses volatile memory as buffers.
 pub trait FileReadWriteVolatile {
     /// Read bytes from this file into the given slice, returning the number of bytes read on
diff --git a/sys_util/src/lib.rs b/sys_util/src/lib.rs
index 31b1662..7f97650 100644
--- a/sys_util/src/lib.rs
+++ b/sys_util/src/lib.rs
@@ -64,7 +64,7 @@ pub use crate::timerfd::*;
 pub use poll_token_derive::*;
 
 pub use crate::file_traits::{
-    AsRawFds, FileReadWriteAtVolatile, FileReadWriteVolatile, FileSetLen, FileSync,
+    AsRawFds, FileGetLen, FileReadWriteAtVolatile, FileReadWriteVolatile, FileSetLen, FileSync,
 };
 pub use crate::guest_memory::Error as GuestMemoryError;
 pub use crate::mmap::Error as MmapError;