From 4963e37dc8688700ef0547c21640b4e1f96643bc Mon Sep 17 00:00:00 2001 From: Rob Bradford Date: Fri, 10 Jul 2020 15:43:12 +0100 Subject: [PATCH] qcow, virtio-devices: Break cyclic dependency Move the definition of RawFile from virtio-devices crate into qcow crate. All the code that consumes RawFile also already depends on the qcow crate for image file type detection so this change breaks the need for the qcow crate to depend on the very large virtio-devices crate. Signed-off-by: Rob Bradford --- Cargo.lock | 1 - qcow/Cargo.toml | 1 - qcow/src/qcow.rs | 22 ++- qcow/src/qcow_raw_file.rs | 2 +- qcow/src/raw_file.rs | 324 ++++++++++++++++++++++++++++++++++++ vhost_user_block/src/lib.rs | 2 +- virtio-devices/src/block.rs | 317 +---------------------------------- vmm/src/device_manager.rs | 2 +- 8 files changed, 341 insertions(+), 330 deletions(-) create mode 100644 qcow/src/raw_file.rs diff --git a/Cargo.lock b/Cargo.lock index 60777f7ac..a50869662 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -894,7 +894,6 @@ dependencies = [ "log 0.4.8", "remain", "tempfile", - "virtio-devices", "vmm-sys-util", ] diff --git a/qcow/Cargo.toml b/qcow/Cargo.toml index 7b3160c13..c0f21b32b 100644 --- a/qcow/Cargo.toml +++ b/qcow/Cargo.toml @@ -14,7 +14,6 @@ libc = "0.2.72" log = "0.4.8" remain = "0.2.2" vmm-sys-util = ">=0.3.1" -virtio-devices = { path = "../virtio-devices" } [dev-dependencies] tempfile = "3.1.0" diff --git a/qcow/src/qcow.rs b/qcow/src/qcow.rs index 95bb297b7..5fa68c8b3 100644 --- a/qcow/src/qcow.rs +++ b/qcow/src/qcow.rs @@ -6,26 +6,26 @@ extern crate log; mod qcow_raw_file; +mod raw_file; mod refcount; mod vec_cache; +use crate::qcow_raw_file::QcowRawFile; +use crate::refcount::RefCount; +use crate::vec_cache::{CacheMap, Cacheable, VecCache}; use byteorder::{BigEndian, ReadBytesExt, WriteBytesExt}; use libc::{EINVAL, ENOSPC, ENOTSUP}; use remain::sorted; -use virtio_devices::RawFile; +use std::cmp::{max, min}; +use std::fmt::{self, Display}; +use std::io::{self, Read, Seek, SeekFrom, Write}; +use std::mem::size_of; use vmm_sys_util::{ file_traits::FileSetLen, file_traits::FileSync, seek_hole::SeekHole, write_zeroes::PunchHole, write_zeroes::WriteZeroes, }; -use std::cmp::{max, min}; -use std::fmt::{self, Display}; -use std::io::{self, Read, Seek, SeekFrom, Write}; -use std::mem::size_of; - -use crate::qcow_raw_file::QcowRawFile; -use crate::refcount::RefCount; -use crate::vec_cache::{CacheMap, Cacheable, VecCache}; +pub use crate::raw_file::RawFile; #[sorted] #[derive(Debug)] @@ -367,8 +367,7 @@ fn max_refcount_clusters(refcount_order: u32, cluster_size: u32, num_clusters: u /// /// ``` /// # use std::io::{Read, Seek, SeekFrom}; -/// # use virtio_devices::RawFile; -/// # use qcow::{self, QcowFile}; +/// # use qcow::{self, QcowFile, RawFile}; /// # fn test(file: std::fs::File) -> std::io::Result<()> { /// let mut raw_img = RawFile::new(file, false); /// let mut q = QcowFile::from(raw_img).expect("Can't open qcow file"); @@ -1703,7 +1702,6 @@ mod tests { use super::*; use std::io::{Read, Seek, SeekFrom, Write}; use tempfile::tempfile; - use virtio_devices::RawFile; fn valid_header_v3() -> Vec { vec![ diff --git a/qcow/src/qcow_raw_file.rs b/qcow/src/qcow_raw_file.rs index ff81584b2..2c56cef23 100644 --- a/qcow/src/qcow_raw_file.rs +++ b/qcow/src/qcow_raw_file.rs @@ -2,10 +2,10 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE-BSD-3-Clause file. +use super::RawFile; use byteorder::{BigEndian, ReadBytesExt, WriteBytesExt}; use std::io::{self, BufWriter, Seek, SeekFrom}; use std::mem::size_of; -use virtio_devices::RawFile; use vmm_sys_util::write_zeroes::WriteZeroes; /// A qcow file. Allows reading/writing clusters and appending clusters. diff --git a/qcow/src/raw_file.rs b/qcow/src/raw_file.rs new file mode 100644 index 000000000..23d189033 --- /dev/null +++ b/qcow/src/raw_file.rs @@ -0,0 +1,324 @@ +// Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// +// Portions Copyright 2017 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE-BSD-3-Clause file. +// +// Copyright © 2020 Intel Corporation +// +// SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause + +use libc::c_void; +use std::alloc::{alloc_zeroed, dealloc, Layout}; +use std::convert::TryInto; +use std::fs::{File, Metadata}; +use std::io::{self, Read, Seek, SeekFrom, Write}; +use std::os::unix::io::{AsRawFd, RawFd}; +use std::slice; +use vmm_sys_util::{seek_hole::SeekHole, write_zeroes::PunchHole}; + +#[derive(Debug)] +pub struct RawFile { + file: File, + alignment: usize, + position: u64, +} + +const BLK_ALIGNMENTS: [usize; 2] = [512, 4096]; + +fn is_valid_alignment(fd: RawFd, alignment: usize) -> bool { + let layout = Layout::from_size_align(alignment, alignment).unwrap(); + let ptr = unsafe { alloc_zeroed(layout) }; + + let ret = unsafe { + ::libc::pread( + fd, + ptr as *mut c_void, + alignment, + alignment.try_into().unwrap(), + ) + }; + + unsafe { dealloc(ptr, layout) }; + + ret >= 0 +} + +impl RawFile { + pub fn new(file: File, direct_io: bool) -> Self { + // Assume no alignment restrictions if we aren't using O_DIRECT. + let mut alignment = 0; + if direct_io { + for align in &BLK_ALIGNMENTS { + if is_valid_alignment(file.as_raw_fd(), *align) { + alignment = *align; + break; + } + } + } + RawFile { + file, + alignment: alignment.try_into().unwrap(), + position: 0, + } + } + + fn round_up(&self, offset: u64) -> u64 { + let align: u64 = self.alignment.try_into().unwrap(); + ((offset / (align + 1)) + 1) * align + } + + fn round_down(&self, offset: u64) -> u64 { + let align: u64 = self.alignment.try_into().unwrap(); + (offset / align) * align + } + + fn is_aligned(&self, buf: &[u8]) -> bool { + if self.alignment == 0 { + return true; + } + + let align64: u64 = self.alignment.try_into().unwrap(); + + (self.position % align64 == 0) + && ((buf.as_ptr() as usize) % self.alignment == 0) + && (buf.len() % self.alignment == 0) + } + + pub fn set_len(&self, size: u64) -> std::io::Result<()> { + self.file.set_len(size) + } + + pub fn metadata(&self) -> std::io::Result { + self.file.metadata() + } + + pub fn try_clone(&self) -> std::io::Result { + Ok(RawFile { + file: self.file.try_clone().expect("RawFile cloning failed"), + alignment: self.alignment, + position: self.position, + }) + } + + pub fn sync_all(&self) -> std::io::Result<()> { + self.file.sync_all() + } + + pub fn sync_data(&self) -> std::io::Result<()> { + self.file.sync_data() + } +} + +impl Read for RawFile { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + if self.is_aligned(buf) { + match self.file.read(buf) { + Ok(r) => { + self.position = self.position.checked_add(r.try_into().unwrap()).unwrap(); + Ok(r) + } + Err(e) => Err(e), + } + } else { + let rounded_pos: u64 = self.round_down(self.position); + let file_offset: usize = self + .position + .checked_sub(rounded_pos) + .unwrap() + .try_into() + .unwrap(); + let buf_len: usize = buf.len(); + let rounded_len: usize = self + .round_up( + file_offset + .checked_add(buf_len) + .unwrap() + .try_into() + .unwrap(), + ) + .try_into() + .unwrap(); + + let layout = Layout::from_size_align(rounded_len, self.alignment).unwrap(); + let tmp_ptr = unsafe { alloc_zeroed(layout) }; + let tmp_buf = unsafe { slice::from_raw_parts_mut(tmp_ptr, rounded_len) }; + + // This can eventually replaced with read_at once its interface + // has been stabilized. + let ret = unsafe { + ::libc::pread64( + self.file.as_raw_fd(), + tmp_buf.as_mut_ptr() as *mut c_void, + tmp_buf.len(), + rounded_pos.try_into().unwrap(), + ) + }; + if ret < 0 { + unsafe { dealloc(tmp_ptr, layout) }; + return Err(io::Error::last_os_error()); + } + + let read: usize = ret.try_into().unwrap(); + if read < file_offset { + unsafe { dealloc(tmp_ptr, layout) }; + return Ok(0); + } + + let mut to_copy = read - file_offset; + if to_copy > buf_len { + to_copy = buf_len; + } + + buf.copy_from_slice(&tmp_buf[file_offset..(file_offset + buf_len)]); + unsafe { dealloc(tmp_ptr, layout) }; + + self.seek(SeekFrom::Current(to_copy.try_into().unwrap())) + .unwrap(); + Ok(to_copy.try_into().unwrap()) + } + } +} + +impl Write for RawFile { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + if self.is_aligned(buf) { + match self.file.write(buf) { + Ok(r) => { + self.position = self.position.checked_add(r.try_into().unwrap()).unwrap(); + Ok(r) + } + Err(e) => Err(e), + } + } else { + let rounded_pos: u64 = self.round_down(self.position); + let file_offset: usize = self + .position + .checked_sub(rounded_pos) + .unwrap() + .try_into() + .unwrap(); + let buf_len: usize = buf.len(); + let rounded_len: usize = self + .round_up( + file_offset + .checked_add(buf_len) + .unwrap() + .try_into() + .unwrap(), + ) + .try_into() + .unwrap(); + + let layout = Layout::from_size_align(rounded_len, self.alignment).unwrap(); + let tmp_ptr = unsafe { alloc_zeroed(layout) }; + let tmp_buf = unsafe { slice::from_raw_parts_mut(tmp_ptr, rounded_len) }; + + // This can eventually replaced with read_at once its interface + // has been stabilized. + let ret = unsafe { + ::libc::pread64( + self.file.as_raw_fd(), + tmp_buf.as_mut_ptr() as *mut c_void, + tmp_buf.len(), + rounded_pos.try_into().unwrap(), + ) + }; + if ret < 0 { + unsafe { dealloc(tmp_ptr, layout) }; + return Err(io::Error::last_os_error()); + }; + + tmp_buf[file_offset..(file_offset + buf_len)].copy_from_slice(buf); + + // This can eventually replaced with write_at once its interface + // has been stabilized. + let ret = unsafe { + ::libc::pwrite64( + self.file.as_raw_fd(), + tmp_buf.as_ptr() as *const c_void, + tmp_buf.len(), + rounded_pos.try_into().unwrap(), + ) + }; + + unsafe { dealloc(tmp_ptr, layout) }; + + if ret < 0 { + return Err(io::Error::last_os_error()); + } + + let written: usize = ret.try_into().unwrap(); + if written < file_offset { + Ok(0) + } else { + let mut to_seek = written - file_offset; + if to_seek > buf_len { + to_seek = buf_len; + } + + self.seek(SeekFrom::Current(to_seek.try_into().unwrap())) + .unwrap(); + Ok(to_seek.try_into().unwrap()) + } + } + } + + fn flush(&mut self) -> std::io::Result<()> { + self.file.sync_all() + } +} + +impl Seek for RawFile { + fn seek(&mut self, newpos: SeekFrom) -> std::io::Result { + match self.file.seek(newpos) { + Ok(pos) => { + self.position = pos; + Ok(pos) + } + Err(e) => Err(e), + } + } +} + +impl PunchHole for RawFile { + fn punch_hole(&mut self, offset: u64, length: u64) -> std::io::Result<()> { + self.file.punch_hole(offset, length) + } +} + +impl SeekHole for RawFile { + fn seek_hole(&mut self, offset: u64) -> std::io::Result> { + match self.file.seek_hole(offset) { + Ok(pos) => { + if let Some(p) = pos { + self.position = p; + } + Ok(pos) + } + Err(e) => Err(e), + } + } + + fn seek_data(&mut self, offset: u64) -> std::io::Result> { + match self.file.seek_data(offset) { + Ok(pos) => { + if let Some(p) = pos { + self.position = p; + } + Ok(pos) + } + Err(e) => Err(e), + } + } +} + +impl Clone for RawFile { + fn clone(&self) -> Self { + RawFile { + file: self.file.try_clone().expect("RawFile cloning failed"), + alignment: self.alignment, + position: self.position, + } + } +} diff --git a/vhost_user_block/src/lib.rs b/vhost_user_block/src/lib.rs index 4b573ec98..7aa16b18c 100644 --- a/vhost_user_block/src/lib.rs +++ b/vhost_user_block/src/lib.rs @@ -207,7 +207,7 @@ impl VhostUserBlkBackend { options.custom_flags(libc::O_DIRECT); } let image: File = options.open(&image_path).unwrap(); - let mut raw_img: virtio_devices::RawFile = virtio_devices::RawFile::new(image, direct); + let mut raw_img: qcow::RawFile = qcow::RawFile::new(image, direct); let image_id = build_disk_image_id(&PathBuf::from(&image_path)); let image_type = qcow::detect_image_type(&mut raw_img).unwrap(); diff --git a/virtio-devices/src/block.rs b/virtio-devices/src/block.rs index 6f14eb140..be3611d45 100644 --- a/virtio-devices/src/block.rs +++ b/virtio-devices/src/block.rs @@ -15,21 +15,18 @@ use super::{ }; use crate::VirtioInterrupt; use anyhow::anyhow; -use libc::{c_void, EFD_NONBLOCK}; +use libc::EFD_NONBLOCK; use serde::ser::{Serialize, SerializeStruct, Serializer}; -use std::alloc::{alloc_zeroed, dealloc, Layout}; use std::cmp; use std::collections::HashMap; -use std::convert::TryInto; -use std::fs::{File, Metadata}; +use std::fs::File; use std::io::{self, Read, Seek, SeekFrom, Write}; use std::num::Wrapping; use std::ops::DerefMut; use std::os::linux::fs::MetadataExt; -use std::os::unix::io::{AsRawFd, FromRawFd, RawFd}; +use std::os::unix::io::{AsRawFd, FromRawFd}; use std::path::PathBuf; use std::result; -use std::slice; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::sync::{Arc, Mutex}; use std::thread; @@ -43,7 +40,7 @@ use vm_migration::{ Migratable, MigratableError, Pausable, Snapshot, SnapshotDataSection, Snapshottable, Transportable, }; -use vmm_sys_util::{eventfd::EventFd, seek_hole::SeekHole, write_zeroes::PunchHole}; +use vmm_sys_util::eventfd::EventFd; const SECTOR_SHIFT: u8 = 9; pub const SECTOR_SIZE: u64 = (0x01 as u64) << SECTOR_SHIFT; @@ -103,312 +100,6 @@ impl ExecuteError { pub trait DiskFile: Read + Seek + Write + Clone {} impl DiskFile for D {} -#[derive(Debug)] -pub struct RawFile { - file: File, - alignment: usize, - position: u64, -} - -const BLK_ALIGNMENTS: [usize; 2] = [512, 4096]; - -fn is_valid_alignment(fd: RawFd, alignment: usize) -> bool { - let layout = Layout::from_size_align(alignment, alignment).unwrap(); - let ptr = unsafe { alloc_zeroed(layout) }; - - let ret = unsafe { - ::libc::pread( - fd, - ptr as *mut c_void, - alignment, - alignment.try_into().unwrap(), - ) - }; - - unsafe { dealloc(ptr, layout) }; - - ret >= 0 -} - -impl RawFile { - pub fn new(file: File, direct_io: bool) -> Self { - // Assume no alignment restrictions if we aren't using O_DIRECT. - let mut alignment = 0; - if direct_io { - for align in &BLK_ALIGNMENTS { - if is_valid_alignment(file.as_raw_fd(), *align) { - alignment = *align; - break; - } - } - } - RawFile { - file, - alignment: alignment.try_into().unwrap(), - position: 0, - } - } - - fn round_up(&self, offset: u64) -> u64 { - let align: u64 = self.alignment.try_into().unwrap(); - ((offset / (align + 1)) + 1) * align - } - - fn round_down(&self, offset: u64) -> u64 { - let align: u64 = self.alignment.try_into().unwrap(); - (offset / align) * align - } - - fn is_aligned(&self, buf: &[u8]) -> bool { - if self.alignment == 0 { - return true; - } - - let align64: u64 = self.alignment.try_into().unwrap(); - - (self.position % align64 == 0) - && ((buf.as_ptr() as usize) % self.alignment == 0) - && (buf.len() % self.alignment == 0) - } - - pub fn set_len(&self, size: u64) -> std::io::Result<()> { - self.file.set_len(size) - } - - pub fn metadata(&self) -> std::io::Result { - self.file.metadata() - } - - pub fn try_clone(&self) -> std::io::Result { - Ok(RawFile { - file: self.file.try_clone().expect("RawFile cloning failed"), - alignment: self.alignment, - position: self.position, - }) - } - - pub fn sync_all(&self) -> std::io::Result<()> { - self.file.sync_all() - } - - pub fn sync_data(&self) -> std::io::Result<()> { - self.file.sync_data() - } -} - -impl Read for RawFile { - fn read(&mut self, buf: &mut [u8]) -> std::io::Result { - if self.is_aligned(buf) { - match self.file.read(buf) { - Ok(r) => { - self.position = self.position.checked_add(r.try_into().unwrap()).unwrap(); - Ok(r) - } - Err(e) => Err(e), - } - } else { - let rounded_pos: u64 = self.round_down(self.position); - let file_offset: usize = self - .position - .checked_sub(rounded_pos) - .unwrap() - .try_into() - .unwrap(); - let buf_len: usize = buf.len(); - let rounded_len: usize = self - .round_up( - file_offset - .checked_add(buf_len) - .unwrap() - .try_into() - .unwrap(), - ) - .try_into() - .unwrap(); - - let layout = Layout::from_size_align(rounded_len, self.alignment).unwrap(); - let tmp_ptr = unsafe { alloc_zeroed(layout) }; - let tmp_buf = unsafe { slice::from_raw_parts_mut(tmp_ptr, rounded_len) }; - - // This can eventually replaced with read_at once its interface - // has been stabilized. - let ret = unsafe { - ::libc::pread64( - self.file.as_raw_fd(), - tmp_buf.as_mut_ptr() as *mut c_void, - tmp_buf.len(), - rounded_pos.try_into().unwrap(), - ) - }; - if ret < 0 { - unsafe { dealloc(tmp_ptr, layout) }; - return Err(io::Error::last_os_error()); - } - - let read: usize = ret.try_into().unwrap(); - if read < file_offset { - unsafe { dealloc(tmp_ptr, layout) }; - return Ok(0); - } - - let mut to_copy = read - file_offset; - if to_copy > buf_len { - to_copy = buf_len; - } - - buf.copy_from_slice(&tmp_buf[file_offset..(file_offset + buf_len)]); - unsafe { dealloc(tmp_ptr, layout) }; - - self.seek(SeekFrom::Current(to_copy.try_into().unwrap())) - .unwrap(); - Ok(to_copy.try_into().unwrap()) - } - } -} - -impl Write for RawFile { - fn write(&mut self, buf: &[u8]) -> std::io::Result { - if self.is_aligned(buf) { - match self.file.write(buf) { - Ok(r) => { - self.position = self.position.checked_add(r.try_into().unwrap()).unwrap(); - Ok(r) - } - Err(e) => Err(e), - } - } else { - let rounded_pos: u64 = self.round_down(self.position); - let file_offset: usize = self - .position - .checked_sub(rounded_pos) - .unwrap() - .try_into() - .unwrap(); - let buf_len: usize = buf.len(); - let rounded_len: usize = self - .round_up( - file_offset - .checked_add(buf_len) - .unwrap() - .try_into() - .unwrap(), - ) - .try_into() - .unwrap(); - - let layout = Layout::from_size_align(rounded_len, self.alignment).unwrap(); - let tmp_ptr = unsafe { alloc_zeroed(layout) }; - let tmp_buf = unsafe { slice::from_raw_parts_mut(tmp_ptr, rounded_len) }; - - // This can eventually replaced with read_at once its interface - // has been stabilized. - let ret = unsafe { - ::libc::pread64( - self.file.as_raw_fd(), - tmp_buf.as_mut_ptr() as *mut c_void, - tmp_buf.len(), - rounded_pos.try_into().unwrap(), - ) - }; - if ret < 0 { - unsafe { dealloc(tmp_ptr, layout) }; - return Err(io::Error::last_os_error()); - }; - - tmp_buf[file_offset..(file_offset + buf_len)].copy_from_slice(buf); - - // This can eventually replaced with write_at once its interface - // has been stabilized. - let ret = unsafe { - ::libc::pwrite64( - self.file.as_raw_fd(), - tmp_buf.as_ptr() as *const c_void, - tmp_buf.len(), - rounded_pos.try_into().unwrap(), - ) - }; - - unsafe { dealloc(tmp_ptr, layout) }; - - if ret < 0 { - return Err(io::Error::last_os_error()); - } - - let written: usize = ret.try_into().unwrap(); - if written < file_offset { - Ok(0) - } else { - let mut to_seek = written - file_offset; - if to_seek > buf_len { - to_seek = buf_len; - } - - self.seek(SeekFrom::Current(to_seek.try_into().unwrap())) - .unwrap(); - Ok(to_seek.try_into().unwrap()) - } - } - } - - fn flush(&mut self) -> std::io::Result<()> { - self.file.sync_all() - } -} - -impl Seek for RawFile { - fn seek(&mut self, newpos: SeekFrom) -> std::io::Result { - match self.file.seek(newpos) { - Ok(pos) => { - self.position = pos; - Ok(pos) - } - Err(e) => Err(e), - } - } -} - -impl PunchHole for RawFile { - fn punch_hole(&mut self, offset: u64, length: u64) -> std::io::Result<()> { - self.file.punch_hole(offset, length) - } -} - -impl SeekHole for RawFile { - fn seek_hole(&mut self, offset: u64) -> std::io::Result> { - match self.file.seek_hole(offset) { - Ok(pos) => { - if let Some(p) = pos { - self.position = p; - } - Ok(pos) - } - Err(e) => Err(e), - } - } - - fn seek_data(&mut self, offset: u64) -> std::io::Result> { - match self.file.seek_data(offset) { - Ok(pos) => { - if let Some(p) = pos { - self.position = p; - } - Ok(pos) - } - Err(e) => Err(e), - } - } -} - -impl Clone for RawFile { - fn clone(&self) -> Self { - RawFile { - file: self.file.try_clone().expect("RawFile cloning failed"), - alignment: self.alignment, - position: self.position, - } - } -} - #[derive(Clone, Copy, Debug, PartialEq)] pub enum RequestType { In, diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 02abad770..bdf1c293c 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -1602,7 +1602,7 @@ impl DeviceManager { ) .map_err(DeviceManagerError::Disk)?; - let mut raw_img = virtio_devices::RawFile::new(image, disk_cfg.direct); + let mut raw_img = qcow::RawFile::new(image, disk_cfg.direct); let image_type = qcow::detect_image_type(&mut raw_img) .map_err(DeviceManagerError::DetectImageType)?;