diff --git a/Cargo.lock b/Cargo.lock index caa9a5683..6cdc63a0f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3905,7 +3905,7 @@ dependencies = [ [[package]] name = "p9ds" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/p9fs#4b8362b894e8ecbb6da34e2e71d7a248918ecd8f" +source = "git+https://github.com/oxidecomputer/p9fs#113f170aff6aa1d5add00c19b3a2f3241e16a763" dependencies = [ "ispf", "num_enum 0.5.11", diff --git a/lib/propolis/src/hw/virtio/p9fs.rs b/lib/propolis/src/hw/virtio/p9fs.rs index 8cfa8cf65..4f6ab0366 100644 --- a/lib/propolis/src/hw/virtio/p9fs.rs +++ b/lib/propolis/src/hw/virtio/p9fs.rs @@ -32,7 +32,6 @@ use p9ds::proto::{ self, Dirent, MessageType, P9Version, Qid, QidType, Rattach, Rclunk, Rgetattr, Rlerror, Rlopen, Rread, Rreaddir, Rstatfs, Rwalk, Tattach, Tgetattr, Tlopen, Tread, Treaddir, Tstatfs, Twalk, Version, - P9_GETATTR_BASIC, }; use slog::{warn, Logger}; @@ -44,6 +43,19 @@ use slog::{warn, Logger}; /// RREAD header stated size. const P9FS_VIRTIO_READ_HEADROOM: usize = 20; +// Form the rread header. Unfortunately we can't do this with the Rread +// structure because the count is baked into the data field which is tied +// to the length of the vector and filling that vector is what we're +// explicitly trying to avoid here. +#[repr(C, packed)] +#[derive(Copy, Clone)] +struct RreadHeader { + size: u32, + typ: u8, + tag: u16, + count: u32, +} + #[usdt::provider(provider = "propolis")] mod probes { fn p9fs_cfg_read() {} @@ -335,11 +347,11 @@ impl HostFSHandler { let file = match fid.file { Some(ref f) => f, None => { - // the file is not open warn!(self.log, "read: file not open: {:?}", &fid.pathbuf,); return write_error(EINVAL as u32, chain, mem); } }; + let metadata = match file.metadata() { Ok(m) => m, Err(e) => { @@ -366,19 +378,22 @@ impl HostFSHandler { return write_buf(buf, chain, mem); } - let read_count = u32::min(msize, msg.count); + let space_left = u64::from(msize) + - (size_of::() + P9FS_VIRTIO_READ_HEADROOM) as u64; - let space_left = read_count as usize - - size_of::() // Rread.size - - size_of::() // Rread.typ - - size_of::() // Rread.tag - - size_of::() // Rread.data.len - - P9FS_VIRTIO_READ_HEADROOM; + let msglen = + std::cmp::min(u64::from(msg.count), metadata.len() - msg.offset); - let buflen = - std::cmp::min(space_left, (metadata.len() - msg.offset) as usize); + let buflen = std::cmp::min(space_left, msglen); - p9_write_file(&file, chain, mem, buflen, msg.offset as i64); + p9_read_file( + &file, + chain, + msg.tag, + mem, + buflen as usize, + msg.offset as i64, + ); } fn do_statfs(&self, fid: &mut Fid, chain: &mut Chain, mem: &MemCtx) { @@ -428,7 +443,14 @@ impl HostFSHandler { write_buf(buf, chain, mem); } - fn do_getattr(&self, fid: &mut Fid, chain: &mut Chain, mem: &MemCtx) { + fn do_getattr( + &self, + fid: &mut Fid, + tag: u16, + valid: u64, + chain: &mut Chain, + mem: &MemCtx, + ) { let metadata = match fs::metadata(&fid.pathbuf) { Ok(m) => m, Err(e) => { @@ -437,8 +459,6 @@ impl HostFSHandler { } }; - // valid: u64, - let valid = P9_GETATTR_BASIC; // qid: Qid, let qid = Qid { typ: { @@ -456,15 +476,20 @@ impl HostFSHandler { // mode: u32, let mode = metadata.mode(); // uid: u32, - //let uid = metadata.uid(); let uid = 0; //squash for now // gid: u32, - //let gid = metadata.gid(); let gid = 0; //squash for now // nlink: u64, let nlink = metadata.nlink(); // rdev: u64, - let rdev = metadata.rdev(); + let rdev = + if metadata.is_file() || metadata.is_dir() || metadata.is_symlink() + { + 0 // Regular files, directories, and symlinks should have rdev = 0 + } else { + metadata.rdev() // Only device files should have non-zero rdev + }; + // attrsize: u64, let attrsize = metadata.size(); // blksize: u64, @@ -492,7 +517,7 @@ impl HostFSHandler { // data_version: u64, let data_version = 0; // reserved for future use in spec - let resp = Rgetattr::new( + let mut resp = Rgetattr::new( valid, qid, mode, @@ -514,6 +539,7 @@ impl HostFSHandler { gen, data_version, ); + resp.tag = tag; let mut out = ispf::to_bytes_le(&resp).unwrap(); let buf = out.as_mut_slice(); @@ -915,7 +941,9 @@ impl P9Handler for HostFSHandler { let msg: Tgetattr = ispf::from_bytes_le(&msg_buf).unwrap(); match self.fileserver.lock() { Ok(ref mut fs) => match fs.fids.get_mut(&msg.fid) { - Some(ref mut fid) => self.do_getattr(fid, chain, mem), + Some(ref mut fid) => { + self.do_getattr(fid, msg.tag, msg.request_mask, chain, mem) + } None => { warn!(self.log, "getattr: fid {} not found", msg.fid); return write_error(ENOENT as u32, chain, mem); @@ -951,32 +979,20 @@ pub(crate) fn write_error(ecode: u32, chain: &mut Chain, mem: &MemCtx) { write_buf(buf, chain, mem); } -fn p9_write_file( +fn p9_read_file( file: &File, chain: &mut Chain, + tag: u16, mem: &MemCtx, count: usize, offset: i64, ) { - // Form the rread header. Unfortunately we can't do this with the Rread - // structure because the count is baked into the data field which is tied - // to the length of the vector and filling that vector is what we're - // explicitly trying to avoid here. - #[repr(C, packed)] - #[derive(Copy, Clone)] - struct Header { - size: u32, - typ: u8, - tag: u16, - count: u32, - } - - let size = size_of::
() + count; + let size = size_of::() + count; - let h = Header { + let h = RreadHeader { size: size as u32, typ: MessageType::Rread as u8, - tag: 0, + tag, count: count as u32, };