devices: virtio: block: use FileReadWriteAtVolatile

Use the "at" variants of the read/write functions in the block device.
This reduces the number of syscalls on the host per I/O to one
(pread64/pwrite64) rather than two (lseek + read/write).

The CompositeDiskFile implementation is also updated in this commit,
since it's both a producer and consumer of DiskFile, and it isn't
trivial to update it in a separate commit without breaking compilation.

BUG=None
TEST=Start Crostini on kevin, banon, and nami

Change-Id: I031e7e87cd6c99504db8c56b1725ea51c1e27a53
Signed-off-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1845948
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Stephen Barber <smbarber@chromium.org>

5 files changed, 34 insertions, 35 deletions

diff --git a/devices/src/virtio/block.rs b/devices/src/virtio/block.rs
index a2a1d3d..2288849 100644
--- a/devices/src/virtio/block.rs
+++ b/devices/src/virtio/block.rs
@@ -601,15 +601,14 @@ impl Block {
                     .checked_shl(u32::from(SECTOR_SHIFT))
                     .ok_or(ExecuteError::OutOfRange)?;
                 check_range(offset, data_len as u64, disk_size)?;
-                disk.seek(SeekFrom::Start(offset))
-                    .map_err(|e| ExecuteError::Seek { ioerr: e, sector })?;
-                let actual_length = writer.write_from(disk, data_len).map_err(|desc_error| {
-                    ExecuteError::ReadIo {
-                        length: data_len,
-                        sector,
-                        desc_error,
-                    }
-                })?;
+                let actual_length =
+                    writer
+                        .write_from_at(disk, data_len, offset)
+                        .map_err(|desc_error| ExecuteError::ReadIo {
+                            length: data_len,
+                            sector,
+                            desc_error,
+                        })?;
                 if actual_length < data_len {
                     return Err(ExecuteError::ShortRead {
                         sector,
@@ -624,11 +623,9 @@ impl Block {
                     .checked_shl(u32::from(SECTOR_SHIFT))
                     .ok_or(ExecuteError::OutOfRange)?;
                 check_range(offset, data_len as u64, disk_size)?;
-                disk.seek(SeekFrom::Start(offset))
-                    .map_err(|e| ExecuteError::Seek { ioerr: e, sector })?;
                 let actual_length =
                     reader
-                        .read_to(disk, data_len)
+                        .read_to_at(disk, data_len, offset)
                         .map_err(|desc_error| ExecuteError::WriteIo {
                             length: data_len,
                             sector,
diff --git a/disk/src/composite.rs b/disk/src/composite.rs
index 5d6e5a2..6c4ae04 100644
--- a/disk/src/composite.rs
+++ b/disk/src/composite.rs
@@ -14,7 +14,7 @@ use crate::{create_disk_file, DiskFile, ImageType};
 use data_model::VolatileSlice;
 use protos::cdisk_spec;
 use remain::sorted;
-use sys_util::{AsRawFds, FileReadWriteVolatile, FileSetLen, FileSync, PunchHole, WriteZeroes};
+use sys_util::{AsRawFds, FileReadWriteAtVolatile, FileSetLen, FileSync, PunchHole, WriteZeroes};
 
 #[sorted]
 #[derive(Debug)]
@@ -235,12 +235,10 @@ impl FileSync for CompositeDiskFile {
 //
 // If one of the component disks does a partial read or write, that also gets passed
 // transparently to the parent.
-impl FileReadWriteVolatile for CompositeDiskFile {
-    fn read_volatile(&mut self, slice: VolatileSlice) -> io::Result<usize> {
-        let cursor_location = self.cursor_location;
+impl FileReadWriteAtVolatile for CompositeDiskFile {
+    fn read_at_volatile(&mut self, slice: VolatileSlice, offset: u64) -> io::Result<usize> {
+        let cursor_location = offset;
         let disk = self.disk_at_offset(cursor_location)?;
-        disk.file
-            .seek(SeekFrom::Start(cursor_location - disk.offset))?;
         let subslice = if cursor_location + slice.size() > disk.offset + disk.length {
             let new_size = disk.offset + disk.length - cursor_location;
             slice
@@ -249,17 +247,12 @@ impl FileReadWriteVolatile for CompositeDiskFile {
         } else {
             slice
         };
-        let result = disk.file.read_volatile(subslice);
-        if let Ok(size) = result {
-            self.cursor_location += size as u64;
-        }
-        result
+        disk.file
+            .read_at_volatile(subslice, cursor_location - disk.offset)
     }
-    fn write_volatile(&mut self, slice: VolatileSlice) -> io::Result<usize> {
-        let cursor_location = self.cursor_location;
+    fn write_at_volatile(&mut self, slice: VolatileSlice, offset: u64) -> io::Result<usize> {
+        let cursor_location = offset;
         let disk = self.disk_at_offset(cursor_location)?;
-        disk.file
-            .seek(SeekFrom::Start(cursor_location - disk.offset))?;
         let subslice = if cursor_location + slice.size() > disk.offset + disk.length {
             let new_size = disk.offset + disk.length - cursor_location;
             slice
@@ -268,11 +261,8 @@ impl FileReadWriteVolatile for CompositeDiskFile {
         } else {
             slice
         };
-        let result = disk.file.write_volatile(subslice);
-        if let Ok(size) = result {
-            self.cursor_location += size as u64;
-        }
-        result
+        disk.file
+            .write_at_volatile(subslice, cursor_location - disk.offset)
     }
 }
 
diff --git a/disk/src/disk.rs b/disk/src/disk.rs
index f63d63c..b5692a3 100644
--- a/disk/src/disk.rs
+++ b/disk/src/disk.rs
@@ -11,7 +11,7 @@ use libc::EINVAL;
 use qcow::{QcowFile, QCOW_MAGIC};
 use remain::sorted;
 use sys_util::{
-    AsRawFds, FileReadWriteVolatile, FileSetLen, FileSync, PunchHole, SeekHole, WriteZeroes,
+    AsRawFds, FileReadWriteAtVolatile, FileSetLen, FileSync, PunchHole, SeekHole, WriteZeroes,
 };
 
 #[cfg(feature = "composite-disk")]
@@ -38,15 +38,23 @@ pub enum Error {
 pub type Result<T> = std::result::Result<T, Error>;
 
 /// The prerequisites necessary to support a block device.
+#[rustfmt::skip] // rustfmt won't wrap the long list of trait bounds.
 pub trait DiskFile:
-    FileSetLen + FileSync + FileReadWriteVolatile + PunchHole + Seek + WriteZeroes + Send + AsRawFds
+    FileSetLen
+    + FileSync
+    + FileReadWriteAtVolatile
+    + PunchHole
+    + Seek
+    + WriteZeroes
+    + Send
+    + AsRawFds
 {
 }
 impl<
         D: FileSetLen
             + FileSync
             + PunchHole
-            + FileReadWriteVolatile
+            + FileReadWriteAtVolatile
             + Seek
             + WriteZeroes
             + Send
diff --git a/seccomp/arm/block_device.policy b/seccomp/arm/block_device.policy
index 62f4ee7..1ec4053 100644
--- a/seccomp/arm/block_device.policy
+++ b/seccomp/arm/block_device.policy
@@ -10,6 +10,8 @@ fstat64: 1
 fsync: 1
 ftruncate64: 1
 _llseek: 1
+preadv: 1
+pwritev: 1
 timerfd_create: 1
 timerfd_gettime: 1
 timerfd_settime: 1
diff --git a/seccomp/x86_64/block_device.policy b/seccomp/x86_64/block_device.policy
index 20eca1a..6bb5c32 100644
--- a/seccomp/x86_64/block_device.policy
+++ b/seccomp/x86_64/block_device.policy
@@ -10,6 +10,8 @@ fstat: 1
 fsync: 1
 ftruncate: 1
 lseek: 1
+preadv: 1
+pwritev: 1
 timerfd_create: 1
 timerfd_gettime: 1
 timerfd_settime: 1