From af770c814ba721d2009b713db7ed26b4fe905098 Mon Sep 17 00:00:00 2001 From: Wei Liu Date: Mon, 6 Dec 2021 10:57:42 +0000 Subject: [PATCH] block_util: provide and use AsyncAdaptor trait The observation is that the code in question was used to bridge synchronized and asynchronized code. We can group the functions for that purpose under an adaptor trait. To limit the scope of locking, the users of the trait are required to implement a method to return a MutexGuard for the underlying file. This then allows us to use concrete types (QcowFile and Vhdx) in code, which is easier to read than a bunch of traits. No functional change intended. Signed-off-by: Wei Liu --- block_util/src/lib.rs | 178 +++++++++++++++++------------------- block_util/src/qcow_sync.rs | 32 +++---- block_util/src/vhdx_sync.rs | 41 ++++----- 3 files changed, 118 insertions(+), 133 deletions(-) diff --git a/block_util/src/lib.rs b/block_util/src/lib.rs index b58f941d1..408c47d77 100644 --- a/block_util/src/lib.rs +++ b/block_util/src/lib.rs @@ -23,7 +23,6 @@ pub mod vhdx_sync; use crate::async_io::{AsyncIo, AsyncIoError, AsyncIoResult}; #[cfg(feature = "io_uring")] use io_uring::{opcode, IoUring, Probe}; -use std::any::Any; use std::cmp; use std::convert::TryInto; use std::fs::File; @@ -31,7 +30,7 @@ use std::io::{self, IoSlice, IoSliceMut, Read, Seek, SeekFrom, Write}; use std::os::linux::fs::MetadataExt; use std::path::Path; use std::result; -use std::sync::{Arc, Mutex}; +use std::sync::MutexGuard; use versionize::{VersionMap, Versionize, VersionizeResult}; use versionize_derive::Versionize; use virtio_bindings::bindings::virtio_blk::*; @@ -477,103 +476,98 @@ pub fn block_io_uring_is_supported() -> bool { false } -pub trait ReadWriteSeekFile: Read + Write + Seek { - // Provides a mutable reference to the Any trait. This is useful to let - // the caller have access to the underlying type behind the trait. - fn as_any(&mut self) -> &mut dyn Any; -} +pub trait AsyncAdaptor +where + F: Read + Write + Seek, +{ + fn read_vectored_sync( + &mut self, + offset: libc::off_t, + iovecs: Vec, + user_data: u64, + eventfd: &EventFd, + completion_list: &mut Vec<(u64, i32)>, + ) -> AsyncIoResult<()> { + // Convert libc::iovec into IoSliceMut + let mut slices = Vec::new(); + for iovec in iovecs.iter() { + slices.push(IoSliceMut::new(unsafe { std::mem::transmute(*iovec) })); + } -impl ReadWriteSeekFile for F { - fn as_any(&mut self) -> &mut dyn Any { - self - } -} + let result = { + let mut file = self.file(); -pub fn read_vectored_sync( - offset: libc::off_t, - iovecs: Vec, - user_data: u64, - file: &mut Arc>, - eventfd: &EventFd, - completion_list: &mut Vec<(u64, i32)>, -) -> AsyncIoResult<()> { - // Convert libc::iovec into IoSliceMut - let mut slices = Vec::new(); - for iovec in iovecs.iter() { - slices.push(IoSliceMut::new(unsafe { std::mem::transmute(*iovec) })); - } + // Move the cursor to the right offset + file.seek(SeekFrom::Start(offset as u64)) + .map_err(AsyncIoError::ReadVectored)?; - let result = { - let mut file = file.lock().unwrap(); + // Read vectored + file.read_vectored(slices.as_mut_slice()) + .map_err(AsyncIoError::ReadVectored)? + }; - // Move the cursor to the right offset - file.seek(SeekFrom::Start(offset as u64)) - .map_err(AsyncIoError::ReadVectored)?; - - // Read vectored - file.read_vectored(slices.as_mut_slice()) - .map_err(AsyncIoError::ReadVectored)? - }; - - completion_list.push((user_data, result as i32)); - eventfd.write(1).unwrap(); - - Ok(()) -} - -pub fn write_vectored_sync( - offset: libc::off_t, - iovecs: Vec, - user_data: u64, - file: &mut Arc>, - eventfd: &EventFd, - completion_list: &mut Vec<(u64, i32)>, -) -> AsyncIoResult<()> { - // Convert libc::iovec into IoSlice - let mut slices = Vec::new(); - for iovec in iovecs.iter() { - slices.push(IoSlice::new(unsafe { std::mem::transmute(*iovec) })); - } - - let result = { - let mut file = file.lock().unwrap(); - - // Move the cursor to the right offset - file.seek(SeekFrom::Start(offset as u64)) - .map_err(AsyncIoError::WriteVectored)?; - - // Write vectored - file.write_vectored(slices.as_slice()) - .map_err(AsyncIoError::WriteVectored)? - }; - - completion_list.push((user_data, result as i32)); - eventfd.write(1).unwrap(); - - Ok(()) -} - -pub fn fsync_sync( - user_data: Option, - file: &mut Arc>, - eventfd: &EventFd, - completion_list: &mut Vec<(u64, i32)>, -) -> AsyncIoResult<()> { - let result: i32 = { - let mut file = file.lock().unwrap(); - - // Flush - file.flush().map_err(AsyncIoError::Fsync)?; - - 0 - }; - - if let Some(user_data) = user_data { - completion_list.push((user_data, result)); + completion_list.push((user_data, result as i32)); eventfd.write(1).unwrap(); + + Ok(()) } - Ok(()) + fn write_vectored_sync( + &mut self, + offset: libc::off_t, + iovecs: Vec, + user_data: u64, + eventfd: &EventFd, + completion_list: &mut Vec<(u64, i32)>, + ) -> AsyncIoResult<()> { + // Convert libc::iovec into IoSlice + let mut slices = Vec::new(); + for iovec in iovecs.iter() { + slices.push(IoSlice::new(unsafe { std::mem::transmute(*iovec) })); + } + + let result = { + let mut file = self.file(); + + // Move the cursor to the right offset + file.seek(SeekFrom::Start(offset as u64)) + .map_err(AsyncIoError::WriteVectored)?; + + // Write vectored + file.write_vectored(slices.as_slice()) + .map_err(AsyncIoError::WriteVectored)? + }; + + completion_list.push((user_data, result as i32)); + eventfd.write(1).unwrap(); + + Ok(()) + } + + fn fsync_sync( + &mut self, + user_data: Option, + eventfd: &EventFd, + completion_list: &mut Vec<(u64, i32)>, + ) -> AsyncIoResult<()> { + let result: i32 = { + let mut file = self.file(); + + // Flush + file.flush().map_err(AsyncIoError::Fsync)?; + + 0 + }; + + if let Some(user_data) = user_data { + completion_list.push((user_data, result)); + eventfd.write(1).unwrap(); + } + + Ok(()) + } + + fn file(&mut self) -> MutexGuard; } pub enum ImageType { diff --git a/block_util/src/qcow_sync.rs b/block_util/src/qcow_sync.rs index 654c37118..b4b4b1cb3 100644 --- a/block_util/src/qcow_sync.rs +++ b/block_util/src/qcow_sync.rs @@ -3,15 +3,15 @@ // SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause use crate::async_io::{AsyncIo, AsyncIoResult, DiskFile, DiskFileError, DiskFileResult}; -use crate::{fsync_sync, read_vectored_sync, write_vectored_sync, ReadWriteSeekFile}; +use crate::AsyncAdaptor; use qcow::{QcowFile, RawFile, Result as QcowResult}; use std::fs::File; -use std::io::SeekFrom; -use std::sync::{Arc, Mutex}; +use std::io::{Seek, SeekFrom}; +use std::sync::{Arc, Mutex, MutexGuard}; use vmm_sys_util::eventfd::EventFd; pub struct QcowDiskSync { - qcow_file: Arc>, + qcow_file: Arc>, } impl QcowDiskSync { @@ -35,13 +35,13 @@ impl DiskFile for QcowDiskSync { } pub struct QcowSync { - qcow_file: Arc>, + qcow_file: Arc>, eventfd: EventFd, completion_list: Vec<(u64, i32)>, } impl QcowSync { - pub fn new(qcow_file: Arc>) -> Self { + pub fn new(qcow_file: Arc>) -> Self { QcowSync { qcow_file, eventfd: EventFd::new(libc::EFD_NONBLOCK) @@ -51,6 +51,12 @@ impl QcowSync { } } +impl AsyncAdaptor for Arc> { + fn file(&mut self) -> MutexGuard { + self.lock().unwrap() + } +} + impl AsyncIo for QcowSync { fn notifier(&self) -> &EventFd { &self.eventfd @@ -62,11 +68,10 @@ impl AsyncIo for QcowSync { iovecs: Vec, user_data: u64, ) -> AsyncIoResult<()> { - read_vectored_sync( + self.qcow_file.read_vectored_sync( offset, iovecs, user_data, - &mut self.qcow_file, &self.eventfd, &mut self.completion_list, ) @@ -78,23 +83,18 @@ impl AsyncIo for QcowSync { iovecs: Vec, user_data: u64, ) -> AsyncIoResult<()> { - write_vectored_sync( + self.qcow_file.write_vectored_sync( offset, iovecs, user_data, - &mut self.qcow_file, &self.eventfd, &mut self.completion_list, ) } fn fsync(&mut self, user_data: Option) -> AsyncIoResult<()> { - fsync_sync( - user_data, - &mut self.qcow_file, - &self.eventfd, - &mut self.completion_list, - ) + self.qcow_file + .fsync_sync(user_data, &self.eventfd, &mut self.completion_list) } fn complete(&mut self) -> Vec<(u64, i32)> { diff --git a/block_util/src/vhdx_sync.rs b/block_util/src/vhdx_sync.rs index 6846b6dcf..48b0e3ac5 100644 --- a/block_util/src/vhdx_sync.rs +++ b/block_util/src/vhdx_sync.rs @@ -3,14 +3,14 @@ // SPDX-License-Identifier: Apache-2.0 use crate::async_io::{AsyncIo, AsyncIoResult, DiskFile, DiskFileError, DiskFileResult}; -use crate::{fsync_sync, read_vectored_sync, write_vectored_sync, ReadWriteSeekFile}; +use crate::AsyncAdaptor; use std::fs::File; -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Mutex, MutexGuard}; use vhdx::vhdx::{Result as VhdxResult, Vhdx}; use vmm_sys_util::eventfd::EventFd; pub struct VhdxDiskSync { - vhdx_file: Arc>, + vhdx_file: Arc>, } impl VhdxDiskSync { @@ -23,14 +23,7 @@ impl VhdxDiskSync { impl DiskFile for VhdxDiskSync { fn size(&mut self) -> DiskFileResult { - Ok(self - .vhdx_file - .lock() - .unwrap() - .as_any() - .downcast_ref::() - .unwrap() - .virtual_disk_size()) + Ok(self.vhdx_file.lock().unwrap().virtual_disk_size()) } fn new_async_io(&self, _ring_depth: u32) -> DiskFileResult> { @@ -42,15 +35,13 @@ impl DiskFile for VhdxDiskSync { } pub struct VhdxSync { - vhdx_file: Arc>, + vhdx_file: Arc>, eventfd: EventFd, completion_list: Vec<(u64, i32)>, } impl VhdxSync { - pub fn new( - vhdx_file: Arc>, - ) -> std::io::Result { + pub fn new(vhdx_file: Arc>) -> std::io::Result { Ok(VhdxSync { vhdx_file, eventfd: EventFd::new(libc::EFD_NONBLOCK)?, @@ -59,6 +50,12 @@ impl VhdxSync { } } +impl AsyncAdaptor for Arc> { + fn file(&mut self) -> MutexGuard { + self.lock().unwrap() + } +} + impl AsyncIo for VhdxSync { fn notifier(&self) -> &EventFd { &self.eventfd @@ -70,11 +67,10 @@ impl AsyncIo for VhdxSync { iovecs: Vec, user_data: u64, ) -> AsyncIoResult<()> { - read_vectored_sync( + self.vhdx_file.read_vectored_sync( offset, iovecs, user_data, - &mut self.vhdx_file, &self.eventfd, &mut self.completion_list, ) @@ -86,23 +82,18 @@ impl AsyncIo for VhdxSync { iovecs: Vec, user_data: u64, ) -> AsyncIoResult<()> { - write_vectored_sync( + self.vhdx_file.write_vectored_sync( offset, iovecs, user_data, - &mut self.vhdx_file, &self.eventfd, &mut self.completion_list, ) } fn fsync(&mut self, user_data: Option) -> AsyncIoResult<()> { - fsync_sync( - user_data, - &mut self.vhdx_file, - &self.eventfd, - &mut self.completion_list, - ) + self.vhdx_file + .fsync_sync(user_data, &self.eventfd, &mut self.completion_list) } fn complete(&mut self) -> Vec<(u64, i32)> {