ptfs: switch to new mount-fd implementation

Switch to new mount-fd implementation.

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

Author: jiangliu

src/passthrough/async_io.rs

@@ -30,9 +30,9 @@ impl<S: BitmapSlice + Send + Sync + 'static> BackendFileSystem for PassthroughFs
 }
 
 impl<'a> InodeData {
-    async fn async_get_file(&self, mount_fds: &MountFds) -> io::Result<InodeFile<'_>> {
+    async fn async_get_file(&self) -> io::Result<InodeFile<'_>> {
         // The io_uring doesn't support open_by_handle_at yet, so use sync io.
-        self.get_file(mount_fds)
+        self.get_file()
     }
 }
 

src/passthrough/file_handle.rs

@@ -3,20 +3,16 @@
 // found in the LICENSE-BSD-3-Clause file.
 
 use std::cmp::Ordering;
-use std::collections::HashMap;
 use std::ffi::CStr;
 use std::fmt::{Debug, Formatter};
 use std::fs::File;
 use std::io;
 use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
-use std::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard};
+use std::sync::Arc;
 
-use vmm_sys_util::{
-    fam::{FamStruct, FamStructWrapper},
-    generate_fam_struct_impl,
-};
+use vmm_sys_util::fam::{FamStruct, FamStructWrapper};
 
-use crate::passthrough::PassthroughFs;
+use super::mount_fd::{MountFd, MountFds, MountId};
 
 /// An arbitrary maximum size for CFileHandle::f_handle.
 ///
@@ -64,7 +60,7 @@ pub struct CFileHandleInner {
     pub f_handle: __IncompleteArrayField<libc::c_char>,
 }
 
-generate_fam_struct_impl!(
+vmm_sys_util::generate_fam_struct_impl!(
     CFileHandleInner,
     libc::c_char,
     f_handle,
@@ -230,46 +226,70 @@ impl FileHandle {
                 handle: c_fh,
             })
         } else {
-            let e = io::Error::last_os_error();
-            Err(e)
+            Err(io::Error::last_os_error())
         }
     }
+}
+
+pub struct OpenableFileHandle {
+    handle: Arc<FileHandle>,
+    mount_fd: Arc<MountFd>,
+}
+
+impl Debug for OpenableFileHandle {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let fh = self.handle.handle.wrapper.as_fam_struct_ref();
+        write!(
+            f,
+            "Openable file handle: mountfd {}, type {}, len {}",
+            self.mount_fd.file().as_raw_fd(),
+            fh.handle_type,
+            fh.handle_bytes
+        )
+    }
+}
 
