disk: qcow: fix write_zeroes with backing file

The qcow disk format implemented the discard and write zeroes requests
by deallocating the underlying cluster when possible, with the
assumption that a deallocated cluster would return all zeroes if it is
read until it is written again.

Add an alternate implementation of write_zeroes that is used when a
backing file is present that uses the straightforward approach of
allocating all clusters and zeroing them out in the raw file rather than
using deallocation.

BUG=b:171230599
TEST=BLKZEROOUT test case
TEST=cargo test -p disk

Change-Id: I745f6dc7aa411ec9b1be0150ba1bc96c011ada9a
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2490605
Reviewed-by: Dylan Reid <dgreid@chromium.org>
Reviewed-by: Cody Schuffelen <schuffelen@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>

disk/src/qcow/mod.rs

@@ -1251,7 +1251,7 @@ impl QcowFile {
     }
 
     // Deallocate the storage for the cluster starting at `address`.
-    // Any future reads of this cluster will return all zeroes.
+    // Any future reads of this cluster will return all zeroes (or the backing file, if in use).
     fn deallocate_cluster(&mut self, address: u64) -> std::io::Result<()> {
         if address >= self.virtual_size() as u64 {
             return Err(std::io::Error::from_raw_os_error(EINVAL));
@@ -1322,9 +1322,10 @@ impl QcowFile {
         Ok(())
     }
 
-    // Deallocate the storage for `length` bytes starting at `address`.
+    // Fill a range of `length` bytes starting at `address` with zeroes.
     // Any future reads of this range will return all zeroes.
-    fn deallocate_bytes(&mut self, address: u64, length: usize) -> std::io::Result<()> {
+    // If there is no backing file, this will deallocate cluster storage when possible.
+    fn zero_bytes(&mut self, address: u64, length: usize) -> std::io::Result<()> {
         let write_count: usize = self.limit_range_file(address, length);
 
         let mut nwritten: usize = 0;
@@ -1332,14 +1333,22 @@ impl QcowFile {
             let curr_addr = address + nwritten as u64;
             let count = self.limit_range_cluster(curr_addr, write_count - nwritten);
 
-            if count == self.raw_file.cluster_size() as usize {
-                // Full cluster - deallocate the storage.
+            if self.backing_file.is_none() && count == self.raw_file.cluster_size() as usize {
+                // Full cluster and no backing file in use - deallocate the storage.
                 self.deallocate_cluster(curr_addr)?;
             } else {
-                // Partial cluster - zero out the relevant bytes if it was allocated.
-                // Any space in unallocated clusters can be left alone, since
-                // unallocated clusters already read back as zeroes.
-                if let Some(offset) = self.file_offset_read(curr_addr)? {
+                // Partial cluster - zero out the relevant bytes.
+                let offset = if self.backing_file.is_some() {
+                    // There is a backing file, so we need to allocate a cluster in order to
+                    // zero out the hole-punched bytes such that the backing file contents do not
+                    // show through.
+                    Some(self.file_offset_write(curr_addr)?)
+                } else {
+                    // Any space in unallocated clusters can be left alone, since
+                    // unallocated clusters already read back as zeroes.
+                    self.file_offset_read(curr_addr)?
+                };
+                if let Some(offset) = offset {
                     // Partial cluster - zero it out.
                     self.raw_file
                         .file_mut()
@@ -1695,7 +1704,7 @@ impl PunchHole for QcowFile {
         let mut offset = offset;
         while remaining > 0 {
             let chunk_length = min(remaining, std::usize::MAX as u64) as usize;
-            self.deallocate_bytes(offset, chunk_length)?;
+            self.zero_bytes(offset, chunk_length)?;
             remaining -= chunk_length as u64;
             offset += chunk_length as u64;
         }
@@ -2083,6 +2092,39 @@ mod tests {
         });
     }
 
+    #[test]
+    fn write_zeroes_backing() {
+        let disk_file = basic_file(&valid_header());
+        let mut backing = QcowFile::from(disk_file).unwrap();
+        // Write some test data.
+        let b = [0x55u8; 0x1000];
+        backing
+            .seek(SeekFrom::Start(0xfff2000))
+            .expect("Failed to seek.");
+        backing.write(&b).expect("Failed to write test string.");
+        let wrapping_disk_file = basic_file(&valid_header());
+        let mut wrapping = QcowFile::from(wrapping_disk_file).unwrap();
+        wrapping.set_backing_file(Some(Box::new(backing)));
+        // Overwrite the test data with zeroes.
+        // This should allocate new clusters in the wrapping file so that they can be zeroed.
+        wrapping
+            .seek(SeekFrom::Start(0xfff2000))
+            .expect("Failed to seek.");
+        wrapping
+            .write_zeroes_all(0x200)
+            .expect("Failed to write zeroes.");
+        // Verify that the correct part of the data was zeroed out.
+        let mut buf = [0u8; 0x1000];
+        wrapping
+            .seek(SeekFrom::Start(0xfff2000))
+            .expect("Failed to seek.");
+        wrapping.read(&mut buf).expect("Failed to read.");
+        assert_eq!(buf[0], 0);
+        assert_eq!(buf[0x1FF], 0);
+        assert_eq!(buf[0x200], 0x55);
+        assert_eq!(buf[0xFFF], 0x55);
+    }
+
     #[test]
     fn test_header() {
         with_basic_file(&valid_header(), |disk_file: File| {