From 43d53886f786668b2797e9e3b6b4066a454612a8 Mon Sep 17 00:00:00 2001 From: zjregee Date: Tue, 13 Aug 2024 15:29:09 +0800 Subject: [PATCH] feat(ovfs): add lookup and unit tests (#4997) * feat: add lookup * feat: add tests for reader and writer * typo * typo * fix cargo fmt check --- integrations/virtiofs/src/filesystem.rs | 66 ++++++- .../virtiofs/src/filesystem_message.rs | 20 ++ integrations/virtiofs/src/virtiofs_util.rs | 178 ++++++++++++++++++ 3 files changed, 261 insertions(+), 3 deletions(-) diff --git a/integrations/virtiofs/src/filesystem.rs b/integrations/virtiofs/src/filesystem.rs index 4fb38b15dc2..a9fe6a8f37e 100644 --- a/integrations/virtiofs/src/filesystem.rs +++ b/integrations/virtiofs/src/filesystem.rs @@ -15,8 +15,11 @@ // specific language governing permissions and limitations // under the License. +use std::collections::HashMap; +use std::io::Read; use std::io::Write; use std::mem::size_of; +use std::sync::Mutex; use std::time::Duration; use log::debug; @@ -51,6 +54,7 @@ enum FileType { File, } +#[derive(Clone)] struct OpenedFile { path: String, metadata: Attr, @@ -113,6 +117,9 @@ pub struct Filesystem { uid: u32, gid: u32, opened_files: Slab, + // Since we need to manually manage the allocation of inodes, + // we record the inode of each opened file here. + opened_files_map: Mutex>, } impl Filesystem { @@ -130,6 +137,7 @@ impl Filesystem { uid: 1000, gid: 1000, opened_files: Slab::new(), + opened_files_map: Mutex::new(HashMap::new()), } } @@ -145,6 +153,7 @@ impl Filesystem { match opcode { Opcode::Init => self.init(in_header, r, w), Opcode::Destroy => self.destroy(in_header, r, w), + Opcode::Lookup => self.lookup(in_header, r, w), Opcode::Getattr => self.getattr(in_header, r, w), Opcode::Setattr => self.setattr(in_header, r, w), } @@ -227,6 +236,46 @@ impl Filesystem { Ok(0) } + fn lookup(&self, in_header: InHeader, mut r: Reader, w: Writer) -> Result { + let name_len = in_header.len as usize - size_of::(); + let mut buf = vec![0u8; name_len]; + r.read_exact(&mut buf).map_err(|e| { + new_unexpected_error("failed to decode protocol messages", Some(e.into())) + })?; + let name = String::from_utf8(buf).map_err(|e| { + new_unexpected_error("failed to decode protocol messages", Some(e.into())) + })?; + + debug!("lookup: parent inode={} name={}", in_header.nodeid, name); + + let parent_path = match self + .opened_files + .get(in_header.nodeid as usize) + .map(|f| f.path.clone()) + { + Some(path) => path, + None => return Filesystem::reply_error(in_header.unique, w), + }; + + let path = format!("{}/{}", parent_path, name); + let metadata = match self.rt.block_on(self.do_get_metadata(&path)) { + Ok(metadata) => metadata, + Err(_) => return Filesystem::reply_error(in_header.unique, w), + }; + + let out = EntryOut { + nodeid: metadata.metadata.ino, + entry_valid: DEFAULT_TTL.as_secs(), + attr_valid: DEFAULT_TTL.as_secs(), + entry_valid_nsec: DEFAULT_TTL.subsec_nanos(), + attr_valid_nsec: DEFAULT_TTL.subsec_nanos(), + attr: metadata.metadata, + ..Default::default() + }; + + Filesystem::reply_ok(Some(out), None, in_header.unique, w) + } + fn getattr(&self, in_header: InHeader, _r: Reader, w: Writer) -> Result { debug!("getattr: inode={}", in_header.nodeid); @@ -239,11 +288,10 @@ impl Filesystem { None => return Filesystem::reply_error(in_header.unique, w), }; - let mut metadata = match self.rt.block_on(self.do_get_metadata(&path)) { + let metadata = match self.rt.block_on(self.do_get_metadata(&path)) { Ok(metadata) => metadata, Err(_) => return Filesystem::reply_error(in_header.unique, w), }; - metadata.metadata.ino = in_header.nodeid; let out = AttrOut { attr_valid: DEFAULT_TTL.as_secs(), @@ -265,7 +313,19 @@ impl Filesystem { impl Filesystem { async fn do_get_metadata(&self, path: &str) -> Result { let metadata = self.core.stat(path).await.map_err(opendal_error2error)?; - let attr = opendal_metadata2opened_file(path, &metadata, self.uid, self.gid); + let mut attr = opendal_metadata2opened_file(path, &metadata, self.uid, self.gid); + + let mut opened_files_map = self.opened_files_map.lock().unwrap(); + if let Some(inode) = opened_files_map.get(path) { + attr.metadata.ino = *inode; + } else { + let inode = self + .opened_files + .insert(attr.clone()) + .expect("failed to allocate inode"); + attr.metadata.ino = inode as u64; + opened_files_map.insert(path.to_string(), inode as u64); + } Ok(attr) } diff --git a/integrations/virtiofs/src/filesystem_message.rs b/integrations/virtiofs/src/filesystem_message.rs index dd924507e45..c26d33fbc33 100644 --- a/integrations/virtiofs/src/filesystem_message.rs +++ b/integrations/virtiofs/src/filesystem_message.rs @@ -23,6 +23,7 @@ use crate::error::*; /// The corresponding value needs to be aligned with the specification. #[non_exhaustive] pub enum Opcode { + Lookup = 1, Getattr = 3, Setattr = 4, Init = 26, @@ -34,6 +35,7 @@ impl TryFrom for Opcode { fn try_from(value: u32) -> Result { match value { + 1 => Ok(Opcode::Lookup), 3 => Ok(Opcode::Getattr), 4 => Ok(Opcode::Setattr), 26 => Ok(Opcode::Init), @@ -137,6 +139,23 @@ pub struct InitOut { pub unused: [u32; 7], } +/// EntryOut is used to return the file entry in the filesystem call. +/// +/// The fields of the struct need to conform to the specific format of the virtiofs message. +/// Currently, we only need to align them exactly with virtiofsd. +/// Reference: https://gitlab.com/virtio-fs/virtiofsd/-/blob/main/src/fuse.rs?ref_type=heads#L737 +#[repr(C)] +#[derive(Debug, Default, Clone, Copy)] +pub struct EntryOut { + pub nodeid: u64, + pub generation: u64, + pub entry_valid: u64, + pub attr_valid: u64, + pub entry_valid_nsec: u32, + pub attr_valid_nsec: u32, + pub attr: Attr, +} + /// AttrOut is used to return the file attributes in the filesystem call. /// /// The fields of the struct need to conform to the specific format of the virtiofs message. @@ -158,4 +177,5 @@ unsafe impl ByteValued for InHeader {} unsafe impl ByteValued for OutHeader {} unsafe impl ByteValued for InitIn {} unsafe impl ByteValued for InitOut {} +unsafe impl ByteValued for EntryOut {} unsafe impl ByteValued for AttrOut {} diff --git a/integrations/virtiofs/src/virtiofs_util.rs b/integrations/virtiofs/src/virtiofs_util.rs index bcbdf6ea918..ea531c69078 100644 --- a/integrations/virtiofs/src/virtiofs_util.rs +++ b/integrations/virtiofs/src/virtiofs_util.rs @@ -45,6 +45,11 @@ struct DescriptorChainConsumer<'a, B> { } impl<'a, B: BitmapSlice> DescriptorChainConsumer<'a, B> { + #[cfg(test)] + fn available_bytes(&self) -> usize { + self.buffers.iter().fold(0, |count, vs| count + vs.len()) + } + fn bytes_consumed(&self) -> usize { self.bytes_consumed } @@ -146,6 +151,16 @@ impl<'a, B: Bitmap + BitmapSlice + 'static> Reader<'a, B> { self.read_exact(buf)?; Ok(unsafe { obj.assume_init() }) } + + #[cfg(test)] + pub fn available_bytes(&self) -> usize { + self.buffer.available_bytes() + } + + #[cfg(test)] + pub fn bytes_read(&self) -> usize { + self.buffer.bytes_consumed() + } } impl<'a, B: BitmapSlice> io::Read for Reader<'a, B> { @@ -216,6 +231,11 @@ impl<'a, B: Bitmap + BitmapSlice + 'static> Writer<'a, B> { }) } + #[cfg(test)] + pub fn available_bytes(&self) -> usize { + self.buffer.available_bytes() + } + pub fn bytes_written(&self) -> usize { self.buffer.bytes_consumed() } @@ -245,3 +265,161 @@ impl<'a, B: BitmapSlice> Write for Writer<'a, B> { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + use virtio_queue::Queue; + use virtio_queue::QueueOwnedT; + use virtio_queue::QueueT; + use vm_memory::Bytes; + use vm_memory::GuestAddress; + use vm_memory::Le16; + use vm_memory::Le32; + use vm_memory::Le64; + + const VIRTQ_DESC_F_NEXT: u16 = 0x1; + const VIRTQ_DESC_F_WRITE: u16 = 0x2; + + enum DescriptorType { + Readable, + Writable, + } + + // Helper structure for testing, used to define the layout of the descriptor chain. + #[derive(Copy, Clone, Debug, Default)] + #[repr(C)] + struct VirtqDesc { + addr: Le64, + len: Le32, + flags: Le16, + next: Le16, + } + + // Helper structure for testing, used to define the layout of the available ring. + #[derive(Copy, Clone, Debug, Default)] + #[repr(C)] + struct VirtqAvail { + flags: Le16, + idx: Le16, + ring: Le16, + } + + unsafe impl ByteValued for VirtqAvail {} + unsafe impl ByteValued for VirtqDesc {} + + // Helper function for testing, used to create a descriptor chain with the specified descriptors. + fn create_descriptor_chain( + memory: &GuestMemoryMmap, + descriptor_array_addr: GuestAddress, + mut buffers_start_addr: GuestAddress, + descriptors: Vec<(DescriptorType, u32)>, + ) -> DescriptorChain<&GuestMemoryMmap> { + let descriptors_len = descriptors.len(); + for (index, (type_, size)) in descriptors.into_iter().enumerate() { + let mut flags = 0; + if let DescriptorType::Writable = type_ { + flags |= VIRTQ_DESC_F_WRITE; + } + if index + 1 < descriptors_len { + flags |= VIRTQ_DESC_F_NEXT; + } + + let desc = VirtqDesc { + addr: buffers_start_addr.raw_value().into(), + len: size.into(), + flags: flags.into(), + next: (index as u16 + 1).into(), + }; + + buffers_start_addr = buffers_start_addr.checked_add(size as u64).unwrap(); + + memory + .write_obj( + desc, + descriptor_array_addr + .checked_add((index * std::mem::size_of::()) as u64) + .unwrap(), + ) + .unwrap(); + } + + let avail_ring = descriptor_array_addr + .checked_add((descriptors_len * std::mem::size_of::()) as u64) + .unwrap(); + let avail = VirtqAvail { + flags: 0.into(), + idx: 1.into(), + ring: 0.into(), + }; + memory.write_obj(avail, avail_ring).unwrap(); + + let mut queue = Queue::new(4).unwrap(); + queue + .try_set_desc_table_address(descriptor_array_addr) + .unwrap(); + queue.try_set_avail_ring_address(avail_ring).unwrap(); + queue.set_ready(true); + queue.iter(memory).unwrap().next().unwrap() + } + + #[test] + fn simple_chain_reader_test() { + let memory_start_addr = GuestAddress(0x0); + let memory = GuestMemoryMmap::from_ranges(&[(memory_start_addr, 0x1000)]).unwrap(); + + let chain = create_descriptor_chain( + &memory, + GuestAddress(0x0), + GuestAddress(0x100), + vec![ + (DescriptorType::Readable, 8), + (DescriptorType::Readable, 16), + (DescriptorType::Readable, 18), + (DescriptorType::Readable, 64), + ], + ); + + let mut reader = Reader::new(&memory, chain).unwrap(); + assert_eq!(reader.available_bytes(), 106); + assert_eq!(reader.bytes_read(), 0); + + let mut buffer = [0; 64]; + reader.read_exact(&mut buffer).unwrap(); + assert_eq!(reader.available_bytes(), 42); + assert_eq!(reader.bytes_read(), 64); + assert_eq!(reader.read(&mut buffer).unwrap(), 42); + assert_eq!(reader.available_bytes(), 0); + assert_eq!(reader.bytes_read(), 106); + } + + #[test] + fn simple_chain_writer_test() { + let memory_start_addr = GuestAddress(0x0); + let memory = GuestMemoryMmap::from_ranges(&[(memory_start_addr, 0x1000)]).unwrap(); + + let chain = create_descriptor_chain( + &memory, + GuestAddress(0x0), + GuestAddress(0x100), + vec![ + (DescriptorType::Writable, 8), + (DescriptorType::Writable, 16), + (DescriptorType::Writable, 18), + (DescriptorType::Writable, 64), + ], + ); + + let mut writer = Writer::new(&memory, chain).unwrap(); + assert_eq!(writer.available_bytes(), 106); + assert_eq!(writer.bytes_written(), 0); + + let buffer = [0; 64]; + writer.write_all(&buffer).unwrap(); + assert_eq!(writer.available_bytes(), 42); + assert_eq!(writer.bytes_written(), 64); + assert_eq!(writer.write(&buffer).unwrap(), 42); + assert_eq!(writer.available_bytes(), 0); + assert_eq!(writer.bytes_written(), 106); + } +}