+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_with_mount_fds<F>(
+    pub fn from_name_at<F>(
         dir_fd: RawFd,
         path: &CStr,
         mount_fds: &MountFds,
         reopen_dir: F,
-    ) -> io::Result<Self>
+    ) -> io::Result<OpenableFileHandle>
     where
         F: FnOnce(RawFd, libc::c_int, u32) -> io::Result<File>,
     {
-        let handle = Self::from_name_at(dir_fd, path).map_err(|e| {
+        let handle = FileHandle::from_name_at(dir_fd, path).map_err(|e| {
             error!("from_name_at failed error {:?}", e);
             e
         })?;
 
-        mount_fds.ensure_mount_point(handle.mnt_id, dir_fd, path, reopen_dir)?;
+        let mount_fd = mount_fds
+            .get(handle.mnt_id, reopen_dir)
+            .map_err(|e| e.into_inner())?;
 
-        Ok(handle)
+        Ok(OpenableFileHandle {
+            handle: Arc::new(handle),
+            mount_fd,
+        })
     }
 
     /// Open a file handle (low-level wrapper).
     ///
     /// `mount_fd` must be an open non-`O_PATH` file descriptor for an inode on the same mount as
     /// the file to be opened, i.e. the mount given by `self.mnt_id`.
-    fn open(&self, mount_fd: &impl AsRawFd, flags: libc::c_int) -> io::Result<File> {
+    pub fn open(&self, flags: libc::c_int) -> io::Result<File> {
         let ret = unsafe {
             open_by_handle_at(
-                mount_fd.as_raw_fd(),
-                self.handle.wrapper.as_fam_struct_ptr(),
+                self.mount_fd.file().as_raw_fd(),
+                self.handle.handle.wrapper.as_fam_struct_ptr(),
                 flags,
             )
         };
@@ -284,115 +304,12 @@ impl FileHandle {
         }
     }
 
-    /// Open a file handle, using the given `mount_fds` hash map.
-    ///
-    /// Look up `self.mnt_id` in `mount_fds`, and pass the result to `self.open()`.
-    pub fn open_with_mount_fds(
-        &self,
-        mount_fds: &MountFds,
-        flags: libc::c_int,
-    ) -> io::Result<File> {
-        let mount_fds_locked = mount_fds.get_map();
-        let mount_file = mount_fds_locked.get(&self.mnt_id).ok_or_else(|| {
-            error!(
-                "open_with_mount_fds: mnt_id {:?} is not found.",
-                &self.mnt_id
-            );
-            io::Error::from_raw_os_error(libc::ENODEV)
-        })?;
-
-        self.open(mount_file, flags)
-    }
-}
-
-/// Struct to maintain <mount ID, mountpoint file> mapping for open_by_handle_at().
-///
-/// Creating a file handle only returns a mount ID; opening a file handle requires an open fd on the
-/// respective mount.  This is a type in which we can store fds that we know are associated with a
-/// given mount ID, so that when opening a handle we can look it up.
-#[derive(Default)]
-pub struct MountFds {
-    pub(crate) map: RwLock<HashMap<u64, File>>,
-}
-
-impl MountFds {
-    pub fn new() -> Self {
-        MountFds::default()
-    }
-
-    pub fn get_map(&self) -> RwLockReadGuard<'_, HashMap<u64, std::fs::File>> {
-        self.map.read().unwrap()
-    }
-
-    pub fn get_map_mut(&self) -> RwLockWriteGuard<'_, HashMap<u64, std::fs::File>> {
-        self.map.write().unwrap()
+    pub fn mount_id(&self) -> MountId {
+        self.handle.mnt_id
     }
 
-    fn ensure_mount_point<F>(
-        &self,
-        mnt_id: u64,
-        dir_fd: RawFd,
-        path: &CStr,
-        reopen_dir: F,
-    ) -> io::Result<()>
-    where
-        F: FnOnce(RawFd, libc::c_int, u32) -> io::Result<File>,
-    {
-        if self.get_map().contains_key(&mnt_id) {
-            return Ok(());
-        }
-
-        let (path_fd, _path_file) = if path.to_bytes().is_empty() {
-            // `open_by_handle_at()` needs a non-`O_PATH` fd, and `dir` may be `O_PATH`, so we
-            // have to open a new fd here
-            // We do not know whether `dir`/`path` is a special file, though, and we must not open
-            // special files with anything but `O_PATH`, so we have to get some `O_PATH` fd first
-            // that we can stat to find out whether it is safe to open.
-            // (When opening a new fd here, keep a `File` object around so the fd is closed when it
-            // goes out of scope.)
-            (dir_fd, None)
-        } else {
-            let f = PassthroughFs::<()>::open_file(
-                dir_fd,
-                path,
-                libc::O_PATH | libc::O_NOFOLLOW | libc::O_CLOEXEC,
-                0,
-            )
-            .map_err(|e| {
-                error!(
-                    "from_name_at_with_mount_fds: open_file on {:?} failed error {:?}",
-                    path, e
-                );
-                e
-            })?;
-            (f.as_raw_fd(), Some(f))
-        };
-
-        // liubo: TODO find mnt id
-        // Ensure that `file` refers to an inode with the mount ID we need
-        // if statx(&file, None)?.mnt_id != handle.mnt_id {
-        //     return Err(io::Error::from_raw_os_error(libc::EIO));
-        // }
-
-        let st = PassthroughFs::<()>::stat_fd(path_fd, None)?;
-        // Ensure that we can safely reopen `path_fd` with `O_RDONLY`
-        let file_type = st.st_mode & libc::S_IFMT;
-        if file_type != libc::S_IFREG && file_type != libc::S_IFDIR {
-            error!(
-                "from_name_at_with_mount_fds: file {:?} is special file",
-                path
-            );
-            return Err(io::Error::from_raw_os_error(libc::EIO));
-        }
-
-        let file = reopen_dir(
-            path_fd,
-            libc::O_RDONLY | libc::O_NOFOLLOW | libc::O_CLOEXEC,
-            st.st_mode,
-        )?;
-        self.get_map_mut().insert(mnt_id, file);
-
-        Ok(())
+    pub fn file_handle(&self) -> &Arc<FileHandle> {
+        &self.handle
     }
 }
 

src/passthrough/inode_store.rs

@@ -42,7 +42,8 @@ impl InodeStore {
     pub fn insert(&mut self, data: Arc<InodeData>) {
         self.by_id.insert(data.id, data.inode);
         if let FileOrHandle::Handle(handle) = &data.file_or_handle {
-            self.by_handle.insert(handle.clone(), data.inode);
+            self.by_handle
+                .insert(handle.file_handle().clone(), data.inode);
         }
         self.data.insert(data.inode, data);
     }
@@ -59,7 +60,7 @@ impl InodeStore {
 
         if let Some(data) = data.as_ref() {
             if let FileOrHandle::Handle(handle) = &data.file_or_handle {
-                self.by_handle.remove(handle);
+                self.by_handle.remove(handle.file_handle());
             }
             self.by_id.remove(&data.id);
         }
@@ -119,7 +120,9 @@ mod test {
                 (FileOrHandle::File(f1), FileOrHandle::File(f2)) => {
                     f1.as_raw_fd() == f2.as_raw_fd()
                 }
-                (FileOrHandle::Handle(h1), FileOrHandle::Handle(h2)) => h1 == h2,
+                (FileOrHandle::Handle(h1), FileOrHandle::Handle(h2)) => {
+                    h1.file_handle() == h2.file_handle()
+                }
                 _ => false,
             }
         }

src/passthrough/mod.rs

@@ -29,8 +29,9 @@ use std::time::Duration;
 
 use vm_memory::{bitmap::BitmapSlice, ByteValued};
 
-use self::file_handle::{FileHandle, MountFds};
+use self::file_handle::{FileHandle, OpenableFileHandle};
 use self::inode_store::{InodeId, InodeStore};
+use self::mount_fd::MountFds;
 use crate::abi::fuse_abi as fuse;
 use crate::abi::fuse_abi::Opcode;
 use crate::api::filesystem::Entry;
@@ -43,6 +44,7 @@ use crate::api::{
 mod async_io;
 mod file_handle;
 mod inode_store;
+mod mount_fd;
 mod sync_io;
 
 type Inode = u64;
@@ -137,14 +139,14 @@ impl InodeStat {
 #[derive(Debug)]
 enum FileOrHandle {
     File(File),
-    Handle(Arc<FileHandle>),
+    Handle(Arc<OpenableFileHandle>),
 }
 
 impl FileOrHandle {
     fn handle(&self) -> Option<&FileHandle> {
         match self {
             FileOrHandle::File(_) => None,
-            FileOrHandle::Handle(h) => Some(h.deref()),
+            FileOrHandle::Handle(h) => Some(h.file_handle().deref()),
         }
     }
 }
@@ -207,11 +209,11 @@ impl InodeData {
         }
     }
 
-    fn get_file(&self, mount_fds: &MountFds) -> io::Result<InodeFile<'_>> {
+    fn get_file(&self) -> io::Result<InodeFile<'_>> {
         match &self.file_or_handle {
             FileOrHandle::File(f) => Ok(InodeFile::Ref(f)),
             FileOrHandle::Handle(h) => {
-                let f = h.open_with_mount_fds(mount_fds, libc::O_PATH)?;
+                let f = h.open(libc::O_PATH)?;
                 Ok(InodeFile::Owned(f))
             }
         }
@@ -679,15 +681,18 @@ impl<S: BitmapSlice + Send + Sync> PassthroughFs<S> {
                 (None, Some(a)) => (cfg.entry_timeout, a),
                 (None, None) => (cfg.entry_timeout, cfg.attr_timeout),
             };
+
+        let mount_fds = MountFds::new(None)?;
+
         Ok(PassthroughFs {
             inode_map: InodeMap::new(),
             next_inode: AtomicU64::new(fuse::ROOT_ID + 1),
             ino_allocator: UniqueInodeGenerator::new(),
 
             handle_map: HandleMap::new(),
             next_handle: AtomicU64::new(1),
-            mount_fds: MountFds::new(),
 
+            mount_fds,
             proc_self_fd,
 
             writeback: AtomicBool::new(false),
@@ -779,7 +784,7 @@ impl<S: BitmapSlice + Send + Sync> PassthroughFs<S> {
     /// This function is used by Nydus blobfs
     pub fn readlinkat_proc_file(&self, inode: Inode) -> io::Result<PathBuf> {
         let data = self.inode_map.get(inode)?;
-        let file = data.get_file(&self.mount_fds)?;
+        let file = data.get_file()?;
         let pathname = CString::new(format!("{}", file.as_raw_fd()))
             .map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e))?;
 
@@ -892,7 +897,7 @@ impl<S: BitmapSlice + Send + Sync> PassthroughFs<S> {
         F: FnOnce(RawFd, libc::c_int, u32) -> io::Result<File>,
     {
         let handle = if use_handle {
-            FileHandle::from_name_at_with_mount_fds(dir_fd, name, mount_fds, reopen_dir)
+            OpenableFileHandle::from_name_at(dir_fd, name, mount_fds, reopen_dir)
         } else {
             Err(io::Error::from_raw_os_error(libc::ENOTSUP))
         };
@@ -936,7 +941,7 @@ impl<S: BitmapSlice + Send + Sync> PassthroughFs<S> {
             }
             FileOrHandle::Handle(h) => InodeStat {
                 stat: Self::stat_fd(dir_fd, Some(name))?,
-                mnt_id: h.mnt_id,
+                mnt_id: h.mount_id(),
             },
         };
 
@@ -981,7 +986,7 @@ impl<S: BitmapSlice + Send + Sync> PassthroughFs<S> {
             };
 
         let dir = self.inode_map.get(parent)?;
-        let dir_file = dir.get_file(&self.mount_fds)?;
+        let dir_file = dir.get_file()?;
         let (file_or_handle, st, id) = Self::open_file_or_handle(
             self.cfg.inode_file_handles,
             self.cfg.enable_mntid,