Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6661,6 +6661,7 @@ dependencies = [
"async-trait",
"disk_backend",
"disk_prwrap",
"disklayer_ram",
"futures",
"getrandom 0.3.3",
"guestmem",
Expand Down Expand Up @@ -8993,7 +8994,6 @@ dependencies = [
"mesh",
"pal_async",
"pal_event",
"parking_lot",
"scsi_buffers",
"task_control",
"test_with_tracing",
Expand Down
1 change: 1 addition & 0 deletions vm/devices/storage/scsidisk/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ getrandom.workspace = true

[dev-dependencies]
disk_prwrap.workspace = true
disklayer_ram.workspace = true
test_with_tracing.workspace = true

[lints]
Expand Down
137 changes: 21 additions & 116 deletions vm/devices/storage/scsidisk/src/scsidvd/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::executor::block_on;
use guestmem::GuestMemory;
use guestmem::MemoryWrite;
use inspect::Inspect;
use pal_async::async_test;
use scsi::AdditionalSenseCode;
use scsi::ScsiOp;
Expand All @@ -2399,117 +2395,26 @@ 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 {
// The disk is always created writable so the test can pre-fill it with
// pattern data. The SCSI DVD layer enforces read-only at the command
// level; these tests exercise DVD protocol behavior, not write
// protection.
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);
block_on(disk.write_vectored(&buffers.buffer(&mem), 0, false)).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");
Expand Down Expand Up @@ -2638,14 +2543,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();
Expand Down Expand Up @@ -2680,14 +2585,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,
Expand Down
1 change: 0 additions & 1 deletion vm/devices/virtio/virtio_blk/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
125 changes: 3 additions & 122 deletions vm/devices/virtio/virtio_blk/src/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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();

Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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
Expand Down
Loading