devices: virtio: ensure all block data is transferred

Add _exact/_all variants of the FileReadWriteAtVolatile functions on
descriptor Reader/Writer, and use them in the block device to replace
the short read/short write error cases.  This ensures all data is
read/written even if the underlying implementation (in particular,
qcow2) does not transfer the full amount of data in one
read_vectored_at_volatile/write_vectored_at_volatile call.

BUG=chromium:1023422
TEST=`mkfs.btrfs /dev/vdb` with a qcow2 disk

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

devices/src/virtio/block.rs

@@ -141,11 +141,6 @@ enum ExecuteError {
         sector: u64,
         desc_error: io::Error,
     },
-    ShortRead {
-        sector: u64,
-        expected_length: usize,
-        actual_length: usize,
-    },
     Seek {
         ioerr: io::Error,
         sector: u64,
@@ -156,11 +151,6 @@ enum ExecuteError {
         sector: u64,
         desc_error: io::Error,
     },
-    ShortWrite {
-        sector: u64,
-        expected_length: usize,
-        actual_length: usize,
-    },
     DiscardWriteZeroes {
         ioerr: Option<io::Error>,
         sector: u64,
@@ -193,15 +183,6 @@ impl Display for ExecuteError {
                 "io error reading {} bytes from sector {}: {}",
                 length, sector, desc_error,
             ),
-            ShortRead {
-                sector,
-                expected_length,
-                actual_length,
-            } => write!(
-                f,
-                "short read: {} bytes of {} at sector {}",
-                actual_length, expected_length, sector
-            ),
             Seek { ioerr, sector } => write!(f, "failed to seek to sector {}: {}", sector, ioerr),
             TimerFd(e) => write!(f, "{}", e),
             WriteIo {
@@ -213,15 +194,6 @@ impl Display for ExecuteError {
                 "io error writing {} bytes to sector {}: {}",
                 length, sector, desc_error,
             ),
-            ShortWrite {
-                sector,
-                expected_length,
-                actual_length,
-            } => write!(
-                f,
-                "short write: {} bytes of {} at sector {}",
-                actual_length, expected_length, sector
-            ),
             DiscardWriteZeroes {
                 ioerr: Some(ioerr),
                 sector,
@@ -258,11 +230,9 @@ impl ExecuteError {
             ExecuteError::WriteStatus(_) => VIRTIO_BLK_S_IOERR,
             ExecuteError::Flush(_) => VIRTIO_BLK_S_IOERR,
             ExecuteError::ReadIo { .. } => VIRTIO_BLK_S_IOERR,
-            ExecuteError::ShortRead { .. } => VIRTIO_BLK_S_IOERR,
             ExecuteError::Seek { .. } => VIRTIO_BLK_S_IOERR,
             ExecuteError::TimerFd(_) => VIRTIO_BLK_S_IOERR,
             ExecuteError::WriteIo { .. } => VIRTIO_BLK_S_IOERR,
-            ExecuteError::ShortWrite { .. } => VIRTIO_BLK_S_IOERR,
             ExecuteError::DiscardWriteZeroes { .. } => VIRTIO_BLK_S_IOERR,
             ExecuteError::ReadOnly { .. } => VIRTIO_BLK_S_IOERR,
             ExecuteError::OutOfRange { .. } => VIRTIO_BLK_S_IOERR,
@@ -613,21 +583,13 @@ impl Block {
                     .checked_shl(u32::from(SECTOR_SHIFT))
                     .ok_or(ExecuteError::OutOfRange)?;
                 check_range(offset, data_len as u64, disk_size)?;
-                let actual_length =
                 writer
-                        .write_from_at(disk, data_len, offset)
+                    .write_all_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,
-                        expected_length: data_len,
-                        actual_length,
-                    });
-                }
             }
             VIRTIO_BLK_T_OUT => {
                 let data_len = reader.available_bytes();
@@ -635,21 +597,13 @@ impl Block {
                     .checked_shl(u32::from(SECTOR_SHIFT))
                     .ok_or(ExecuteError::OutOfRange)?;
                 check_range(offset, data_len as u64, disk_size)?;
-                let actual_length =
                 reader
-                        .read_to_at(disk, data_len, offset)
+                    .read_exact_to_at(disk, data_len, offset)
                     .map_err(|desc_error| ExecuteError::WriteIo {
                         length: data_len,
                         sector,
                         desc_error,
                     })?;
-                if actual_length < data_len {
-                    return Err(ExecuteError::ShortWrite {
-                        sector,
-                        expected_length: data_len,
-                        actual_length,
-                    });
-                }
                 if !*flush_timer_armed {
                     flush_timer
                         .reset(flush_delay, None)

devices/src/virtio/descriptor_utils.rs

@@ -309,6 +309,32 @@ impl<'a> Reader<'a> {
         Ok(())
     }
 
+    pub fn read_exact_to_at<F: FileReadWriteAtVolatile>(
+        &mut self,
+        mut dst: F,
+        mut count: usize,
+        mut off: u64,
+    ) -> io::Result<()> {
+        while count > 0 {
+            match self.read_to_at(&mut dst, count, off) {
+                Ok(0) => {
+                    return Err(io::Error::new(
+                        io::ErrorKind::UnexpectedEof,
+                        "failed to fill whole buffer",
+                    ))
+                }
+                Ok(n) => {
+                    count -= n;
+                    off += n as u64;
+                }
+                Err(ref e) if e.kind() == io::ErrorKind::Interrupted => {}
+                Err(e) => return Err(e),
+            }
+        }
+
+        Ok(())
+    }
+
     /// Returns number of bytes available for reading.  May return an error if the combined
     /// lengths of all the buffers in the DescriptorChain would cause an integer overflow.
     pub fn available_bytes(&self) -> usize {
@@ -460,6 +486,31 @@ impl<'a> Writer<'a> {
         Ok(())
     }
 
+    pub fn write_all_from_at<F: FileReadWriteAtVolatile>(
+        &mut self,
+        mut src: F,
+        mut count: usize,
+        mut off: u64,
+    ) -> io::Result<()> {
+        while count > 0 {
+            match self.write_from_at(&mut src, count, off) {
+                Ok(0) => {
+                    return Err(io::Error::new(
+                        io::ErrorKind::WriteZero,
+                        "failed to write whole buffer",
+                    ))
+                }
+                Ok(n) => {
+                    count -= n;
+                    off += n as u64;
+                }
+                Err(ref e) if e.kind() == io::ErrorKind::Interrupted => {}
+                Err(e) => return Err(e),
+            }
+        }
+        Ok(())
+    }
+
     /// Returns number of bytes already written to the descriptor chain buffer.
     pub fn bytes_written(&self) -> usize {
         self.buffer.bytes_consumed()