From bfd97545c7247012349a7cb3eb6ba5ff34ce6fda Mon Sep 17 00:00:00 2001 From: liuxiangdong Date: Thu, 24 Jul 2025 18:54:47 +0800 Subject: [PATCH] file: obtain IO alignment size from file system DirectIO may degrade into bufferIO when not aligned, and IO will not report errors at this time. Detecting alignment by judging the return value of IO is unreliable. Directly obtain the alignment values presented by the operating system. Signed-off-by: liuxiangdong --- block_backend/src/qcow2/mod.rs | 8 ++-- machine_manager/src/config/mod.rs | 2 +- util/src/file.rs | 75 +++++-------------------------- 3 files changed, 16 insertions(+), 69 deletions(-) diff --git a/block_backend/src/qcow2/mod.rs b/block_backend/src/qcow2/mod.rs index 4f5675ad..28b5ee5c 100644 --- a/block_backend/src/qcow2/mod.rs +++ b/block_backend/src/qcow2/mod.rs @@ -2185,7 +2185,7 @@ mod test { None, ) .unwrap(); - let (req_align, buf_align) = get_file_alignment(&image.file, true); + let (req_align, buf_align) = get_file_alignment(&image.file, true).unwrap(); let conf = BlockProperty { id: path.to_string(), format: DiskFormat::Qcow2, @@ -2509,7 +2509,7 @@ mod test { let alloc_clusters: Vec = vec![1, 2, 4, 8, 16, 32]; for n_clusters in alloc_clusters { let image = TestImage::new(path, image_bits, cluster_bits); - let (req_align, buf_align) = get_file_alignment(&image.file, true); + let (req_align, buf_align) = get_file_alignment(&image.file, true).unwrap(); let conf = BlockProperty { id: path.to_string(), format: DiskFormat::Qcow2, @@ -2586,7 +2586,7 @@ mod test { // and then use the aligned interval for recying disk. for (offset_begin, offset_end) in test_data { let mut image = TestImage::new(path, image_bits, cluster_bits); - let (req_align, buf_align) = get_file_alignment(&image.file, true); + let (req_align, buf_align) = get_file_alignment(&image.file, true).unwrap(); conf.req_align = req_align; conf.buf_align = buf_align; let mut qcow2_driver = image.create_qcow2_driver(conf.clone()); @@ -2628,7 +2628,7 @@ mod test { let cluster_bits = 16; let cluster_size = 1 << cluster_bits; let mut image = TestImage::new(path, image_bits, cluster_bits); - let (req_align, buf_align) = get_file_alignment(&image.file, true); + let (req_align, buf_align) = get_file_alignment(&image.file, true).unwrap(); let conf = BlockProperty { id: path.to_string(), format: DiskFormat::Qcow2, diff --git a/machine_manager/src/config/mod.rs b/machine_manager/src/config/mod.rs index 35d705c1..95d55420 100644 --- a/machine_manager/src/config/mod.rs +++ b/machine_manager/src/config/mod.rs @@ -298,7 +298,7 @@ impl VmConfig { } } let file = open_file(path, read_only, direct)?; - let (req_align, buf_align) = get_file_alignment(&file, direct); + let (req_align, buf_align) = get_file_alignment(&file, direct)?; if req_align == 0 || buf_align == 0 { bail!( "Failed to detect alignment requirement of drive file {}.", diff --git a/util/src/file.rs b/util/src/file.rs index c6821176..a1939432 100644 --- a/util/src/file.rs +++ b/util/src/file.rs @@ -11,7 +11,7 @@ // See the Mulan PSL v2 for more details. use std::fs::{remove_file, File, OpenOptions}; -use std::os::unix::fs::OpenOptionsExt; +use std::os::unix::fs::{MetadataExt, OpenOptionsExt}; use std::os::unix::io::AsRawFd; use std::path::Path; @@ -19,7 +19,6 @@ use anyhow::{bail, Context, Ok, Result}; use nix::fcntl::{fcntl, FcntlArg}; use nix::unistd::getpid; -const MIN_FILE_ALIGN: u32 = 512; pub const MAX_FILE_ALIGN: u32 = 4096; /// Permission to read const FILE_LOCK_READ: u64 = 0x01; @@ -49,72 +48,20 @@ pub fn open_file(path: &str, read_only: bool, direct: bool) -> Result { Ok(file) } -/// # Safety -/// -/// The length of buf must not less than size. -unsafe fn is_io_aligned(file: &File, buf: u64, size: usize) -> bool { - // SAFETY: caller promises buf and size are valid. - let ret = unsafe { - libc::pread( - file.as_raw_fd() as libc::c_int, - buf as *mut libc::c_void, - size as libc::size_t, - 0, - ) - }; - ret >= 0 || nix::errno::errno() != libc::EINVAL -} - -pub fn get_file_alignment(file: &File, direct: bool) -> (u32, u32) { +pub fn get_file_alignment(file: &File, direct: bool) -> Result<(u32, u32)> { if !direct { - return (1, 1); + return Ok((1, 1)); } - let mut req_align: u32 = 0; - let mut buf_align: u32 = 0; - // SAFETY: we allocate aligned memory and free it later. - let aligned_buffer = unsafe { - libc::memalign( - MAX_FILE_ALIGN as libc::size_t, - (MAX_FILE_ALIGN * 2) as libc::size_t, - ) - }; - if aligned_buffer.is_null() { - log::warn!("OOM occurs when get file alignment, assume max alignment"); - return (MAX_FILE_ALIGN, MAX_FILE_ALIGN); - } - - // Guess alignment requirement of request. - let mut align = MIN_FILE_ALIGN; - while align <= MAX_FILE_ALIGN { - // SAFETY: aligned_buffer is valid and length is enough. - if unsafe { is_io_aligned(file, aligned_buffer as u64, align as usize) } { - req_align = align; - break; - } - align <<= 1; - } - - // Guess alignment requirement of buffer. - let mut align = MIN_FILE_ALIGN; - while align <= MAX_FILE_ALIGN { - // SAFETY: aligned_buffer is valid and length is enough. - if unsafe { - is_io_aligned( - file, - aligned_buffer as u64 + u64::from(align), - MAX_FILE_ALIGN as usize, - ) - } { - buf_align = align; - break; - } - align <<= 1; - } + // DirectIO may degrade into bufferIO when not aligned, and IO will not report errors at this time. + // Detecting alignment by judging the return value of IO is unreliable. + // Directly obtain the alignment values presented by the Filesystem. + let metadata = file + .metadata() + .with_context(|| "Failed to get file metadata.")?; + let blk_size = metadata.blksize() as u32; - // SAFETY: the memory is allocated by us and will not be used anymore. - unsafe { libc::free(aligned_buffer) }; - (req_align, buf_align) + Ok((blk_size, blk_size)) } fn do_fcntl_lock( -- Gitee