Replace redundant test disk implementations with RAM disk backend

Co-authored-by: jstarks <9548354+jstarks@users.noreply.github.com>

Author: Copilot

Cargo.lock

@@ -6661,6 +6661,7 @@ dependencies = [
  "async-trait",
  "disk_backend",
  "disk_prwrap",
+ "disklayer_ram",
  "futures",
  "getrandom 0.3.3",
  "guestmem",
@@ -8993,7 +8994,6 @@ dependencies = [
  "mesh",
  "pal_async",
  "pal_event",
- "parking_lot",
  "scsi_buffers",
  "task_control",
  "test_with_tracing",

vm/devices/storage/scsidisk/Cargo.toml

@@ -37,6 +37,7 @@ getrandom.workspace = true
 
 [dev-dependencies]
 disk_prwrap.workspace = true
+disklayer_ram.workspace = true
 test_with_tracing.workspace = true
 
 [lints]

vm/devices/storage/scsidisk/src/scsidvd/mod.rs

@@ -2381,12 +2381,8 @@ mod tests {
     use crate::scsi;
     use crate::scsidvd::ISO_SECTOR_SIZE;
     use crate::scsidvd::SimpleScsiDvd;
-    use disk_backend::Disk;
-    use disk_backend::DiskError;
-    use disk_backend::DiskIo;
+    use futures::FutureExt;
     use guestmem::GuestMemory;
-    use guestmem::MemoryWrite;
-    use inspect::Inspect;
     use pal_async::async_test;
     use scsi::AdditionalSenseCode;
     use scsi::ScsiOp;
@@ -2399,117 +2395,25 @@ mod tests {
     use test_with_tracing::test;
     use zerocopy::IntoBytes;
 
-    #[derive(Debug)]
-    struct TestDisk {
-        sector_count: u64,
-        sector_size: u32,
-        storage: Vec<u8>,
-        read_only: bool,
-    }
-
-    impl Inspect for TestDisk {
-        fn inspect(&self, req: inspect::Request<'_>) {
-            req.respond();
-        }
-    }
-
-    impl TestDisk {
-        pub fn new(sector_size: u32, sector_count: u64, read_only: bool) -> TestDisk {
-            let buffer = make_repeat_data_buffer(sector_count as usize, sector_size as usize);
-
-            TestDisk {
-                sector_count,
-                sector_size,
-                storage: buffer,
-                read_only,
-            }
-        }
-    }
-
-    impl DiskIo for TestDisk {
-        fn disk_type(&self) -> &str {
-            "test"
-        }
-
-        fn sector_count(&self) -> u64 {
-            self.sector_count
-        }
-
-        fn sector_size(&self) -> u32 {
-            self.sector_size
-        }
-
-        fn is_read_only(&self) -> bool {
-            self.read_only
-        }
-
-        fn disk_id(&self) -> Option<[u8; 16]> {
-            None
-        }
-
-        fn physical_sector_size(&self) -> u32 {
-            self.sector_size
-        }
-
-        fn is_fua_respected(&self) -> bool {
-            false
-        }
-
-        async fn eject(&self) -> Result<(), DiskError> {
-            Err(DiskError::UnsupportedEject)
-        }
-
-        async fn read_vectored(
-            &self,
-            buffers: &RequestBuffers<'_>,
-            sector: u64,
-        ) -> Result<(), DiskError> {
-            let offset = sector as usize * self.sector_size() as usize;
-            let end_point = offset + buffers.len();
-
-            if self.storage.len() < end_point {
-                return Err(DiskError::IllegalBlock);
-            }
-
-            buffers.writer().write(&self.storage[offset..end_point])?;
-            Ok(())
-        }
-
-        async fn write_vectored(
-            &self,
-            _buffers: &RequestBuffers<'_>,
-            _sector: u64,
-            _fua: bool,
-        ) -> Result<(), DiskError> {
-            todo!()
-        }
-
-        async fn sync_cache(&self) -> Result<(), DiskError> {
-            todo!()
-        }
-
-        async fn unmap(
-            &self,
-            _sector: u64,
-            _count: u64,
-            _block_level_only: bool,
-        ) -> Result<(), DiskError> {
-            Ok(())
-        }
-
-        fn unmap_behavior(&self) -> disk_backend::UnmapBehavior {
-            disk_backend::UnmapBehavior::Ignored
-        }
-    }
-
-    fn new_scsi_dvd(sector_size: u32, sector_count: u64, read_only: bool) -> SimpleScsiDvd {
-        let disk = TestDisk::new(sector_size, sector_count, read_only);
-        let scsi_dvd = SimpleScsiDvd::new(Some(Disk::new(disk).unwrap()));
+    fn new_scsi_dvd(sector_size: u32, sector_count: u64) -> SimpleScsiDvd {
+        let disk_size = sector_count * sector_size as u64;
+        let disk = disklayer_ram::ram_disk_with_sector_size(disk_size, false, sector_size).unwrap();
+
+        // Pre-fill the disk with the repeating pattern expected by tests.
+        let pattern = make_repeat_data_buffer(sector_count as usize, sector_size as usize);
+        let mem = GuestMemory::allocate(pattern.len());
+        mem.write_at(0, &pattern).unwrap();
+        let buffers = OwnedRequestBuffers::linear(0, pattern.len(), false);
+        disk.write_vectored(&buffers.buffer(&mem), 0, false)
+            .now_or_never()
+            .expect("RAM disk write should not block")
+            .unwrap();
+
+        let scsi_dvd = SimpleScsiDvd::new(Some(disk));
         let sector_shift = ISO_SECTOR_SIZE.trailing_zeros() as u8;
         assert_eq!(scsi_dvd.sector_count(), sector_count / scsi_dvd.balancer());
         assert_eq!(scsi_dvd.sector_shift(), sector_shift);
         if let Media::Loaded(disk) = &*scsi_dvd.media.read() {
-            assert_eq!(disk.is_read_only(), read_only);
             assert_eq!(disk.sector_size(), sector_size);
         } else {
             panic!("unexpected Media::Unloaded");
@@ -2638,14 +2542,14 @@ mod tests {
 
     #[test]
     fn validate_new_scsi_dvd() {
-        new_scsi_dvd(512, 2048, true);
+        new_scsi_dvd(512, 2048);
     }
 
     #[async_test]
     async fn validate_read16() {
         let sector_size = 512;
         let sector_count = 2048;
-        let mut scsi_dvd = new_scsi_dvd(sector_size, sector_count, true);
+        let mut scsi_dvd = new_scsi_dvd(sector_size, sector_count);
 
         let dvd_sector_size = ISO_SECTOR_SIZE as u64;
         let dvd_sector_count = scsi_dvd.sector_count();
@@ -2680,14 +2584,14 @@ mod tests {
 
     #[test]
     fn validate_save_restore_scsi_dvd_no_change() {
-        let scsi_dvd = new_scsi_dvd(512, 2048, true);
+        let scsi_dvd = new_scsi_dvd(512, 2048);
         let saved_state = save_scsi_dvd(&scsi_dvd);
         restore_scsi_dvd(saved_state, &scsi_dvd);
     }
 
     #[test]
     fn validate_save_restore_scsi_dvd_with_sense_data() {
-        let scsi_dvd = new_scsi_dvd(512, 2048, true);
+        let scsi_dvd = new_scsi_dvd(512, 2048);
         let mut saved_state = save_scsi_dvd(&scsi_dvd);
         saved_state.sense_data = Some(SavedSenseData {
             sense_key: SenseKey::UNIT_ATTENTION.0,

vm/devices/virtio/virtio_blk/Cargo.toml

@@ -32,7 +32,6 @@ disklayer_ram.workspace = true
 mesh.workspace = true
 pal_async.workspace = true
 pal_event.workspace = true
-parking_lot.workspace = true
 test_with_tracing.workspace = true
 
 [lints]

vm/devices/virtio/virtio_blk/src/integration_tests.rs

@@ -12,18 +12,11 @@ use crate::spec::VirtioBlkDiscardWriteZeroes;
 use crate::spec::*;
 use core::mem::offset_of;
 use disk_backend::Disk;
-use disk_backend::DiskError;
-use disk_backend::DiskIo;
 use guestmem::GuestMemory;
-use guestmem::MemoryRead;
-use guestmem::MemoryWrite;
-use inspect::Inspect;
 use pal_async::DefaultDriver;
 use pal_async::async_test;
 use pal_async::wait::PolledWait;
 use pal_event::Event;
-use parking_lot::Mutex;
-use scsi_buffers::RequestBuffers;
 use std::time::Duration;
 use test_with_tracing::test;
 use virtio::QueueResources;
@@ -828,118 +821,6 @@ async fn sector_offset_correctness(driver: DefaultDriver) {
     assert!(buf.iter().all(|&b| b == 0), "sector 9 should be zeroes");
 }
 
-// --- 4K-sector test disk ---
-
-/// A simple in-memory disk with configurable sector size, used to test the
-/// sector shift conversion path with non-512-byte sectors.
-#[derive(Inspect)]
-struct TestDisk4K {
-    sector_size: u32,
-    #[inspect(skip)]
-    storage: Mutex<Vec<u8>>,
-    #[inspect(skip)]
-    supports_discard: bool,
-}
-
-impl TestDisk4K {
-    fn new(total_bytes: usize, sector_size: u32) -> Self {
-        assert!(sector_size.is_power_of_two() && sector_size >= 512);
-        assert_eq!(total_bytes % sector_size as usize, 0);
-        Self {
-            sector_size,
-            storage: Mutex::new(vec![0u8; total_bytes]),
-            supports_discard: false,
-        }
-    }
-
-    fn with_discard(mut self) -> Self {
-        self.supports_discard = true;
-        self
-    }
-}
-
-impl DiskIo for TestDisk4K {
-    fn disk_type(&self) -> &str {
-        "test-4k"
-    }
-
-    fn sector_count(&self) -> u64 {
-        self.storage.lock().len() as u64 / self.sector_size as u64
-    }
-
-    fn sector_size(&self) -> u32 {
-        self.sector_size
-    }
-
-    fn disk_id(&self) -> Option<[u8; 16]> {
-        None
-    }
-
-    fn physical_sector_size(&self) -> u32 {
-        self.sector_size
-    }
-
-    fn is_fua_respected(&self) -> bool {
-        false
-    }
-
-    fn is_read_only(&self) -> bool {
-        false
-    }
-
-    async fn read_vectored(
-        &self,
-        buffers: &RequestBuffers<'_>,
-        sector: u64,
-    ) -> Result<(), DiskError> {
-        let offset = sector as usize * self.sector_size as usize;
-        let end = offset + buffers.len();
-        let storage = self.storage.lock();
-        if end > storage.len() {
-            return Err(DiskError::IllegalBlock);
-        }
-        buffers.writer().write(&storage[offset..end])?;
-        Ok(())
-    }
-
-    async fn write_vectored(
-        &self,
-        buffers: &RequestBuffers<'_>,
-        sector: u64,
-        _fua: bool,
-    ) -> Result<(), DiskError> {
-        let offset = sector as usize * self.sector_size as usize;
-        let end = offset + buffers.len();
-        let mut storage = self.storage.lock();
-        if end > storage.len() {
-            return Err(DiskError::IllegalBlock);
-        }
-        buffers.reader().read(&mut storage[offset..end])?;
-        Ok(())
-    }
-
-    async fn sync_cache(&self) -> Result<(), DiskError> {
-        Ok(())
-    }
-
-    async fn unmap(
-        &self,
-        _sector: u64,
-        _count: u64,
-        _block_level_only: bool,
-    ) -> Result<(), DiskError> {
-        Ok(())
-    }
-
-    fn unmap_behavior(&self) -> disk_backend::UnmapBehavior {
-        if self.supports_discard {
-            disk_backend::UnmapBehavior::Unspecified
-        } else {
-            disk_backend::UnmapBehavior::Ignored
-        }
-    }
-}
-
 // --- Sector shift regression tests ---
 
 /// Write and read via a 4096-byte-sector disk to exercise the sector shift
@@ -954,7 +835,7 @@ impl DiskIo for TestDisk4K {
 #[async_test]
 async fn write_read_4k_sector_disk(driver: DefaultDriver) {
     // 64 KiB disk with 4096-byte sectors → 16 disk sectors.
-    let disk = Disk::new(TestDisk4K::new(64 * 1024, 4096)).unwrap();
+    let disk = disklayer_ram::ram_disk_with_sector_size(64 * 1024, false, 4096).unwrap();
     let mut harness = TestHarness::new(&driver, disk, false);
     harness.enable();
 
@@ -992,7 +873,7 @@ async fn write_read_4k_sector_disk(driver: DefaultDriver) {
 #[async_test]
 async fn sector_shift_multiple_offsets_4k(driver: DefaultDriver) {
     // 128 KiB disk with 4096-byte sectors → 32 disk sectors.
-    let disk = Disk::new(TestDisk4K::new(128 * 1024, 4096)).unwrap();
+    let disk = disklayer_ram::ram_disk_with_sector_size(128 * 1024, false, 4096).unwrap();
     let mut harness = TestHarness::new(&driver, disk, false);
     harness.enable();
 
@@ -1055,7 +936,7 @@ async fn check_discard(
 }
 
 fn test_disk_4k_discard() -> Disk {
-    Disk::new(TestDisk4K::new(64 * 1024, 4096).with_discard()).unwrap()
+    disklayer_ram::ram_disk_with_sector_size(64 * 1024, false, 4096).unwrap()
 }
 
 /// Discard with properly aligned sector and num_sectors on a 4K disk