ptfs: optimize implementation of do_lookup()

Optimize implementation of do_lookup() by:
- use statx() instead of fsstat64() when possible to get stat with mount id
- create openable handle on demand, avoid unnecessary operations
- remove InodeStat, using StatExt instead
- remove function stat() and open_proc_file()

Signed-off-by: Jiang Liu <[email protected]>

Author: jiangliu

src/passthrough/file_handle.rs

@@ -13,8 +13,8 @@ use std::sync::Arc;
 
 use vmm_sys_util::fam::{FamStruct, FamStructWrapper};
 
-use super::mount_fd::{MountFd, MountFds, MountId};
-use super::util::enosys;
+use super::mount_fd::{MPRResult, MountFd, MountFds, MountId};
+use crate::api::EMPTY_CSTR;
 
 /// An arbitrary maximum size for CFileHandle::f_handle.
 ///
@@ -243,6 +243,33 @@ impl FileHandle {
             handle: c_fh,
         }))
     }
+
+    /// Create a file handle from a `fd`.
+    /// This is a wrapper around `from_name_at()` and so has the same interface.
+    pub fn from_fd(fd: &impl AsRawFd) -> io::Result<Option<Self>> {
+        // Safe because this is a constant value and a valid C string.
+        let empty_path = unsafe { CStr::from_bytes_with_nul_unchecked(EMPTY_CSTR) };
+        Self::from_name_at(fd, empty_path)
+    }
+
+    /// Return an openable copy of the file handle by ensuring that `mount_fd` contains a valid fd
+    /// for the mount the file handle is for.
+    ///
+    /// `reopen_fd` will be invoked to duplicate an `O_PATH` fd with custom `libc::open()` flags.
+    pub fn into_openable<F>(
+        self,
+        mount_fds: &MountFds,
+        reopen_fd: F,
+    ) -> MPRResult<OpenableFileHandle>
+    where
+        F: FnOnce(RawFd, libc::c_int, u32) -> io::Result<File>,
+    {
+        let mount_fd = mount_fds.get(self.mnt_id, reopen_fd)?;
+        Ok(OpenableFileHandle {
+            handle: Arc::new(self),
+            mount_fd,
+        })
+    }
 }
 
 pub struct OpenableFileHandle {
@@ -264,39 +291,6 @@ impl Debug for OpenableFileHandle {
 }
 
 impl OpenableFileHandle {
-    /// Create a file handle for the given file.
-    ///
-    /// Also ensure that `mount_fds` contains a valid fd for the mount the file is on (so that
-    /// `handle.open_with_mount_fds()` will work).
-    ///
-    /// If `path` is empty, `reopen_dir` may be invoked to duplicate `dir` with custom
-    /// `libc::open()` flags.
-    pub fn from_name_at<F>(
-        dir_fd: &impl AsRawFd,
-        path: &CStr,
-        mount_fds: &MountFds,
-        reopen_dir: F,
-    ) -> io::Result<OpenableFileHandle>
-    where
-        F: FnOnce(RawFd, libc::c_int, u32) -> io::Result<File>,
-    {
-        let handle = FileHandle::from_name_at(dir_fd, path)
-            .map_err(|e| {
-                error!("from_name_at failed error {:?}", e);
-                e
-            })?
-            .ok_or_else(enosys)?;
-
-        let mount_fd = mount_fds
-            .get(handle.mnt_id, reopen_dir)
-            .map_err(|e| e.into_inner())?;
-
-        Ok(OpenableFileHandle {
-            handle: Arc::new(handle),
-            mount_fd,
-        })
-    }
-
     /// Open a file from an openable file handle.
     pub fn open(&self, flags: libc::c_int) -> io::Result<File> {
         let ret = unsafe {
@@ -317,10 +311,6 @@ impl OpenableFileHandle {
         }
     }
 
-    pub fn mount_id(&self) -> MountId {
-        self.handle.mnt_id
-    }
-
     pub fn file_handle(&self) -> &Arc<FileHandle> {
         &self.handle
     }
@@ -443,29 +433,4 @@ mod tests {
             0
         );
     }
-
-    #[test]
-    fn test_openable_file_handle() {
-        let uid = getuid();
-        if !uid.is_root() {
-            return;
-        }
-
-        let topdir = env!("CARGO_MANIFEST_DIR");
-        let dir = File::open(topdir).unwrap();
-        let filename = CString::new("build.rs").unwrap();
-        let mount_fds = MountFds::new(None).unwrap();
-
-        let file_handle =
-            OpenableFileHandle::from_name_at(&dir, &filename, &mount_fds, |_fd, _flags, _mode| {
-                File::open(topdir)
-            })
-            .unwrap();
-        assert_eq!(file_handle.handle.mnt_id, file_handle.mount_id());
-
-        let mut file = file_handle.open(libc::O_RDONLY).unwrap();
-        let mut buffer = [0u8; 1024];
-        let res = file.read(&mut buffer).unwrap();
-        assert!(res > 0);
-    }
 }

src/passthrough/inode_store.rs

@@ -5,7 +5,8 @@ use std::collections::BTreeMap;
 use std::sync::Arc;
 
 use super::file_handle::FileHandle;
-use super::{Inode, InodeData, InodeHandle, InodeStat};
+use super::statx::StatExt;
+use super::{Inode, InodeData, InodeHandle};
 
 #[derive(Clone, Copy, Default, PartialOrd, Ord, PartialEq, Eq, Debug)]
 /// Identify an inode in `PassthroughFs` by `InodeId`.
@@ -17,12 +18,11 @@ pub struct InodeId {
 
 impl InodeId {
     #[inline]
-    pub(super) fn from_stat(ist: &InodeStat) -> Self {
-        let st = ist.get_stat();
+    pub(super) fn from_stat(st: &StatExt) -> Self {
         InodeId {
-            ino: st.st_ino,
-            dev: st.st_dev,
-            mnt: ist.get_mnt_id(),
+            ino: st.st.st_ino,
+            dev: st.st.st_dev,
+            mnt: st.mnt_id,
         }
     }
 }
@@ -103,7 +103,7 @@ mod test {
 
     use std::ffi::CStr;
     use std::mem::MaybeUninit;
-    use std::os::unix::io::{AsRawFd, RawFd};
+    use std::os::unix::io::AsRawFd;
     use std::sync::atomic::Ordering;
     use vmm_sys_util::tempfile::TempFile;
 
@@ -127,14 +127,14 @@ mod test {
         }
     }
 
-    fn stat_fd(fd: RawFd) -> io::Result<libc::stat64> {
+    fn stat_fd(fd: &impl AsRawFd) -> io::Result<libc::stat64> {
         let mut st = MaybeUninit::<libc::stat64>::zeroed();
         let null_path = unsafe { CStr::from_bytes_with_nul_unchecked(b"\0") };
 
         // Safe because the kernel will only write data in `st` and we check the return value.
         let res = unsafe {
             libc::fstatat64(
-                fd,
+                fd.as_raw_fd(),
                 null_path.as_ptr(),
                 st.as_mut_ptr(),
                 libc::AT_EMPTY_PATH | libc::AT_SYMLINK_NOFOLLOW,
@@ -156,32 +156,20 @@ mod test {
 
         let inode1: Inode = 3;
         let inode2: Inode = 4;
-        let inode_stat1 = InodeStat {
-            stat: stat_fd(tmpfile1.as_file().as_raw_fd()).unwrap(),
+        let inode_stat1 = StatExt {
+            st: stat_fd(tmpfile1.as_file()).unwrap(),
             mnt_id: 0,
         };
-        let inode_stat2 = InodeStat {
-            stat: stat_fd(tmpfile2.as_file().as_raw_fd()).unwrap(),
+        let inode_stat2 = StatExt {
+            st: stat_fd(tmpfile2.as_file()).unwrap(),
             mnt_id: 0,
         };
         let id1 = InodeId::from_stat(&inode_stat1);
         let id2 = InodeId::from_stat(&inode_stat2);
         let file_or_handle1 = InodeHandle::File(tmpfile1.into_file());
         let file_or_handle2 = InodeHandle::File(tmpfile2.into_file());
-        let data1 = InodeData::new(
-            inode1,
-            file_or_handle1,
-            2,
-            id1,
-            inode_stat1.get_stat().st_mode,
-        );
-        let data2 = InodeData::new(
-            inode2,
-            file_or_handle2,
-            2,
-            id2,
-            inode_stat2.get_stat().st_mode,
-        );
+        let data1 = InodeData::new(inode1, file_or_handle1, 2, id1, inode_stat1.st.st_mode);
+        let data2 = InodeData::new(inode2, file_or_handle2, 2, id2, inode_stat2.st.st_mode);
         let data1 = Arc::new(data1);
         let data2 = Arc::new(data2);