From b595f1dbc3222c485f454f07d88f49fb65c4e41f Mon Sep 17 00:00:00 2001 From: Anatol Belski Date: Sat, 25 Apr 2026 00:39:24 +0200 Subject: [PATCH] block: Move VHDX format files into formats/vhdx/ Move VHDX format implementation into a structured directory layout: vhdx/mod.rs -> formats/vhdx/internal/mod.rs (Vhdx) vhdx/vhdx_bat.rs -> formats/vhdx/internal/bat.rs vhdx/vhdx_header.rs -> formats/vhdx/internal/header.rs vhdx/vhdx_io.rs -> formats/vhdx/internal/io.rs vhdx/vhdx_metadata.rs -> formats/vhdx/internal/metadata.rs vhdx_sync.rs -> formats/vhdx/mod.rs (VhdxDisk) Extract VhdxSync from vhdx_sync.rs into formats/vhdx/worker/sync.rs. Drop the vhdx_ prefix from internal file names since the parent directory already provides the namespace. Update all internal cross references to use the new module paths. Re-export formats::vhdx as vhdx_sync in lib.rs for backward compatibility. Signed-off-by: Anatol Belski --- block/src/factory.rs | 2 +- block/src/formats/mod.rs | 1 + .../vhdx/internal/bat.rs} | 4 +- .../vhdx/internal/header.rs} | 2 +- .../vhdx/internal/io.rs} | 45 ++++--- .../vhdx/internal/metadata.rs} | 4 +- .../{vhdx => formats/vhdx/internal}/mod.rs | 22 ++-- block/src/formats/vhdx/mod.rs | 111 ++++++++++++++++++ block/src/formats/vhdx/worker/mod.rs | 7 ++ .../vhdx/worker/sync.rs} | 97 +-------------- block/src/lib.rs | 8 +- 11 files changed, 163 insertions(+), 140 deletions(-) rename block/src/{vhdx/vhdx_bat.rs => formats/vhdx/internal/bat.rs} (97%) rename block/src/{vhdx/vhdx_header.rs => formats/vhdx/internal/header.rs} (99%) rename block/src/{vhdx/vhdx_io.rs => formats/vhdx/internal/io.rs} (81%) rename block/src/{vhdx/vhdx_metadata.rs => formats/vhdx/internal/metadata.rs} (99%) rename block/src/{vhdx => formats/vhdx/internal}/mod.rs (94%) create mode 100644 block/src/formats/vhdx/mod.rs create mode 100644 block/src/formats/vhdx/worker/mod.rs rename block/src/{vhdx_sync.rs => formats/vhdx/worker/sync.rs} (50%) diff --git a/block/src/factory.rs b/block/src/factory.rs index d3d50111c..ba9abbe81 100644 --- a/block/src/factory.rs +++ b/block/src/factory.rs @@ -22,8 +22,8 @@ use crate::disk_file::AsyncFullDiskFile; use crate::error::{BlockError, BlockErrorKind, BlockResult}; use crate::formats::raw::{RawBackend, RawDisk}; use crate::formats::vhd::VhdDisk; +use crate::formats::vhdx::VhdxDisk; use crate::qcow_disk::QcowDisk; -use crate::vhdx_sync::VhdxDisk; use crate::{ ImageType, block_aio_is_supported, detect_image_type, open_disk_image, preallocate_disk, }; diff --git a/block/src/formats/mod.rs b/block/src/formats/mod.rs index 0282640ac..ba1697320 100644 --- a/block/src/formats/mod.rs +++ b/block/src/formats/mod.rs @@ -9,3 +9,4 @@ pub mod raw; pub mod vhd; +pub mod vhdx; diff --git a/block/src/vhdx/vhdx_bat.rs b/block/src/formats/vhdx/internal/bat.rs similarity index 97% rename from block/src/vhdx/vhdx_bat.rs rename to block/src/formats/vhdx/internal/bat.rs index 52ab55cb7..04f30e406 100644 --- a/block/src/vhdx/vhdx_bat.rs +++ b/block/src/formats/vhdx/internal/bat.rs @@ -10,8 +10,8 @@ use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; use remain::sorted; use thiserror::Error; -use crate::vhdx::vhdx_header::RegionTableEntry; -use crate::vhdx::vhdx_metadata::DiskSpec; +use super::header::RegionTableEntry; +use super::metadata::DiskSpec; // Payload BAT Entry States pub const PAYLOAD_BLOCK_NOT_PRESENT: u64 = 0; diff --git a/block/src/vhdx/vhdx_header.rs b/block/src/formats/vhdx/internal/header.rs similarity index 99% rename from block/src/vhdx/vhdx_header.rs rename to block/src/formats/vhdx/internal/header.rs index 5c8c3e37d..be7ebb124 100644 --- a/block/src/vhdx/vhdx_header.rs +++ b/block/src/formats/vhdx/internal/header.rs @@ -341,7 +341,7 @@ impl RegionTableEntry { // SAFETY: the assertion above makes sure the buffer size is correct. let mut region_table_entry: RegionTableEntry = unsafe { *(buffer.as_ptr().cast()) }; - let uuid = crate::vhdx::uuid_from_guid(buffer); + let uuid = super::uuid_from_guid(buffer); region_table_entry.guid = uuid; Ok(region_table_entry) diff --git a/block/src/vhdx/vhdx_io.rs b/block/src/formats/vhdx/internal/io.rs similarity index 81% rename from block/src/vhdx/vhdx_io.rs rename to block/src/formats/vhdx/internal/io.rs index 96ce4ef4c..80e52bfbc 100644 --- a/block/src/vhdx/vhdx_io.rs +++ b/block/src/formats/vhdx/internal/io.rs @@ -8,8 +8,8 @@ use std::io::{self, Read, Seek, SeekFrom, Write}; use remain::sorted; use thiserror::Error; -use crate::vhdx::vhdx_bat::{self, BatEntry, VhdxBatError}; -use crate::vhdx::vhdx_metadata::{self, DiskSpec}; +use super::bat::{self, BatEntry, VhdxBatError}; +use super::metadata::{self, DiskSpec}; const SECTOR_SIZE: u64 = 512; @@ -74,7 +74,7 @@ impl Sector { return Err(VhdxIoError::InvalidBatIndex); } }; - sector.file_offset = bat_entry & vhdx_bat::BAT_FILE_OFF_MASK; + sector.file_offset = bat_entry & bat::BAT_FILE_OFF_MASK; if sector.file_offset != 0 { sector.file_offset += sector.block_offset; } @@ -108,12 +108,12 @@ pub fn read( } }; - match bat_entry & vhdx_bat::BAT_STATE_BIT_MASK { - vhdx_bat::PAYLOAD_BLOCK_NOT_PRESENT - | vhdx_bat::PAYLOAD_BLOCK_UNDEFINED - | vhdx_bat::PAYLOAD_BLOCK_UNMAPPED - | vhdx_bat::PAYLOAD_BLOCK_ZERO => {} - vhdx_bat::PAYLOAD_BLOCK_FULLY_PRESENT => { + match bat_entry & bat::BAT_STATE_BIT_MASK { + bat::PAYLOAD_BLOCK_NOT_PRESENT + | bat::PAYLOAD_BLOCK_UNDEFINED + | 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( @@ -122,7 +122,7 @@ pub fn read( ) .map_err(VhdxIoError::ReadSectorBlock)?; } - vhdx_bat::PAYLOAD_BLOCK_PARTIALLY_PRESENT => { + bat::PAYLOAD_BLOCK_PARTIALLY_PRESENT => { return Err(VhdxIoError::UnsupportedMode); } _ => { @@ -162,13 +162,12 @@ pub fn write( } }; - match bat_entry & vhdx_bat::BAT_STATE_BIT_MASK { - vhdx_bat::PAYLOAD_BLOCK_NOT_PRESENT - | vhdx_bat::PAYLOAD_BLOCK_UNDEFINED - | vhdx_bat::PAYLOAD_BLOCK_UNMAPPED - | vhdx_bat::PAYLOAD_BLOCK_ZERO => { - let file_offset = - align!(disk_spec.image_size, vhdx_metadata::BLOCK_SIZE_MIN as u64); + match bat_entry & bat::BAT_STATE_BIT_MASK { + bat::PAYLOAD_BLOCK_NOT_PRESENT + | bat::PAYLOAD_BLOCK_UNDEFINED + | bat::PAYLOAD_BLOCK_UNMAPPED + | bat::PAYLOAD_BLOCK_ZERO => { + let file_offset = align!(disk_spec.image_size, metadata::BLOCK_SIZE_MIN as u64); let new_size = file_offset .checked_add(disk_spec.block_size as u64) .ok_or(VhdxIoError::InvalidDiskSize)?; @@ -176,12 +175,12 @@ pub fn write( f.set_len(new_size).map_err(VhdxIoError::ResizeFile)?; disk_spec.image_size = new_size; - let new_bat_entry = file_offset - | (vhdx_bat::PAYLOAD_BLOCK_FULLY_PRESENT & vhdx_bat::BAT_STATE_BIT_MASK); + let new_bat_entry = + file_offset | (bat::PAYLOAD_BLOCK_FULLY_PRESENT & bat::BAT_STATE_BIT_MASK); bat[sector.bat_index as usize] = BatEntry(new_bat_entry); BatEntry::write_bat_entries(f, bat_offset, bat).map_err(VhdxIoError::WriteBat)?; - if file_offset < vhdx_metadata::BLOCK_SIZE_MIN as u64 { + if file_offset < metadata::BLOCK_SIZE_MIN as u64 { break; } @@ -192,8 +191,8 @@ pub fn write( ) .map_err(VhdxIoError::ReadSectorBlock)?; } - vhdx_bat::PAYLOAD_BLOCK_FULLY_PRESENT => { - if sector.file_offset < vhdx_metadata::BLOCK_SIZE_MIN as u64 { + bat::PAYLOAD_BLOCK_FULLY_PRESENT => { + if sector.file_offset < metadata::BLOCK_SIZE_MIN as u64 { break; } @@ -204,7 +203,7 @@ pub fn write( ) .map_err(VhdxIoError::ReadSectorBlock)?; } - vhdx_bat::PAYLOAD_BLOCK_PARTIALLY_PRESENT => { + bat::PAYLOAD_BLOCK_PARTIALLY_PRESENT => { return Err(VhdxIoError::UnsupportedMode); } _ => { diff --git a/block/src/vhdx/vhdx_metadata.rs b/block/src/formats/vhdx/internal/metadata.rs similarity index 99% rename from block/src/vhdx/vhdx_metadata.rs rename to block/src/formats/vhdx/internal/metadata.rs index 0410d9af9..f0e31890d 100644 --- a/block/src/vhdx/vhdx_metadata.rs +++ b/block/src/formats/vhdx/internal/metadata.rs @@ -11,7 +11,7 @@ use remain::sorted; use thiserror::Error; use uuid::Uuid; -use crate::vhdx::vhdx_header::RegionTableEntry; +use super::header::RegionTableEntry; const METADATA_SIGN: u64 = 0x6174_6164_6174_656D; const METADATA_ENTRY_SIZE: usize = 32; @@ -315,7 +315,7 @@ impl MetadataTableEntry { // SAFETY: the assertion above makes sure the buffer size is correct. let mut metadata_table_entry: MetadataTableEntry = unsafe { *(buffer.as_ptr().cast()) }; - let uuid = crate::vhdx::uuid_from_guid(buffer); + let uuid = super::uuid_from_guid(buffer); metadata_table_entry.item_id = uuid; if metadata_table_entry.length > METADATA_LENGTH_MAX { diff --git a/block/src/vhdx/mod.rs b/block/src/formats/vhdx/internal/mod.rs similarity index 94% rename from block/src/vhdx/mod.rs rename to block/src/formats/vhdx/internal/mod.rs index f8d404fc5..bc3eaed75 100644 --- a/block/src/vhdx/mod.rs +++ b/block/src/formats/vhdx/internal/mod.rs @@ -12,16 +12,16 @@ use remain::sorted; use thiserror::Error; use uuid::Uuid; +use self::bat::{BatEntry, VhdxBatError}; +use self::header::{RegionInfo, RegionTableEntry, VhdxHeader, VhdxHeaderError}; +use self::io::VhdxIoError; +use self::metadata::{DiskSpec, VhdxMetadataError}; use crate::BlockBackend; -use crate::vhdx::vhdx_bat::{BatEntry, VhdxBatError}; -use crate::vhdx::vhdx_header::{RegionInfo, RegionTableEntry, VhdxHeader, VhdxHeaderError}; -use crate::vhdx::vhdx_io::VhdxIoError; -use crate::vhdx::vhdx_metadata::{DiskSpec, VhdxMetadataError}; -mod vhdx_bat; -mod vhdx_header; -mod vhdx_io; -mod vhdx_metadata; +mod bat; +mod header; +mod io; +mod metadata; #[sorted] #[derive(Error, Debug)] @@ -65,7 +65,7 @@ impl Vhdx { let collected_entries = RegionInfo::new( &mut file, - vhdx_header::REGION_TABLE_1_START, + header::REGION_TABLE_1_START, vhdx_header.region_entry_count(), ) .map_err(VhdxError::ParseVhdxRegionEntry)?; @@ -103,7 +103,7 @@ impl Read for Vhdx { let sector_count = (buf.len() as u64).div_ceil(self.disk_spec.logical_sector_size as u64); let sector_index = self.current_offset / self.disk_spec.logical_sector_size as u64; - let result = vhdx_io::read( + let result = io::read( &mut self.file, buf, &self.disk_spec, @@ -141,7 +141,7 @@ impl Write for Vhdx { .map_err(|e| std::io::Error::other(format!("Failed to update VHDx header: {e}")))?; } - let result = vhdx_io::write( + let result = io::write( &mut self.file, buf, &mut self.disk_spec, diff --git a/block/src/formats/vhdx/mod.rs b/block/src/formats/vhdx/mod.rs new file mode 100644 index 000000000..d1be62c7a --- /dev/null +++ b/block/src/formats/vhdx/mod.rs @@ -0,0 +1,111 @@ +// Copyright © 2021 Intel Corporation +// +// Copyright (c) Meta Platforms, Inc. and affiliates. +// +// SPDX-License-Identifier: Apache-2.0 + +//! VHDX disk format support. +//! +//! Provides [`VhdxDisk`], the `DiskFile` wrapper for dynamic VHDX +//! images. + +pub mod internal; +pub(crate) mod worker; + +use std::fs::File; +use std::os::fd::AsRawFd; +use std::sync::{Arc, Mutex}; + +pub use internal::VhdxError; + +use self::internal::Vhdx; +use self::worker::sync::VhdxSync; +use crate::async_io::{AsyncIo, BorrowedDiskFd, DiskFileError}; +use crate::error::{BlockError, BlockErrorKind, BlockResult, ErrorOp}; +use crate::{BlockBackend, Error, disk_file}; + +#[derive(Debug)] +pub struct VhdxDisk { + // FIXME: The Mutex serializes all VHDX I/O operations across queues, which + // is necessary for correctness but eliminates any parallelism benefit from + // multiqueue. Vhdx::clone() shares the underlying file description across + // threads, so concurrent I/O from multiple queues races on the file offset + // causing data corruption. + // + // A proper fix would require restructuring the VHDX I/O path so that data + // operations can proceed in parallel with independent file descriptors. + vhdx_file: Arc>, +} + +impl VhdxDisk { + pub fn new(f: File) -> BlockResult { + Ok(VhdxDisk { + vhdx_file: Arc::new(Mutex::new(Vhdx::new(f).map_err(|e| { + let kind = match &e { + VhdxError::NotVhdx(_) + | VhdxError::ParseVhdxHeader(_) + | VhdxError::ParseVhdxMetadata(_) + | VhdxError::ParseVhdxRegionEntry(_) => BlockErrorKind::InvalidFormat, + VhdxError::ReadBatEntry(_) => BlockErrorKind::CorruptImage, + VhdxError::ReadFailed(_) | VhdxError::WriteFailed(_) => BlockErrorKind::Io, + }; + BlockError::new(kind, e).with_op(ErrorOp::Open) + })?)), + }) + } +} + +impl disk_file::DiskSize for VhdxDisk { + fn logical_size(&self) -> BlockResult { + Ok(self.vhdx_file.lock().unwrap().virtual_disk_size()) + } +} + +impl disk_file::PhysicalSize for VhdxDisk { + fn physical_size(&self) -> BlockResult { + self.vhdx_file + .lock() + .unwrap() + .physical_size() + .map_err(|e| match e { + Error::GetFileMetadata(io) => { + BlockError::new(BlockErrorKind::Io, Error::GetFileMetadata(io)) + } + _ => unreachable!("unexpected error from Vhdx::physical_size(): {e}"), + }) + } +} + +impl disk_file::DiskFd for VhdxDisk { + fn fd(&self) -> BorrowedDiskFd<'_> { + BorrowedDiskFd::new(self.vhdx_file.lock().unwrap().as_raw_fd()) + } +} + +impl disk_file::Geometry for VhdxDisk {} + +impl disk_file::SparseCapable for VhdxDisk {} + +impl disk_file::Resizable for VhdxDisk { + fn resize(&mut self, _size: u64) -> BlockResult<()> { + Err(BlockError::new( + BlockErrorKind::UnsupportedFeature, + DiskFileError::ResizeError(std::io::Error::other("resize not supported for VHDX")), + ) + .with_op(ErrorOp::Resize)) + } +} + +impl disk_file::DiskFile for VhdxDisk {} + +impl disk_file::AsyncDiskFile for VhdxDisk { + fn try_clone(&self) -> BlockResult> { + Ok(Box::new(VhdxDisk { + vhdx_file: Arc::clone(&self.vhdx_file), + })) + } + + fn create_async_io(&self, _ring_depth: u32) -> BlockResult> { + Ok(Box::new(VhdxSync::new(Arc::clone(&self.vhdx_file)))) + } +} diff --git a/block/src/formats/vhdx/worker/mod.rs b/block/src/formats/vhdx/worker/mod.rs new file mode 100644 index 000000000..461f37061 --- /dev/null +++ b/block/src/formats/vhdx/worker/mod.rs @@ -0,0 +1,7 @@ +// Copyright 2026 The Cloud Hypervisor Authors. All rights reserved. +// +// SPDX-License-Identifier: Apache-2.0 + +//! Sync I/O worker for VHDX images. + +pub(crate) mod sync; diff --git a/block/src/vhdx_sync.rs b/block/src/formats/vhdx/worker/sync.rs similarity index 50% rename from block/src/vhdx_sync.rs rename to block/src/formats/vhdx/worker/sync.rs index 5518ab82e..e8d718f14 100644 --- a/block/src/vhdx_sync.rs +++ b/block/src/formats/vhdx/worker/sync.rs @@ -5,106 +5,13 @@ // SPDX-License-Identifier: Apache-2.0 use std::collections::VecDeque; -use std::fs::File; use std::io::{Read, Seek, SeekFrom, Write}; -use std::os::fd::AsRawFd; use std::sync::{Arc, Mutex}; use vmm_sys_util::eventfd::EventFd; -use crate::async_io::{ - AsyncIo, AsyncIoCompletion, AsyncIoError, AsyncIoOperation, AsyncIoResult, BorrowedDiskFd, - DiskFileError, -}; -use crate::error::{BlockError, BlockErrorKind, BlockResult, ErrorOp}; -use crate::vhdx::{Vhdx, VhdxError}; -use crate::{BlockBackend, Error, disk_file}; - -#[derive(Debug)] -pub struct VhdxDisk { - // FIXME: The Mutex serializes all VHDX I/O operations across queues, which - // is necessary for correctness but eliminates any parallelism benefit from - // multiqueue. Vhdx::clone() shares the underlying file description across - // threads, so concurrent I/O from multiple queues races on the file offset - // causing data corruption. - // - // A proper fix would require restructuring the VHDX I/O path so that data - // operations can proceed in parallel with independent file descriptors. - vhdx_file: Arc>, -} - -impl VhdxDisk { - pub fn new(f: File) -> BlockResult { - Ok(VhdxDisk { - vhdx_file: Arc::new(Mutex::new(Vhdx::new(f).map_err(|e| { - let kind = match &e { - VhdxError::NotVhdx(_) - | VhdxError::ParseVhdxHeader(_) - | VhdxError::ParseVhdxMetadata(_) - | VhdxError::ParseVhdxRegionEntry(_) => BlockErrorKind::InvalidFormat, - VhdxError::ReadBatEntry(_) => BlockErrorKind::CorruptImage, - VhdxError::ReadFailed(_) | VhdxError::WriteFailed(_) => BlockErrorKind::Io, - }; - BlockError::new(kind, e).with_op(ErrorOp::Open) - })?)), - }) - } -} - -impl disk_file::DiskSize for VhdxDisk { - fn logical_size(&self) -> BlockResult { - Ok(self.vhdx_file.lock().unwrap().virtual_disk_size()) - } -} - -impl disk_file::PhysicalSize for VhdxDisk { - fn physical_size(&self) -> BlockResult { - self.vhdx_file - .lock() - .unwrap() - .physical_size() - .map_err(|e| match e { - Error::GetFileMetadata(io) => { - BlockError::new(BlockErrorKind::Io, Error::GetFileMetadata(io)) - } - _ => unreachable!("unexpected error from Vhdx::physical_size(): {e}"), - }) - } -} - -impl disk_file::DiskFd for VhdxDisk { - fn fd(&self) -> BorrowedDiskFd<'_> { - BorrowedDiskFd::new(self.vhdx_file.lock().unwrap().as_raw_fd()) - } -} - -impl disk_file::Geometry for VhdxDisk {} - -impl disk_file::SparseCapable for VhdxDisk {} - -impl disk_file::Resizable for VhdxDisk { - fn resize(&mut self, _size: u64) -> BlockResult<()> { - Err(BlockError::new( - BlockErrorKind::UnsupportedFeature, - DiskFileError::ResizeError(std::io::Error::other("resize not supported for VHDX")), - ) - .with_op(ErrorOp::Resize)) - } -} - -impl disk_file::DiskFile for VhdxDisk {} - -impl disk_file::AsyncDiskFile for VhdxDisk { - fn try_clone(&self) -> BlockResult> { - Ok(Box::new(VhdxDisk { - vhdx_file: Arc::clone(&self.vhdx_file), - })) - } - - fn create_async_io(&self, _ring_depth: u32) -> BlockResult> { - Ok(Box::new(VhdxSync::new(Arc::clone(&self.vhdx_file)))) - } -} +use crate::async_io::{AsyncIo, AsyncIoCompletion, AsyncIoError, AsyncIoOperation, AsyncIoResult}; +use crate::formats::vhdx::internal::Vhdx; pub struct VhdxSync { vhdx_file: Arc>, diff --git a/block/src/lib.rs b/block/src/lib.rs index 99308e8ad..18e92b3c0 100644 --- a/block/src/lib.rs +++ b/block/src/lib.rs @@ -22,10 +22,6 @@ pub(crate) mod qcow_common; pub mod qcow_disk; pub(crate) mod qcow_sync; mod sparse; -pub use formats::raw as raw_disk; -pub mod vhdx; -pub mod vhdx_sync; - use std::alloc::{Layout, alloc_zeroed}; use std::fmt::{self, Debug}; use std::fs::{File, OpenOptions}; @@ -37,6 +33,8 @@ use std::path::Path; use std::str::FromStr; use std::{cmp, mem, result}; +pub use formats::raw as raw_disk; +pub use formats::vhdx::internal as vhdx; #[cfg(feature = "io_uring")] use io_uring::{IoUring, Probe, opcode}; use libc::{ @@ -54,8 +52,8 @@ use vmm_sys_util::{aio, ioctl_io_nr, ioctl_ior_nr}; use crate::async_io::AsyncIoError; use crate::error::{BlockError, BlockErrorKind, BlockResult, ErrorOp}; +use crate::formats::vhdx::VhdxError; use crate::request::SECTOR_SIZE; -use crate::vhdx::VhdxError; #[derive(Error, Debug)] pub enum Error {