Parcourir la source

* Improved the debug messages in the FUSE FS.
* Fixed mutliple bugs in `SectoredBuf`.

Matthew Carr il y a 2 ans
Parent
commit
675b0ef1f1
3 fichiers modifiés avec 160 ajouts et 55 suppressions
  1. 1 1
      Cargo.toml
  2. 69 27
      crates/btlib/src/blocktree.rs
  3. 90 27
      crates/btlib/src/sectored_buf.rs

+ 1 - 1
Cargo.toml

@@ -15,4 +15,4 @@ debug = true
 # Uncomment this to build the tests with optimizations. This provides better performance when using
 # the FUSE file system.
 #[profile.test]
-#opt-level = 2
+#opt-level = 3

+ 69 - 27
crates/btlib/src/blocktree.rs

@@ -893,9 +893,9 @@ mod private {
             flags: u32,
             // This is the second field of the `fuse_open_in` struct, which is currently unused
             // by the kernel. (https://man7.org/linux/man-pages/man4/fuse.4.html)
-            _fuse_flags: u32,
+            fuse_flags: u32,
         ) -> io::Result<(Option<Self::Handle>, OpenOptions)> {
-            debug!("Blocktree::open called on inode {inode}");
+            debug!("Blocktree::open, inode {inode}, flags {flags}, fuse_flags {fuse_flags}");
             let flags: i32 = flags.try_into().box_err()?;
             if flags & libc::O_APPEND != 0 {
                 return Self::unsupported_flag("O_APPEND");
@@ -930,7 +930,7 @@ mod private {
             _flock_release: bool,
             _lock_owner: Option<u64>,
         ) -> io::Result<()> {
-            debug!("Blocktree::release called on inode {inode}");
+            debug!("Blocktree::release, inode {inode}, handle {handle}, flush {flush}");
             self.access_value_mut(inode, |value| {
                 if flush {
                     value.access_block_mut(handle, |block| block.flush().bterr())?;
@@ -947,13 +947,14 @@ mod private {
             inode: Self::Inode,
             _flags: u32,
         ) -> io::Result<(Option<Self::Handle>, OpenOptions)> {
-            debug!("Blocktree::opendir called on inode {inode}");
+            debug!("Blocktree::opendir, inode {inode}");
             let handle = self.access_value_mut(inode, |value| {
                 self.authorizer.exec_allowed(ctx, value.block().meta())?;
                 let handle = value.new_handle()?;
                 value.convert_to_dir(handle)?;
                 Ok(handle)
             })?;
+            debug!("Blockree::opendir, returning handle {handle}");
             Ok((Some(handle), OpenOptions::empty()))
         }
 
@@ -964,7 +965,7 @@ mod private {
             _flags: u32,
             handle: Self::Handle,
         ) -> io::Result<()> {
-            debug!("Blocktree::releasedir called for inode {inode}");
+            debug!("Blocktree::releasedir, inode {inode}, handle {handle}");
             self.access_value_mut(inode, |value| {
                 value.forget_handle(handle);
                 Ok(())
@@ -979,12 +980,16 @@ mod private {
             name: &CStr,
             args: CreateIn,
         ) -> std::io::Result<(Entry, Option<Self::Handle>, OpenOptions)> {
-            debug!("Blocktree::create called on parent {parent}");
-
-            let name = name
-                .to_str()
-                .map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?
-                .to_owned();
+            let name = match name.to_str() {
+                Ok(name) => {
+                    debug!("Blocktree::create, parent {parent}, name '{name}'");
+                    name.to_owned()
+                }
+                Err(err) => {
+                    debug!("Blocktree::create, parent {parent}");
+                    return Err(io::Error::new(io::ErrorKind::InvalidInput, err));
+                }
+            };
 
             // Reserve a free inode.
             let inode = self.next_inode()?;
@@ -1040,7 +1045,7 @@ mod private {
             flags: u32,
             _fuse_flags: u32,
         ) -> io::Result<usize> {
-            debug!("Blocktree::write called called on inode {inode}");
+            debug!("Blocktree::write, inode {inode}, handle {handle}, offset {offset}, size {size}, flags {flags}");
             if flags as libc::c_int == libc::O_RDONLY {
                 return Err(io::Error::new(
                     io::ErrorKind::PermissionDenied,
@@ -1065,7 +1070,7 @@ mod private {
             handle: Self::Handle,
             _lock_owner: u64,
         ) -> io::Result<()> {
-            debug!("Blocktree::flush called for inode {inode}");
+            debug!("Blocktree::flush, inode {inode}, handle {handle}");
             self.access_block_mut(inode, handle, |block| block.flush().bterr())
                 .io_err()
         }
@@ -1081,7 +1086,7 @@ mod private {
             _lock_owner: Option<u64>,
             flags: u32,
         ) -> io::Result<usize> {
-            debug!("Blocktree::read, inode {inode}, handle {handle}, offset {offset}, size {size}");
+            debug!("Blocktree::read, inode {inode}, handle {handle}, offset {offset}, size {size}, flags {flags}");
             if (flags as libc::c_int & libc::O_WRONLY) != 0 {
                 return Err(io::Error::new(
                     io::ErrorKind::PermissionDenied,
@@ -1134,7 +1139,9 @@ mod private {
                 fuse_backend_rs::api::filesystem::DirEntry,
             ) -> io::Result<usize>,
         ) -> io::Result<()> {
-            debug!("Blocktree::readdir called on inode {inode}");
+            debug!(
+                "Blocktree::readdir, inode {inode}, handle {handle}, size {size}, offset {offset}"
+            );
             let mut size: usize = size.try_into().box_err()?;
             self.access_value(inode, |value| {
                 let dir = value
@@ -1173,7 +1180,7 @@ mod private {
             inode: Self::Inode,
             handle: Option<Self::Handle>,
         ) -> io::Result<(stat64, Duration)> {
-            debug!("Blocktree::getattr called for inode {inode}");
+            debug!("Blocktree::getattr, inode {inode}, handle {:?}", handle);
             let stat = if let Some(handle) = handle {
                 self.access_block(inode, handle, |block| block.meta_body().secrets()?.stat())?
             } else {
@@ -1183,7 +1190,7 @@ mod private {
         }
 
         fn forget(&self, _ctx: &Context, inode: Self::Inode, count: u64) {
-            debug!("Blocktree::forget called for inode {inode}");
+            debug!("Blocktree::forget, inode {inode}, count {count}");
             let mut inodes = match self.inodes.write() {
                 Ok(inodes) => inodes,
                 Err(err) => {
@@ -1223,15 +1230,23 @@ mod private {
             offset: u64,
             whence: u32,
         ) -> io::Result<u64> {
-            debug!("Blocktree::lseek called for inode {inode}");
+            debug!("Blocktree::lseek, inode {inode}, handle {handle}, offset {offset}, whence {whence}");
             let seek_from = SeekFrom::whence_offset(whence, offset)?;
             self.access_block_mut(inode, handle, |block| block.seek(seek_from).bterr())
                 .io_err()
         }
 
         fn unlink(&self, ctx: &Context, parent: Self::Inode, name: &CStr) -> io::Result<()> {
-            debug!("Blocktree::unlink called on parent {parent}");
-            let name = name.to_str().box_err()?;
+            let name = match name.to_str().box_err() {
+                Ok(name) => {
+                    debug!("Blocktree::unlink, parent {parent}, name '{name}'");
+                    name
+                }
+                Err(err) => {
+                    debug!("Blocktree::unlink, parent {parent}");
+                    return Err(err);
+                }
+            };
             let (block_path, inode) = self.borrow_block_mut(parent, |block| {
                 self.authorizer.write_allowed(ctx, block.meta())?;
 
@@ -1273,8 +1288,18 @@ mod private {
             newparent: Self::Inode,
             newname: &CStr,
         ) -> io::Result<Entry> {
-            debug!("Blocktree::link called for inode {inode}");
-            let newname = newname.to_str().box_err()?;
+            let newname = match newname.to_str().box_err() {
+                Ok(newname) => {
+                    debug!(
+                        "Blocktree::link, inode {inode}, newparent {newparent}, newname {newname}"
+                    );
+                    newname
+                }
+                Err(err) => {
+                    debug!("Blocktree::link, inode {inode}, newparent {newparent}");
+                    return Err(err);
+                }
+            };
             self.borrow_block_mut(newparent, |block| {
                 self.authorizer.write_allowed(ctx, block.meta())?;
 
@@ -1314,7 +1339,7 @@ mod private {
             handle: Option<Self::Handle>,
             valid: SetattrValid,
         ) -> io::Result<(stat64, Duration)> {
-            debug!("Blocktree::setattr called for inode {inode}");
+            debug!("Blocktree::setattr, inode {inode}, handle {:?}", handle);
             let cb = |block: &mut FileBlock<C>| {
                 self.authorizer.write_allowed(ctx, block.meta())?;
 
@@ -1376,8 +1401,17 @@ mod private {
             mode: u32,
             umask: u32,
         ) -> io::Result<Entry> {
-            debug!("Blocktree::mkdir called");
-            let name = name.to_str().box_err()?.to_owned();
+            debug!("Blocktree::mkdir, parent {parent}");
+            let name = match name.to_str().box_err() {
+                Ok(name) => {
+                    debug!("Blocktree::mkdir, parent {parent}, mode {mode}, umask {umask}, name '{name}'");
+                    name.to_owned()
+                }
+                Err(err) => {
+                    debug!("Blocktree::mkdir, parent {parent}, mode {mode}, umask {umask}");
+                    return Err(err);
+                }
+            };
             let (inode, mut block_path) = self.borrow_block_mut(parent, |block| {
                 self.authorizer.write_allowed(ctx, block.meta())?;
 
@@ -1414,8 +1448,16 @@ mod private {
         }
 
         fn rmdir(&self, ctx: &Context, parent: Self::Inode, name: &CStr) -> io::Result<()> {
-            debug!("Blocktree::rmdir called on parent {parent}");
-            let name = name.to_str().box_err()?.to_owned();
+            let name = match name.to_str().box_err() {
+                Ok(name) => {
+                    debug!("Blocktree::rmdir, parent {parent}, name '{name}'");
+                    name.to_owned()
+                }
+                Err(err) => {
+                    debug!("Blocktree::rmdir, parent {parent}");
+                    return Err(err);
+                }
+            };
             self.borrow_block_mut(parent, |block| {
                 self.authorizer.write_allowed(ctx, block.meta())?;
 

+ 90 - 27
crates/btlib/src/sectored_buf.rs

@@ -31,10 +31,10 @@ mod private {
         pub fn new() -> SectoredBuf<()> {
             SectoredBuf {
                 inner: (),
+                pos: 0,
                 buf: Vec::new(),
                 buf_start: 0,
                 dirty: false,
-                pos: 0,
             }
         }
     }
@@ -77,12 +77,9 @@ mod private {
 
         /// Returns the offset into the internal buffer that corresponds to the current position.
         fn buf_pos(&self) -> usize {
-            self.pos - self.buf_start
-        }
-
-        /// Returns the index of the sector which is currently loaded into the buffer.
-        fn sector_index(&self, pos: u64) -> u64 {
-            pos / (self.sector_sz() as u64)
+            let buf_pos = self.pos - self.buf_start;
+            debug_assert!(buf_pos <= self.buf.len());
+            buf_pos
         }
     }
 
@@ -299,11 +296,18 @@ mod private {
         ) -> io::Result<u64> {
             let pos = self.pos as u64;
             let pos_new = seek_from.abs(|| Ok(pos), || self.size_or_err())?;
-            let sect_index = self.sector_index(pos);
-            let sect_index_new = self.sector_index(pos_new);
-            if sect_index != sect_index_new {
+            let len = self.len();
+            if pos_new > len as u64 {
+                return Err(io::Error::new(
+                    io::ErrorKind::InvalidInput,
+                    format!("can't seek to {pos_new}, only {len} bytes total"),
+                ));
+            }
+            let sect_sz = self.sector_sz() as u64;
+            let sect_index = pos / sect_sz;
+            let sect_index_new = pos_new / sect_sz;
+            if sect_index != sect_index_new || sect_sz == pos - self.buf_start as u64 {
                 pre_seek(self)?;
-                let sect_sz: u64 = self.sector_sz().try_into().box_err()?;
                 let seek_to = sect_index_new * sect_sz;
                 self.inner.seek(SeekFrom::Start(seek_to))?;
                 self.fill_internal_buf()?;
@@ -319,6 +323,22 @@ mod private {
         }
     }
 
+    impl<T: Read + Seek + AsRef<BlockMeta>> TrySeek for SectoredBuf<T> {
+        fn try_seek(&mut self, seek_from: SeekFrom) -> io::Result<()> {
+            self.seek_impl(seek_from, |sich| {
+                if sich.dirty {
+                    Err(io::Error::new(
+                        io::ErrorKind::Unsupported,
+                        "SectoredBuf::try_seek failed because it has unwritten data",
+                    ))
+                } else {
+                    Ok(())
+                }
+            })?;
+            Ok(())
+        }
+    }
+
     impl<U, T: AsRef<U>> AsRef<U> for SectoredBuf<T> {
         fn as_ref(&self) -> &U {
             self.inner.as_ref()
@@ -400,22 +420,6 @@ mod private {
             self.pos
         }
     }
-
-    impl<T: Read + Seek + AsRef<BlockMeta>> TrySeek for SectoredBuf<T> {
-        fn try_seek(&mut self, seek_from: SeekFrom) -> io::Result<()> {
-            self.seek_impl(seek_from, |sich| {
-                if sich.dirty {
-                    Err(io::Error::new(
-                        io::ErrorKind::Unsupported,
-                        "SectoredBuf::try_seek failed because it has unwritten data",
-                    ))
-                } else {
-                    Ok(())
-                }
-            })?;
-            Ok(())
-        }
-    }
 }
 
 #[cfg(test)]
@@ -753,4 +757,63 @@ mod tests {
         const EXPECTED: [u8; EXPECTED_LEN] = integer_array::<EXPECTED_LEN>(OFFSET as u8);
         assert_eq!(&EXPECTED, actual.into_inner().as_slice());
     }
+
+    #[test]
+    fn seek_past_end_is_error() {
+        const SECT_SZ: usize = 4;
+        let mut sectored = SectoredBuf::new()
+            .try_compose(SectoredCursor::new(Vec::new(), SECT_SZ))
+            .unwrap();
+
+        let result = sectored.seek(SeekFrom::Start(1));
+
+        let matched = if let Err(err) = result {
+            io::ErrorKind::InvalidInput == err.kind()
+        } else {
+            false
+        };
+        assert!(matched);
+    }
+
+    #[test]
+    fn seek_to_zero_when_empty() {
+        const SECT_SZ: usize = 4;
+        let mut sectored = SectoredBuf::new()
+            .try_compose(SectoredCursor::new(Vec::new(), SECT_SZ))
+            .unwrap();
+
+        let pos = sectored.seek(SeekFrom::Start(0)).unwrap();
+
+        assert_eq!(0, pos);
+    }
+
+    #[test]
+    fn read_into_consumes_remaining_sector() {
+        // SECT_SZ % 2 is assumed be 0.
+        const SECT_SZ: usize = 4;
+        const DATA_LEN: usize = 2 * SECT_SZ;
+        const DATA: [u8; DATA_LEN] = integer_array::<DATA_LEN>(0);
+        let mut sectored = SectoredBuf::new()
+            .try_compose(SectoredCursor::new(Vec::new(), SECT_SZ))
+            .unwrap();
+
+        sectored.write(&DATA).unwrap();
+        const MID_FIRST: usize = SECT_SZ / 2;
+        sectored.seek(SeekFrom::Start(MID_FIRST as u64)).unwrap();
+        // Notice that the `count` argument plus the current `sectored.pos` equals `SECT_SZ`.
+        // This will cause `pos / sect_sz` to increase by one, but without incrementing
+        // `buf_start`. This creates a special case that `seek_impl` has to take into account.
+        sectored
+            .read_into(&mut BtCursor::new([0u8; MID_FIRST]), MID_FIRST)
+            .unwrap();
+        const MID_SECOND: u64 = 3 * SECT_SZ as u64 / 2;
+        sectored.try_seek(SeekFrom::Start(MID_SECOND)).unwrap();
+
+        const EXPECTED_LEN: usize = SECT_SZ / 2;
+        const EXPECTED: [u8; EXPECTED_LEN] = integer_array::<EXPECTED_LEN>(MID_SECOND as u8);
+        let mut actual = BtCursor::new(Vec::new());
+        let nread = sectored.read_into(&mut actual, EXPECTED_LEN).unwrap();
+        assert_eq!(EXPECTED_LEN, nread);
+        assert_eq!(&EXPECTED, actual.into_inner().as_slice());
+    }
 }