diff --git a/block/src/aligned_file.rs b/block/src/aligned_file.rs index 4f63a34ab..1a30cd938 100644 --- a/block/src/aligned_file.rs +++ b/block/src/aligned_file.rs @@ -68,6 +68,13 @@ impl AlignedFile { alignment: self.alignment, }) } + + /// Wrap `file` with an explicit alignment, bypassing the probe. Used by + /// tests to force the bounce/RMW path without a real O_DIRECT fd. + #[cfg(test)] + pub fn with_alignment(file: File, alignment: usize) -> Self { + AlignedFile { file, alignment } + } } impl FileExt for AlignedFile { diff --git a/block/src/factory.rs b/block/src/factory.rs index f1db7973a..e3518ff1b 100644 --- a/block/src/factory.rs +++ b/block/src/factory.rs @@ -114,7 +114,7 @@ fn open_vhdx( ) -> BlockResult> { info!("Opening VHDX disk file with synchronous backend"); Ok(Box::new( - VhdxDisk::new(file).map_err(|e| e.with_path(options.path))?, + VhdxDisk::new(file, options.direct).map_err(|e| e.with_path(options.path))?, )) } diff --git a/block/src/formats/vhdx/internal/bat.rs b/block/src/formats/vhdx/internal/bat.rs index 826e5ea31..9867bf976 100644 --- a/block/src/formats/vhdx/internal/bat.rs +++ b/block/src/formats/vhdx/internal/bat.rs @@ -2,17 +2,17 @@ // // SPDX-License-Identifier: Apache-2.0 -use std::fs::File; -use std::io::{self, Seek, SeekFrom}; use std::mem::size_of; -use std::result; +use std::os::unix::fs::FileExt; +use std::{io, result}; -use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; +use byteorder::{ByteOrder, LittleEndian}; use remain::sorted; use thiserror::Error; use super::header::RegionTableEntry; use super::metadata::DiskSpec; +use crate::aligned_file::AlignedFile; // Payload BAT Entry States pub const PAYLOAD_BLOCK_NOT_PRESENT: u64 = 0; @@ -48,7 +48,7 @@ pub struct BatEntry(pub u64); impl BatEntry { // Read all BAT entries presented on the disk and insert them to a vector pub fn collect_bat_entries( - f: &mut File, + f: &AlignedFile, disk_spec: &DiskSpec, bat_entry: &RegionTableEntry, ) -> Result> { @@ -64,14 +64,10 @@ impl BatEntry { let mut bat: Vec = Vec::with_capacity(bat_entry.length as usize); let offset = bat_entry.file_offset; for i in 0..entry_count { - f.seek(SeekFrom::Start(offset + i * size_of::() as u64)) + let mut entry = [0u8; size_of::()]; + f.read_exact_at(&mut entry, offset + i * size_of::() as u64) .map_err(VhdxBatError::ReadBat)?; - - let bat_entry = BatEntry( - f.read_u64::() - .map_err(VhdxBatError::ReadBat)?, - ); - bat.insert(i as usize, bat_entry); + bat.insert(i as usize, BatEntry(LittleEndian::read_u64(&entry))); } Ok(bat) @@ -85,13 +81,11 @@ impl BatEntry { // Routine for writing BAT entries to the disk pub fn write_bat_entries( - f: &mut File, + f: &AlignedFile, bat_offset: u64, bat_entries: &[BatEntry], ) -> Result<()> { for i in 0..bat_entries.len() as u64 { - f.seek(SeekFrom::Start(bat_offset + i * size_of::() as u64)) - .map_err(VhdxBatError::WriteBat)?; let bat_entry = match bat_entries.get(i as usize) { Some(entry) => entry.0, None => { @@ -99,7 +93,9 @@ impl BatEntry { } }; - f.write_u64::(bat_entry) + let mut buf = [0u8; size_of::()]; + LittleEndian::write_u64(&mut buf, bat_entry); + f.write_all_at(&buf, bat_offset + i * size_of::() as u64) .map_err(VhdxBatError::WriteBat)?; } Ok(()) diff --git a/block/src/formats/vhdx/internal/header.rs b/block/src/formats/vhdx/internal/header.rs index 0882d3a33..1743ffca2 100644 --- a/block/src/formats/vhdx/internal/header.rs +++ b/block/src/formats/vhdx/internal/header.rs @@ -3,16 +3,17 @@ // SPDX-License-Identifier: Apache-2.0 use std::collections::btree_map::BTreeMap; -use std::fs::File; -use std::io::{self, Read, Seek, SeekFrom, Write}; use std::mem::size_of; -use std::{result, slice}; +use std::os::unix::fs::FileExt; +use std::{io, result, slice}; -use byteorder::{ByteOrder, LittleEndian, ReadBytesExt}; +use byteorder::{ByteOrder, LittleEndian}; use remain::sorted; use thiserror::Error; use uuid::Uuid; +use crate::aligned_file::AlignedFile; + const VHDX_SIGN: u64 = 0x656C_6966_7864_6876; // "vhdxfile" const HEADER_SIGN: u32 = 0x6461_6568; // "head" const REGION_SIGN: u32 = 0x6967_6572; // "regi" @@ -72,14 +73,6 @@ pub enum VhdxHeaderError { RegionOverlap, #[error("Reserved region has non-zero value")] ReservedIsNonZero, - #[error("Failed to seek in File Type Identifier {0}")] - SeekFileTypeIdentifier(#[source] io::Error), - #[error("Failed to seek in headers {0}")] - SeekHeader(#[source] io::Error), - #[error("Failed to seek in region table entries {0}")] - SeekRegionTableEntries(#[source] io::Error), - #[error("Failed to seek in region table header {0}")] - SeekRegionTableHeader(#[source] io::Error), #[error("We do not recognize this entry")] UnrecognizedRegionEntry, #[error("Failed to write header {0}")] @@ -95,12 +88,11 @@ pub struct FileTypeIdentifier { impl FileTypeIdentifier { /// Reads the File Type Identifier structure from a reference VHDx file - pub fn new(f: &mut File) -> Result { - f.seek(SeekFrom::Start(FILE_START)) - .map_err(VhdxHeaderError::SeekFileTypeIdentifier)?; - let _signature = f - .read_u64::() + pub fn new(f: &AlignedFile) -> Result { + let mut buf = [0u8; size_of::()]; + f.read_exact_at(&mut buf, FILE_START) .map_err(VhdxHeaderError::ReadFileTypeIdentifier)?; + let _signature = LittleEndian::read_u64(&buf); if _signature != VHDX_SIGN { return Err(VhdxHeaderError::InvalidVHDXSign); } @@ -126,13 +118,11 @@ pub struct Header { impl Header { /// Reads the Header structure from a reference VHDx file - pub fn new(f: &mut File, start: u64) -> Result
{ + pub fn new(f: &AlignedFile, start: u64) -> Result
{ // Read the whole header into a buffer. We will need it for // calculating checksum. let mut buffer = [0; HEADER_SIZE as usize]; - f.seek(SeekFrom::Start(start)) - .map_err(VhdxHeaderError::SeekHeader)?; - f.read_exact(&mut buffer) + f.read_exact_at(&mut buffer, start) .map_err(VhdxHeaderError::ReadHeader)?; // SAFETY: buffer is of correct size and has been successfully filled. @@ -159,7 +149,7 @@ impl Header { /// Creates and returns new updated header from the provided current header fn update_header( - f: &mut File, + f: &AlignedFile, current_header: &Header, change_data_guid: bool, mut file_write_guid: u128, @@ -193,9 +183,8 @@ impl Header { new_header.checksum = calculate_checksum(&mut buffer, size_of::()); new_header.write_to_buffer(&mut buffer); - f.seek(SeekFrom::Start(start)) - .map_err(VhdxHeaderError::SeekHeader)?; - f.write(&buffer).map_err(VhdxHeaderError::WriteHeader)?; + f.write_all_at(&buffer, start) + .map_err(VhdxHeaderError::WriteHeader)?; Ok(new_header) } @@ -212,13 +201,11 @@ struct RegionTableHeader { impl RegionTableHeader { /// Reads the Region Table Header structure from a reference VHDx file - pub fn new(f: &mut File, start: u64) -> Result { + pub fn new(f: &AlignedFile, start: u64) -> Result { // Read the whole header into a buffer. We will need it for calculating // checksum. let mut buffer = [0u8; REGION_SIZE as usize]; - f.seek(SeekFrom::Start(start)) - .map_err(VhdxHeaderError::SeekRegionTableHeader)?; - f.read_exact(&mut buffer) + f.read_exact_at(&mut buffer, start) .map_err(VhdxHeaderError::ReadRegionTableHeader)?; // SAFETY: buffer is of correct size and has been successfully filled. @@ -253,7 +240,7 @@ pub struct RegionInfo { impl RegionInfo { /// Collect all entries in a BTreeMap from the Region Table and identifies /// BAT and metadata regions - pub fn new(f: &mut File, region_start: u64, entry_count: u32) -> Result { + pub fn new(f: &AlignedFile, region_start: u64, entry_count: u32) -> Result { let mut bat_entry: Option = None; let mut mdr_entry: Option = None; @@ -261,13 +248,12 @@ impl RegionInfo { let mut region_entries = BTreeMap::new(); let mut buffer = [0; REGION_SIZE as usize]; - // Seek after the Region Table Header - f.seek(SeekFrom::Start( + // Read after the Region Table Header + f.read_exact_at( + &mut buffer, region_start + size_of::() as u64, - )) - .map_err(VhdxHeaderError::SeekRegionTableEntries)?; - f.read_exact(&mut buffer) - .map_err(VhdxHeaderError::ReadRegionTableEntries)?; + ) + .map_err(VhdxHeaderError::ReadRegionTableEntries)?; for _ in 0..entry_count { let entry = @@ -366,7 +352,7 @@ pub struct VhdxHeader { impl VhdxHeader { /// Creates a VhdxHeader from a reference to a file - pub fn new(f: &mut File) -> Result { + pub fn new(f: &AlignedFile) -> Result { Ok(VhdxHeader { _file_type_identifier: FileTypeIdentifier::new(f)?, header_1: Header::new(f, HEADER_1_START)?, @@ -403,7 +389,7 @@ impl VhdxHeader { /// current one. Returns both headers as a tuple sequenced the way it was /// received from the parameter list. fn update_header( - f: &mut File, + f: &AlignedFile, header_1: Result
, header_2: Result
, guid: u128, @@ -426,7 +412,7 @@ impl VhdxHeader { // Update the provided headers according to the spec fn update_headers( - f: &mut File, + f: &AlignedFile, header_1: Result
, header_2: Result
, guid: u128, @@ -436,7 +422,7 @@ impl VhdxHeader { VhdxHeader::update_header(f, Ok(header_1), Ok(header_2), guid) } - pub fn update(&mut self, f: &mut File) -> Result<()> { + pub fn update(&mut self, f: &AlignedFile) -> Result<()> { let headers = VhdxHeader::update_headers(f, Ok(self.header_1), Ok(self.header_2), 0)?; self.header_1 = headers.0; self.header_2 = headers.1; diff --git a/block/src/formats/vhdx/internal/io.rs b/block/src/formats/vhdx/internal/io.rs index 924355f38..4909e607f 100644 --- a/block/src/formats/vhdx/internal/io.rs +++ b/block/src/formats/vhdx/internal/io.rs @@ -2,15 +2,15 @@ // // SPDX-License-Identifier: Apache-2.0 -use std::fs::File; -use std::io::{self, Read, Seek, SeekFrom, Write}; -use std::result; +use std::os::unix::fs::FileExt; +use std::{io, result}; use remain::sorted; use thiserror::Error; use super::bat::{self, BatEntry, VhdxBatError}; use super::metadata::{self, DiskSpec}; +use crate::aligned_file::AlignedFile; const SECTOR_SIZE: u64 = 512; @@ -87,7 +87,7 @@ impl Sector { /// VHDx IO read routine: requires relative sector index and count for the /// requested data. pub fn read( - f: &mut File, + f: &AlignedFile, buf: &mut [u8], disk_spec: &DiskSpec, bat: &[BatEntry], @@ -115,11 +115,10 @@ pub fn read( | bat::PAYLOAD_BLOCK_UNMAPPED | bat::PAYLOAD_BLOCK_ZERO => {} bat::PAYLOAD_BLOCK_FULLY_PRESENT => { - f.seek(SeekFrom::Start(sector.file_offset)) - .map_err(VhdxIoError::ReadSectorBlock)?; - f.read_exact( + f.read_exact_at( &mut buf [read_count..(read_count + (sector.free_sectors * SECTOR_SIZE) as usize)], + sector.file_offset, ) .map_err(VhdxIoError::ReadSectorBlock)?; } @@ -140,7 +139,7 @@ pub fn read( /// VHDx IO write routine: requires relative sector index and count for the /// requested data. pub fn write( - f: &mut File, + f: &AlignedFile, buf: &[u8], disk_spec: &mut DiskSpec, bat_offset: u64, @@ -173,7 +172,9 @@ pub fn write( .checked_add(disk_spec.block_size as u64) .ok_or(VhdxIoError::InvalidDiskSize)?; - f.set_len(new_size).map_err(VhdxIoError::ResizeFile)?; + f.file() + .set_len(new_size) + .map_err(VhdxIoError::ResizeFile)?; disk_spec.image_size = new_size; let new_bat_entry = @@ -185,10 +186,9 @@ pub fn write( break; } - f.seek(SeekFrom::Start(file_offset)) - .map_err(VhdxIoError::ReadSectorBlock)?; - f.write_all( + f.write_all_at( &buf[write_count..(write_count + (sector.free_sectors * SECTOR_SIZE) as usize)], + file_offset, ) .map_err(VhdxIoError::ReadSectorBlock)?; } @@ -197,10 +197,9 @@ pub fn write( break; } - f.seek(SeekFrom::Start(sector.file_offset)) - .map_err(VhdxIoError::ReadSectorBlock)?; - f.write_all( + f.write_all_at( &buf[write_count..(write_count + (sector.free_sectors * SECTOR_SIZE) as usize)], + sector.file_offset, ) .map_err(VhdxIoError::ReadSectorBlock)?; } diff --git a/block/src/formats/vhdx/internal/metadata.rs b/block/src/formats/vhdx/internal/metadata.rs index 71cdc3284..211278e2a 100644 --- a/block/src/formats/vhdx/internal/metadata.rs +++ b/block/src/formats/vhdx/internal/metadata.rs @@ -2,17 +2,17 @@ // // SPDX-License-Identifier: Apache-2.0 -use std::fs::File; -use std::io::{self, Read, Seek, SeekFrom}; use std::mem::size_of; -use std::result; +use std::os::unix::fs::FileExt; +use std::{io, result}; -use byteorder::{LittleEndian, ReadBytesExt}; +use byteorder::{ByteOrder, LittleEndian}; use remain::sorted; use thiserror::Error; use uuid::Uuid; use super::header::RegionTableEntry; +use crate::aligned_file::AlignedFile; const METADATA_SIGN: u64 = 0x6174_6164_6174_656D; const METADATA_ENTRY_SIZE: usize = 32; @@ -103,17 +103,18 @@ pub struct DiskSpec { impl DiskSpec { /// Parse all metadata from the provided file and store info in DiskSpec /// structure. - pub fn new(f: &mut File, metadata_region: &RegionTableEntry) -> Result { + pub fn new(f: &AlignedFile, metadata_region: &RegionTableEntry) -> Result { let mut disk_spec = DiskSpec::default(); let mut metadata_presence: u16 = 0; let mut offset = 0; - let metadata = f.metadata().map_err(VhdxMetadataError::ReadMetadata)?; + let metadata = f + .file() + .metadata() + .map_err(VhdxMetadataError::ReadMetadata)?; disk_spec.image_size = metadata.len(); let mut buffer = [0u8; METADATA_TABLE_MAX_SIZE]; - f.seek(SeekFrom::Start(metadata_region.file_offset)) - .map_err(VhdxMetadataError::ReadMetadata)?; - f.read_exact(&mut buffer) + f.read_exact_at(&mut buffer, metadata_region.file_offset) .map_err(VhdxMetadataError::ReadMetadata)?; let metadata_header = @@ -124,18 +125,16 @@ impl DiskSpec { let metadata_entry = MetadataTableEntry::new(&buffer[offset..offset + size_of::()])?; - f.seek(SeekFrom::Start( - metadata_region.file_offset + metadata_entry.offset as u64, - )) - .map_err(VhdxMetadataError::ReadMetadata)?; + let item_offset = metadata_region.file_offset + metadata_entry.offset as u64; if metadata_entry.item_id == Uuid::parse_str(METADATA_FILE_PARAMETER) .map_err(VhdxMetadataError::InvalidUuid)? { - disk_spec.block_size = f - .read_u32::() + let mut item = [0u8; 2 * size_of::()]; + f.read_exact_at(&mut item, item_offset) .map_err(VhdxMetadataError::ReadMetadata)?; + disk_spec.block_size = LittleEndian::read_u32(&item[0..4]); // MUST be at least 1 MiB and not greater than 256 MiB if disk_spec.block_size < BLOCK_SIZE_MIN || disk_spec.block_size > BLOCK_SIZE_MAX { @@ -147,9 +146,7 @@ impl DiskSpec { return Err(VhdxMetadataError::InvalidBlockSize); } - let bits = f - .read_u32::() - .map_err(VhdxMetadataError::ReadMetadata)?; + let bits = LittleEndian::read_u32(&item[4..8]); disk_spec.has_parent = bits & BLOCK_HAS_PARENT != 0; metadata_presence |= METADATA_FILE_PARAMETER_PRESENT; @@ -157,27 +154,30 @@ impl DiskSpec { == Uuid::parse_str(METADATA_VIRTUAL_DISK_SIZE) .map_err(VhdxMetadataError::InvalidUuid)? { - disk_spec.virtual_disk_size = f - .read_u64::() + let mut item = [0u8; size_of::()]; + f.read_exact_at(&mut item, item_offset) .map_err(VhdxMetadataError::ReadMetadata)?; + disk_spec.virtual_disk_size = LittleEndian::read_u64(&item); metadata_presence |= METADATA_VIRTUAL_DISK_SIZE_PRESENT; } else if metadata_entry.item_id == Uuid::parse_str(METADATA_VIRTUAL_DISK_ID) .map_err(VhdxMetadataError::InvalidUuid)? { - disk_spec.disk_id = f - .read_u128::() + let mut item = [0u8; size_of::()]; + f.read_exact_at(&mut item, item_offset) .map_err(VhdxMetadataError::ReadMetadata)?; + disk_spec.disk_id = LittleEndian::read_u128(&item); metadata_presence |= METADATA_VIRTUAL_DISK_ID_PRESENT; } else if metadata_entry.item_id == Uuid::parse_str(METADATA_LOGICAL_SECTOR_SIZE) .map_err(VhdxMetadataError::InvalidUuid)? { - disk_spec.logical_sector_size = f - .read_u32::() + let mut item = [0u8; size_of::()]; + f.read_exact_at(&mut item, item_offset) .map_err(VhdxMetadataError::ReadMetadata)?; + disk_spec.logical_sector_size = LittleEndian::read_u32(&item); if !(disk_spec.logical_sector_size == 512 || disk_spec.logical_sector_size == 4096) { return Err(VhdxMetadataError::InvalidLogicalSectorSize); @@ -188,9 +188,10 @@ impl DiskSpec { == Uuid::parse_str(METADATA_PHYSICAL_SECTOR_SIZE) .map_err(VhdxMetadataError::InvalidUuid)? { - disk_spec.physical_sector_size = f - .read_u32::() + let mut item = [0u8; size_of::()]; + f.read_exact_at(&mut item, item_offset) .map_err(VhdxMetadataError::ReadMetadata)?; + disk_spec.physical_sector_size = LittleEndian::read_u32(&item); if !(disk_spec.physical_sector_size == 512 || disk_spec.physical_sector_size == 4096) { diff --git a/block/src/formats/vhdx/internal/mod.rs b/block/src/formats/vhdx/internal/mod.rs index 0efb895bf..a2a1d0715 100644 --- a/block/src/formats/vhdx/internal/mod.rs +++ b/block/src/formats/vhdx/internal/mod.rs @@ -20,6 +20,7 @@ use self::header::{RegionInfo, RegionTableEntry, VhdxHeader, VhdxHeaderError}; use self::io::VhdxIoError; use self::metadata::{DiskSpec, VhdxMetadataError}; use crate::BlockBackend; +use crate::aligned_file::AlignedFile; mod bat; mod header; @@ -49,7 +50,7 @@ pub type Result = result::Result; #[derive(Debug)] pub struct Vhdx { - file: File, + aligned: AlignedFile, vhdx_header: VhdxHeader, region_entries: BTreeMap, bat_entry: RegionTableEntry, @@ -63,11 +64,13 @@ pub struct Vhdx { impl Vhdx { /// Parse the Vhdx header, BAT, and metadata from a file and store info // in Vhdx structure. - pub fn new(mut file: File) -> Result { - let vhdx_header = VhdxHeader::new(&mut file).map_err(VhdxError::ParseVhdxHeader)?; + pub fn new(file: File, direct_io: bool) -> Result { + let aligned = AlignedFile::new(file, direct_io); + + let vhdx_header = VhdxHeader::new(&aligned).map_err(VhdxError::ParseVhdxHeader)?; let collected_entries = RegionInfo::new( - &mut file, + &aligned, header::REGION_TABLE_1_START, vhdx_header.region_entry_count(), ) @@ -77,12 +80,12 @@ impl Vhdx { let mdr_entry = collected_entries.mdr_entry; let disk_spec = - DiskSpec::new(&mut file, &mdr_entry).map_err(VhdxError::ParseVhdxMetadata)?; - let bat_entries = BatEntry::collect_bat_entries(&mut file, &disk_spec, &bat_entry) + DiskSpec::new(&aligned, &mdr_entry).map_err(VhdxError::ParseVhdxMetadata)?; + let bat_entries = BatEntry::collect_bat_entries(&aligned, &disk_spec, &bat_entry) .map_err(VhdxError::ReadBatEntry)?; Ok(Vhdx { - file, + aligned, vhdx_header, region_entries: collected_entries.region_entries, bat_entry, @@ -107,7 +110,7 @@ impl Read for Vhdx { let sector_index = self.current_offset / self.disk_spec.logical_sector_size as u64; let result = io::read( - &mut self.file, + &self.aligned, buf, &self.disk_spec, &self.bat_entries, @@ -128,7 +131,7 @@ impl Read for Vhdx { impl Write for Vhdx { fn flush(&mut self) -> IoResult<()> { - self.file.flush() + self.aligned.file_mut().flush() } /// Wrapper function to satisfy Write trait implementation for VHDx disk. @@ -140,12 +143,12 @@ impl Write for Vhdx { if self.first_write { self.first_write = false; self.vhdx_header - .update(&mut self.file) + .update(&self.aligned) .map_err(|e| IoError::other(format!("Failed to update VHDx header: {e}")))?; } let result = io::write( - &mut self.file, + &self.aligned, buf, &mut self.disk_spec, self.bat_entry.file_offset, @@ -210,7 +213,8 @@ impl BlockBackend for Vhdx { } fn physical_size(&self) -> result::Result { - self.file + self.aligned + .file() .metadata() .map(|m| m.len()) .map_err(crate::Error::GetFileMetadata) @@ -220,7 +224,7 @@ impl BlockBackend for Vhdx { impl Clone for Vhdx { fn clone(&self) -> Self { Vhdx { - file: self.file.try_clone().unwrap(), + aligned: self.aligned.try_clone().unwrap(), vhdx_header: self.vhdx_header.clone(), region_entries: self.region_entries.clone(), bat_entry: self.bat_entry, @@ -235,7 +239,7 @@ impl Clone for Vhdx { impl AsRawFd for Vhdx { fn as_raw_fd(&self) -> RawFd { - self.file.as_raw_fd() + self.aligned.file().as_raw_fd() } } @@ -250,3 +254,64 @@ pub(crate) fn uuid_from_guid(buf: &[u8]) -> Uuid { buf[8..16].try_into().unwrap(), ) } + +#[cfg(test)] +mod tests { + use std::process::Command; + + use vmm_sys_util::tempfile::TempFile; + + use super::*; + + /// Generate a small dynamic VHDX with `qemu-img`. Returns `None` (and the + /// test is skipped) when `qemu-img` is unavailable, e.g. in minimal CI. + fn dynamic_vhdx(size_mib: u64) -> Option { + let tf = TempFile::new().unwrap(); + let path = tf.as_path(); + let status = Command::new("qemu-img") + .args(["create", "-f", "vhdx", "-o", "subformat=dynamic"]) + .arg(path) + .arg(format!("{size_mib}M")) + .status(); + match status { + Ok(s) if s.success() => Some(tf), + _ => None, + } + } + + /// An unaligned sector write under a forced O_DIRECT alignment must go + /// through `AlignedFile`'s read-modify-write bounce (the data block and the + /// BAT update both land at unaligned host offsets) and read back intact. + #[test] + fn unaligned_write_is_rmw() { + let Some(tf) = dynamic_vhdx(16) else { + eprintln!("skipping unaligned_write_is_rmw: qemu-img unavailable"); + return; + }; + + let file = std::fs::OpenOptions::new() + .read(true) + .write(true) + .open(tf.as_path()) + .unwrap(); + let mut vhdx = Vhdx::new(file, false).unwrap(); + + // Force a non-zero alignment so all of vhdx's positioned I/O exercises + // the bounce/RMW path even though the tempfile is not really O_DIRECT. + vhdx.aligned = AlignedFile::with_alignment(vhdx.aligned.file().try_clone().unwrap(), 512); + + let sector = vhdx.disk_spec.logical_sector_size as usize; + let data: Vec = (0..sector).map(|i| ((i + 1) % 251) as u8).collect(); + + // Write at virtual offset 0 (allocates a new data block + rewrites BAT). + vhdx.seek(SeekFrom::Start(0)).unwrap(); + assert_eq!(vhdx.write(&data).unwrap(), data.len()); + vhdx.flush().unwrap(); + + // Read it back through a fresh, forced-alignment handle. + let mut readback = vec![0u8; sector]; + vhdx.seek(SeekFrom::Start(0)).unwrap(); + assert_eq!(vhdx.read(&mut readback).unwrap(), readback.len()); + assert_eq!(readback, data); + } +} diff --git a/block/src/formats/vhdx/mod.rs b/block/src/formats/vhdx/mod.rs index 7a48d5b77..e68a77a4d 100644 --- a/block/src/formats/vhdx/mod.rs +++ b/block/src/formats/vhdx/mod.rs @@ -38,9 +38,9 @@ pub struct VhdxDisk { } impl VhdxDisk { - pub fn new(f: File) -> BlockResult { + pub fn new(f: File, direct_io: bool) -> BlockResult { Ok(VhdxDisk { - vhdx_file: Arc::new(Mutex::new(Vhdx::new(f).map_err(|e| { + vhdx_file: Arc::new(Mutex::new(Vhdx::new(f, direct_io).map_err(|e| { let kind = match &e { VhdxError::NotVhdx(_) | VhdxError::ParseVhdxHeader(_) diff --git a/fuzz/fuzz_targets/vhdx.rs b/fuzz/fuzz_targets/vhdx.rs index 96caf7eca..739afc296 100644 --- a/fuzz/fuzz_targets/vhdx.rs +++ b/fuzz/fuzz_targets/vhdx.rs @@ -22,7 +22,7 @@ fuzz_target!(|bytes: &[u8]| -> Corpus { disk_file.write_all(&bytes[..]).unwrap(); disk_file.seek(SeekFrom::Start(0)).unwrap(); - let mut vhdx = match Vhdx::new(disk_file) { + let mut vhdx = match Vhdx::new(disk_file, false) { Ok(vhdx) => vhdx, Err(_) => return Corpus::Reject, };