diff --git a/Cargo.lock b/Cargo.lock index c3547d469..30dfe7dff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -739,9 +739,9 @@ dependencies = [ [[package]] name = "imago" -version = "0.1.5" +version = "0.1.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6a94a5babb4a05f462d7736c7263ddd7e721ec0c5e5f26b02d888e9edf75b549" +checksum = "a8e45dce78f8a4700ea570488b35af0d192ee0dd355a1d625f07f6ea49ac25bd" dependencies = [ "async-trait", "bincode", diff --git a/src/devices/Cargo.toml b/src/devices/Cargo.toml index b5024d75a..298c4dcdf 100644 --- a/src/devices/Cargo.toml +++ b/src/devices/Cargo.toml @@ -38,7 +38,7 @@ arch = { path = "../arch" } utils = { path = "../utils" } polly = { path = "../polly" } rutabaga_gfx = { path = "../rutabaga_gfx", features = ["virgl_renderer", "virgl_renderer_next"], optional = true } -imago = { version = "0.1.5", features = ["sync-wrappers", "vm-memory"] } +imago = { version = "0.1.6", features = ["sync-wrappers", "vm-memory"] } [target.'cfg(target_os = "macos")'.dependencies] hvf = { path = "../hvf" } diff --git a/src/devices/src/virtio/block/device.rs b/src/devices/src/virtio/block/device.rs index c4999accc..5ba02319e 100644 --- a/src/devices/src/virtio/block/device.rs +++ b/src/devices/src/virtio/block/device.rs @@ -15,7 +15,7 @@ use std::os::linux::fs::MetadataExt; use std::os::macos::fs::MetadataExt; use std::path::PathBuf; use std::result; -use std::sync::Arc; +use std::sync::{Arc, Mutex}; use std::thread::JoinHandle; use imago::{ @@ -65,18 +65,18 @@ impl CacheType { /// Helper object for setting up all `Block` fields derived from its backing file. pub(crate) struct DiskProperties { cache_type: CacheType, - pub(crate) file: Arc>>, + pub(crate) file: Arc>>>, nsectors: u64, image_id: Vec, } impl DiskProperties { pub fn new( - disk_image: Arc>>, + disk_image: Arc>>>, disk_image_id: Vec, cache_type: CacheType, ) -> io::Result { - let disk_size = disk_image.size(); + let disk_size = disk_image.lock().unwrap().size(); // We only support disk size, which uses the first two words of the configuration space. // If the image is not a multiple of the sector size, the tail bits are not exposed. @@ -95,10 +95,6 @@ impl DiskProperties { }) } - pub fn file(&self) -> &SyncFormatAccess> { - self.file.as_ref() - } - pub fn nsectors(&self) -> u64 { self.nsectors } @@ -146,11 +142,11 @@ impl Drop for DiskProperties { match self.cache_type { CacheType::Writeback => { // flush() first to force any cached data out. - if self.file.flush().is_err() { + if self.file.lock().unwrap().flush().is_err() { error!("Failed to flush block data on drop."); } // Sync data out to physical media on host. - if self.file.sync().is_err() { + if self.file.lock().unwrap().sync().is_err() { error!("Failed to sync block data on drop.") } } @@ -161,12 +157,41 @@ impl Drop for DiskProperties { } } +#[derive(Copy, Clone, Debug, Default)] +#[repr(C, packed)] +struct VirtioBlkGeometry { + cylinders: u16, + heads: u8, + sectors: u8, +} + +#[derive(Copy, Clone, Debug, Default)] +#[repr(C, packed)] +struct VirtioBlkTopology { + physical_block_exp: u8, + alignment_offset: u8, + min_io_size: u16, + opt_io_size: u32, +} + #[derive(Copy, Clone, Debug, Default)] #[repr(C, packed)] struct VirtioBlkConfig { capacity: u64, size_max: u32, seg_max: u32, + geometry: VirtioBlkGeometry, + blk_size: u32, + topology: VirtioBlkTopology, + writeback: u8, + unused0: u8, + num_queues: u16, + max_discard_sectors: u32, + max_discard_seg: u32, + discard_sector_alignment: u32, + max_write_zeroes_sectors: u32, + max_write_zeroes_seg: u32, + write_zeroes_may_unmap: u8, } // Safe because it only has data and has no implicit padding. @@ -177,7 +202,7 @@ pub struct Block { // Host file and properties. disk: Option, cache_type: CacheType, - disk_image: Arc>>, + disk_image: Arc>>>, disk_image_id: Vec, worker_thread: Option>, worker_stopfd: EventFd, @@ -222,6 +247,7 @@ impl Block { #[cfg(target_os = "macos")] let file_opts = file_opts.relaxed_sync(true); let file = ImagoFile::open_sync(file_opts)?; + let discard_alignment = file.discard_align(); let disk_image = match disk_image_format { ImageType::Qcow2 => { @@ -242,14 +268,16 @@ impl Block { } }; - let disk_image = Arc::new(disk_image); + let disk_image = Arc::new(Mutex::new(disk_image)); let disk_properties = - DiskProperties::new(Arc::clone(&disk_image), disk_image_id.clone(), cache_type)?; + DiskProperties::new(disk_image.clone(), disk_image_id.clone(), cache_type)?; let mut avail_features = (1u64 << VIRTIO_F_VERSION_1) | (1u64 << VIRTIO_BLK_F_FLUSH) | (1u64 << VIRTIO_BLK_F_SEG_MAX) + | (1u64 << VIRTIO_BLK_F_DISCARD) + | (1u64 << VIRTIO_BLK_F_WRITE_ZEROES) | (1u64 << VIRTIO_RING_F_EVENT_IDX); if is_disk_read_only { @@ -265,6 +293,13 @@ impl Block { size_max: 0, // QUEUE_SIZE - 2 seg_max: 254, + max_discard_sectors: u32::MAX, + max_discard_seg: 1, + discard_sector_alignment: discard_alignment as u32 / 512, + max_write_zeroes_sectors: u32::MAX, + max_write_zeroes_seg: 1, + write_zeroes_may_unmap: 1, + ..Default::default() }; Ok(Block { diff --git a/src/devices/src/virtio/block/worker.rs b/src/devices/src/virtio/block/worker.rs index 2175b3bf6..195a7d22b 100644 --- a/src/devices/src/virtio/block/worker.rs +++ b/src/devices/src/virtio/block/worker.rs @@ -16,10 +16,13 @@ use vm_memory::{ByteValued, GuestMemoryMmap}; #[allow(dead_code)] #[derive(Debug)] pub enum RequestError { + Discarding(io::Error), + DiscardingToZero(io::Error), FlushingToDisk(io::Error), InvalidDataLength, ReadingFromDescriptor(io::Error), WritingToDescriptor(io::Error), + WritingZeroes(io::Error), UnknownRequest, } @@ -39,10 +42,19 @@ pub struct RequestHeader { _reserved: u32, sector: u64, } - // Safe because RequestHeader only contains plain data. unsafe impl ByteValued for RequestHeader {} +#[derive(Copy, Clone, Default)] +#[repr(C)] +pub struct DiscardWriteData { + sector: u64, + num_sectors: u32, + flags: u32, +} +// Safe because DiscardWriteData only contains plain data. +unsafe impl ByteValued for DiscardWriteData {} + pub struct BlockWorker { queue: Queue, queue_evt: EventFd, @@ -228,7 +240,7 @@ impl BlockWorker { } VIRTIO_BLK_T_FLUSH => match self.disk.cache_type() { CacheType::Writeback => { - let diskfile = self.disk.file(); + let diskfile = self.disk.file.lock().unwrap(); diskfile.flush().map_err(RequestError::FlushingToDisk)?; diskfile.sync().map_err(RequestError::FlushingToDisk)?; Ok(0) @@ -247,6 +259,49 @@ impl BlockWorker { Ok(disk_id.len()) } } + VIRTIO_BLK_T_DISCARD => { + let discard_write_data: DiscardWriteData = reader + .read_obj() + .map_err(RequestError::ReadingFromDescriptor)?; + self.disk + .file + .lock() + .unwrap() + .discard_to_any( + discard_write_data.sector * 512, + discard_write_data.num_sectors as u64 * 512, + ) + .map_err(RequestError::Discarding)?; + Ok(0) + } + VIRTIO_BLK_T_WRITE_ZEROES => { + let discard_write_data: DiscardWriteData = reader + .read_obj() + .map_err(RequestError::ReadingFromDescriptor)?; + let unmap = (discard_write_data.flags & VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP) != 0; + if unmap { + self.disk + .file + .lock() + .unwrap() + .discard_to_zero( + discard_write_data.sector * 512, + discard_write_data.num_sectors as u64 * 512, + ) + .map_err(RequestError::DiscardingToZero)?; + } else { + self.disk + .file + .lock() + .unwrap() + .write_zeroes( + discard_write_data.sector * 512, + discard_write_data.num_sectors as u64 * 512, + ) + .map_err(RequestError::WritingZeroes)?; + } + Ok(0) + } _ => Err(RequestError::UnknownRequest), } } diff --git a/src/devices/src/virtio/file_traits.rs b/src/devices/src/virtio/file_traits.rs index 72ab0e315..e7d2ca17d 100644 --- a/src/devices/src/virtio/file_traits.rs +++ b/src/devices/src/virtio/file_traits.rs @@ -432,7 +432,7 @@ impl FileReadWriteAtVolatile for DiskProperties { .len() .try_into() .map_err(|e| Error::new(ErrorKind::InvalidData, e))?; - self.file().readv(iovec, offset)?; + self.file.lock().unwrap().readv(iovec, offset)?; Ok(full_length) } @@ -450,7 +450,7 @@ impl FileReadWriteAtVolatile for DiskProperties { .len() .try_into() .map_err(|e| Error::new(ErrorKind::InvalidData, e))?; - self.file().writev(iovec, offset)?; + self.file.lock().unwrap().writev(iovec, offset)?; Ok(full_length) } }