Rust PuzleFS filesystem driver

[...]
+++ b/rust/kernel/mount.rs

rust/kernel/mount.rs new file +3 -0

@@ -0,0 +1,71 @@
[...]
+impl Vfsmount {
+    /// Create a new private mount clone based on a path name
+    pub fn new_private_mount(path_name: &CStr) -> Result<Self> {

rust/kernel/mount.rs new file +5 -0

+        if err != 0 {
+            pr_err!("failed to resolve '{}': {}\n", path_name, err);
+            return Err(EINVAL);
+        }
+

rust/kernel/mount.rs new file +15 -1

+        Ok(Self { vfsmount })
+    }
+
+    /// Returns a raw pointer to vfsmount
+    pub fn get(&self) -> *mut bindings::vfsmount {
+        self.vfsmount
+    }
+}
+
+impl Drop for Vfsmount {
+    fn drop(&mut self) {
+        // SAFETY new_private_mount makes sure to return a valid pointer
+        unsafe { bindings::kern_unmount(self.vfsmount) };
+    }
+}
--

2.41.0

Implement from_path, from_path_in_root_mnt, read_with_offset, read_to_end and get_file_size methods for a RegularFile newtype.

Signed-off-by Ariel Miculas <[email protected]>

---
 rust/helpers.c       |   6 ++
 rust/kernel/error.rs |   4 +-
 rust/kernel/file.rs  | 129 ++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 135 insertions(+), 4 deletions(-)

rust/helpers.c +6 -0

index eed8ace52fb5..9e860a554cda 100644
@@ -213,6 +213,12 @@ void *rust_helper_alloc_inode_sb(struct super_block *sb,
 }
 EXPORT_SYMBOL_GPL(rust_helper_alloc_inode_sb);
 
+loff_t rust_helper_i_size_read(const struct inode *inode)
+{
+       return i_size_read(inode);
+}
+EXPORT_SYMBOL_GPL(rust_helper_i_size_read);
+
 /*
  * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
  * as the Rust `usize` type, so we can use it in contexts where Rust

rust/kernel/error.rs +1 -3

index 05fcab6abfe6..e061c83f806a 100644
@@ -273,9 +273,7 @@ pub fn to_result(err: core::ffi::c_int) -> Result {
 ///     }
 /// }
 /// ```
-// TODO: Remove `dead_code` marker once an in-kernel client is available.
-#[allow(dead_code)]
-pub(crate) fn from_err_ptr<T>(ptr: *mut T) -> Result<*mut T> {
+pub fn from_err_ptr<T>(ptr: *mut T) -> Result<*mut T> {
     // CAST: Casting a pointer to `*const core::ffi::c_void` is always valid.
     let const_ptr: *const core::ffi::c_void = ptr.cast();
     // SAFETY: The FFI function does not deref the pointer.

rust/kernel/file.rs +5 -1

index 494939ba74df..a3002c416dbb 100644
@@ -8,11 +8,13 @@
 use crate::{
     bindings,
     cred::Credential,
-    error::{code::*, from_result, Error, Result},
+    error::{code::*, from_err_ptr, from_result, Error, Result},
     fs,
     io_buffer::{IoBufferReader, IoBufferWriter},
     iov_iter::IovIter,
     mm,
+    mount::Vfsmount,
+    str::CStr,
     sync::CondVar,
     types::ARef,
     types::AlwaysRefCounted,

@@ -20,6 +22,7 @@
     types::Opaque,
     user_ptr::{UserSlicePtr, UserSlicePtrReader, UserSlicePtrWriter},
 };
+use alloc::vec::Vec;
 use core::convert::{TryFrom, TryInto};
 use core::{marker, mem, ptr};
 use macros::vtable;

@@ -201,6 +204,130 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
     }
 }
 
+/// A newtype over file, specific to regular files

rust/kernel/file.rs +4 -0

+        // SAFETY: the safety requirements state that file_ptr's reference count was incremented at
+        // least once
+        Ok(RegularFile(unsafe { ARef::from_raw(file_ptr.cast()) }))
+    }

rust/kernel/file.rs +16 -0

+
+    /// Constructs a new [`struct file`] wrapper from a path and a vfsmount.
+    pub fn from_path_in_root_mnt(
+        mount: &Vfsmount,
+        filename: &CStr,
+        flags: i32,
+        mode: u16,
+    ) -> Result<Self> {
+        let file_ptr = {
+            let mnt = mount.get();
+            // construct a path from vfsmount, see file_open_root_mnt
+            let raw_path = bindings::path {
+                mnt,
+                // SAFETY: Vfsmount structure stores a valid vfsmount object
+                dentry: unsafe { (*mnt).mnt_root },
+            };

rust/kernel/file.rs +12 -0

+        let file_ptr = ptr::NonNull::new(file_ptr).ok_or(ENOENT)?;
+
+        // SAFETY: `file_open_root` initializes the refcount with 1
+        Self::create_if_regular(file_ptr)
+    }
+
+    /// Read from the file into the specified buffer
+    pub fn read_with_offset(&self, buf: &mut [u8], offset: u64) -> Result<usize> {
+        Ok({
+            // kernel_read_file expects a pointer to a "void *" buffer
+            let mut ptr_to_buf = buf.as_mut_ptr() as *mut core::ffi::c_void;
+            // Unless we give a non-null pointer to the file size:

rust/kernel/file.rs +6 -0

+                    offset.try_into()?,
+                    &mut ptr_to_buf,
+                    buf.len(),
+                    &mut file_size,
+                    bindings::kernel_read_file_id_READING_UNKNOWN,
+                )

rust/kernel/file.rs +22 -1

+
+    /// Allocate and return a vector containing the contents of the entire file
+    pub fn read_to_end(&self) -> Result<Vec<u8>> {
+        let file_size = self.get_file_size()?;
+        let mut buffer = Vec::try_with_capacity(file_size)?;
+        buffer.try_resize(file_size, 0)?;
+        self.read_with_offset(&mut buffer, 0)?;
+        Ok(buffer)
+    }
+
+    fn get_file_size(&self) -> Result<usize> {
+        // SAFETY: 'file' is valid because it's taken from Self
+        let file_size = unsafe { bindings::i_size_read((*self.0 .0.get()).f_inode) };
+
+        if file_size < 0 {
+            return Err(EINVAL);
+        }
+
+        Ok(file_size.try_into()?)
+    }
+}
+
 /// A file descriptor reservation.
 ///
 /// This allows the creation of a file descriptor in two steps: first, we reserve a slot for it,
--

2.41.0

Quoted 75 lines · Ariel Miculas

Implement from_path, from_path_in_root_mnt, read_with_offset, read_to_end and get_file_size methods for a RegularFile newtype.

Signed-off-by Ariel Miculas <[email protected]>

---
 rust/helpers.c       |   6 ++
 rust/kernel/error.rs |   4 +-
 rust/kernel/file.rs  | 129 ++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 135 insertions(+), 4 deletions(-)

rust/helpers.c +6 -0

index eed8ace52fb5..9e860a554cda 100644
@@ -213,6 +213,12 @@ void *rust_helper_alloc_inode_sb(struct super_block *sb,
 }
 EXPORT_SYMBOL_GPL(rust_helper_alloc_inode_sb);
 
+loff_t rust_helper_i_size_read(const struct inode *inode)
+{
+       return i_size_read(inode);
+}
+EXPORT_SYMBOL_GPL(rust_helper_i_size_read);
+
 /*
  * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
  * as the Rust `usize` type, so we can use it in contexts where Rust

rust/kernel/error.rs +1 -3

index 05fcab6abfe6..e061c83f806a 100644
@@ -273,9 +273,7 @@ pub fn to_result(err: core::ffi::c_int) -> Result {
 ///     }
 /// }
 /// ```
-// TODO: Remove `dead_code` marker once an in-kernel client is available.
-#[allow(dead_code)]
-pub(crate) fn from_err_ptr<T>(ptr: *mut T) -> Result<*mut T> {
+pub fn from_err_ptr<T>(ptr: *mut T) -> Result<*mut T> {
     // CAST: Casting a pointer to `*const core::ffi::c_void` is always valid.
     let const_ptr: *const core::ffi::c_void = ptr.cast();
     // SAFETY: The FFI function does not deref the pointer.

rust/kernel/file.rs +5 -1

index 494939ba74df..a3002c416dbb 100644
@@ -8,11 +8,13 @@
 use crate::{
     bindings,
     cred::Credential,
-    error::{code::*, from_result, Error, Result},
+    error::{code::*, from_err_ptr, from_result, Error, Result},
     fs,
     io_buffer::{IoBufferReader, IoBufferWriter},
     iov_iter::IovIter,
     mm,
+    mount::Vfsmount,
+    str::CStr,
     sync::CondVar,
     types::ARef,
     types::AlwaysRefCounted,

@@ -20,6 +22,7 @@
     types::Opaque,
     user_ptr::{UserSlicePtr, UserSlicePtrReader, UserSlicePtrWriter},
 };
+use alloc::vec::Vec;
 use core::convert::{TryFrom, TryInto};
 use core::{marker, mem, ptr};
 use macros::vtable;

@@ -201,6 +204,130 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
     }
 }
 
+/// A newtype over file, specific to regular files

rust/kernel/file.rs unsafe fn dec_ref(obj: ptr::NonNull<Self>) { · line 217

Quoted 10 lines

rust/kernel/file.rs +10 -0

+pub struct RegularFile(ARef<File>);
+impl RegularFile {
+    /// Creates a new instance of Self if the file is a regular file
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure file_ptr.f_inode is initialized to a valid pointer (e.g. file_ptr is
+    /// a pointer returned by path_openat); It must also ensure that file_ptr's reference count was
+    /// incremented at least once
+    fn create_if_regular(file_ptr: ptr::NonNull<bindings::file>) -> Result<RegularFile> {

This function should be unsafe, correct? You instead take a &bindings::file instead of NonNull but still keep it unsafe, so the "valid pointer" invariant is always reached.

Or, could this take &kernel::file::File instead to reduce some duplication?

rust/kernel/file.rs unsafe fn dec_ref(obj: ptr::NonNull<Self>) { · line 225

Quoted 8 lines

rust/kernel/file.rs +8 -0

+        // SAFETY: file_ptr is a NonNull pointer
+        let inode = unsafe { core::ptr::addr_of!((*file_ptr.as_ptr()).f_inode).read() };
+        // SAFETY: the caller must ensure f_inode is initialized to a valid pointer
+        unsafe {
+            if bindings::S_IFMT & ((*inode).i_mode) as u32 != bindings::S_IFREG {
+                return Err(EINVAL);
+            }
+        }

Nit: factor unsafe { ((*inode).i_mode) } out so it doesn't look like the whole statement is unsafe

Quoted 4 lines

rust/kernel/file.rs +4 -0

+        // SAFETY: the safety requirements state that file_ptr's reference count was incremented at
+        // least once
+        Ok(RegularFile(unsafe { ARef::from_raw(file_ptr.cast()) }))
+    }

rust/kernel/file.rs unsafe fn dec_ref(obj: ptr::NonNull<Self>) { · line 239

Quoted 10 lines

rust/kernel/file.rs +10 -0

+    /// Constructs a new [`struct file`] wrapper from a path.
+    pub fn from_path(filename: &CStr, flags: i32, mode: u16) -> Result<Self> {
+        let file_ptr = unsafe {
+            // SAFETY: filename is a reference, so it's a valid pointer
+            from_err_ptr(bindings::filp_open(
+                filename.as_ptr() as *const i8,
+                flags,
+                mode,
+            ))?
+        };

I mentioned in another email that .cast::<i8>() can be more idiomatic and a bit safer than as, it's a stylistic choice but there are a few places it could be changed here if desired

Also, the // SAFETY comments need to go outside the unsafe block

rust/kernel/file.rs unsafe fn dec_ref(obj: ptr::NonNull<Self>) { · line 244

Quoted 5 lines

rust/kernel/file.rs +5 -0

+        let file_ptr = ptr::NonNull::new(file_ptr).ok_or(ENOENT)?;
+
+        // SAFETY: `filp_open` initializes the refcount with 1
+        Self::create_if_regular(file_ptr)
+    }

Will need unsafe block if create_if_regular becomes unsafe

Quoted 16 lines

rust/kernel/file.rs +16 -0

+
+    /// Constructs a new [`struct file`] wrapper from a path and a vfsmount.
+    pub fn from_path_in_root_mnt(
+        mount: &Vfsmount,
+        filename: &CStr,
+        flags: i32,
+        mode: u16,
+    ) -> Result<Self> {
+        let file_ptr = {
+            let mnt = mount.get();
+            // construct a path from vfsmount, see file_open_root_mnt
+            let raw_path = bindings::path {
+                mnt,
+                // SAFETY: Vfsmount structure stores a valid vfsmount object
+                dentry: unsafe { (*mnt).mnt_root },
+            };

rust/kernel/file.rs unsafe fn dec_ref(obj: ptr::NonNull<Self>) { · line 270

Quoted 10 lines

rust/kernel/file.rs +10 -0

+            unsafe {
+                // SAFETY: raw_path and filename are both references
+                from_err_ptr(bindings::file_open_root(
+                    &raw_path,
+                    filename.as_ptr() as *const i8,
+                    flags,
+                    mode,
+                ))?
+            }
+        };

Is there a reason to use the larger scope block, rather than moving mnt and raw_path out and doing let file_ptr = unsafe { ... }? If so, a comment would be good.

Quoted 17 lines

rust/kernel/file.rs +16 -0

+        let file_ptr = ptr::NonNull::new(file_ptr).ok_or(ENOENT)?;
+
+        // SAFETY: `file_open_root` initializes the refcount with 1
+        Self::create_if_regular(file_ptr)
+    }
+
+    /// Read from the file into the specified buffer
+    pub fn read_with_offset(&self, buf: &mut [u8], offset: u64) -> Result<usize> {
+        Ok({
+            // kernel_read_file expects a pointer to a "void *" buffer
+            let mut ptr_to_buf = buf.as_mut_ptr() as *mut core::ffi::c_void;
+            // Unless we give a non-null pointer to the file size:
+            // 1. we cannot give a non-zero value for the offset
+            // 2. we cannot have offset 0 and buffer_size > file_size
+            let mut file_size = 0;
+

rust/kernel/file.rs unsafe fn dec_ref(obj: ptr::NonNull<Self>) { · line 292

Quoted 6 lines

rust/kernel/file.rs +6 -0

+            // SAFETY: 'file' is valid because it's taken from Self, 'buf' and 'file_size` are
+            // references to the stack variables 'ptr_to_buf' and 'file_size'; ptr_to_buf is also
+            // a pointer to a valid buffer that was obtained from a reference
+            let result = unsafe {
+                bindings::kernel_read_file(
+                    self.0 .0.get(),

Is this spacing intentional? If so, (self.0).0.get() may be cleaner

Quoted 11 lines

rust/kernel/file.rs +10 -0

+                    offset.try_into()?,
+                    &mut ptr_to_buf,
+                    buf.len(),
+                    &mut file_size,
+                    bindings::kernel_read_file_id_READING_UNKNOWN,
+                )
+            };
+
+            // kernel_read_file returns the number of bytes read on success or negative on error.
+            if result < 0 {

rust/kernel/file.rs unsafe fn dec_ref(obj: ptr::NonNull<Self>) { · line 308

Quoted 6 lines

rust/kernel/file.rs +6 -0

+                return Err(Error::from_errno(result.try_into()?));
+            }
+
+            result.try_into()?
+        })
+    }

I think you could remove the block here and just return Ok(result.try_into()?)

Quoted 30 lines

rust/kernel/file.rs +22 -1

+
+    /// Allocate and return a vector containing the contents of the entire file
+    pub fn read_to_end(&self) -> Result<Vec<u8>> {
+        let file_size = self.get_file_size()?;
+        let mut buffer = Vec::try_with_capacity(file_size)?;
+        buffer.try_resize(file_size, 0)?;
+        self.read_with_offset(&mut buffer, 0)?;
+        Ok(buffer)
+    }
+
+    fn get_file_size(&self) -> Result<usize> {
+        // SAFETY: 'file' is valid because it's taken from Self
+        let file_size = unsafe { bindings::i_size_read((*self.0 .0.get()).f_inode) };
+
+        if file_size < 0 {
+            return Err(EINVAL);
+        }
+
+        Ok(file_size.try_into()?)
+    }
+}
+
 /// A file descriptor reservation.
 ///
 /// This allows the creation of a file descriptor in two steps: first, we reserve a slot for it,
--

2.41.0

This allows us to create a Vfsmount structure and pass it to the read callback.

Signed-off-by Ariel Miculas <[email protected]>

---
 rust/kernel/file.rs      | 17 +++++++++++++++--
 samples/rust/puzzlefs.rs | 40 +++++++++++++++++++++++++++++++++++-----
 samples/rust/rust_fs.rs  |  3 ++-
 3 files changed, 52 insertions(+), 8 deletions(-)

rust/kernel/file.rs +15 -2

index a3002c416dbb..af1eb1ee9267 100644
@@ -457,9 +457,15 @@ impl<A: OpenAdapter<T::OpenData>, T: Operations> OperationsVtable<A, T> {
             // `fileref` never outlives this function, so it is guaranteed to be
             // valid.
             let fileref = unsafe { File::from_ptr(file) };
+
+            // SAFETY: into_foreign was called in fs::NewSuperBlock<..., NeedsInit>::init and
+            // it is valid until from_foreign will be called in fs::Tables::free_callback
+            let fs_info =
+                unsafe { <T::Filesystem as fs::Type>::Data::borrow((*(*inode).i_sb).s_fs_info) };
+
             // SAFETY: `arg` was previously returned by `A::convert` and must
             // be a valid non-null pointer.
-            let ptr = T::open(unsafe { &*arg }, fileref)?.into_foreign();
+            let ptr = T::open(fs_info, unsafe { &*arg }, fileref)?.into_foreign();
             // SAFETY: The C contract guarantees that `private_data` is available
             // for implementers of the file operations (no other C code accesses
             // it), so we know that there are no concurrent threads/CPUs accessing

@@ -930,10 +936,17 @@ pub trait Operations {
     /// The type of the context data passed to [`Operations::open`].
     type OpenData: Sync = ();
 
+    /// Data associated with each file system instance.
+    type Filesystem: fs::Type;
+
     /// Creates a new instance of this file.
     ///
     /// Corresponds to the `open` function pointer in `struct file_operations`.
-    fn open(context: &Self::OpenData, file: &File) -> Result<Self::Data>;
+    fn open(
+        fs_info: <<Self::Filesystem as fs::Type>::Data as ForeignOwnable>::Borrowed<'_>,
+        context: &Self::OpenData,
+        file: &File,
+    ) -> Result<Self::Data>;
 
     /// Cleans up after the last reference to the file goes away.
     ///

samples/rust/puzzlefs.rs +35 -5

index 9afd82745b64..8a64e0bd437d 100644
@@ -3,8 +3,14 @@
 //! Rust file system sample.
 
 use kernel::module_fs;
+use kernel::mount::Vfsmount;
 use kernel::prelude::*;
-use kernel::{c_str, file, fs, io_buffer::IoBufferWriter};
+use kernel::{
+    c_str, file, fmt, fs,
+    io_buffer::IoBufferWriter,
+    str::CString,
+    sync::{Arc, ArcBorrow},
+};
 
 mod puzzle;
 // Required by the autogenerated '_capnp.rs' files

@@ -19,6 +25,12 @@
 
 struct PuzzleFsModule;
 
+#[derive(Debug)]
+struct PuzzlefsInfo {
+    base_path: CString,
+    vfs_mount: Arc<Vfsmount>,
+}
+
 #[vtable]
 impl fs::Context<Self> for PuzzleFsModule {
     type Data = ();

@@ -46,14 +58,23 @@ fn try_new() -> Result {
 impl fs::Type for PuzzleFsModule {
     type Context = Self;
     type INodeData = &'static [u8];
+    type Data = Box<PuzzlefsInfo>;
     const SUPER_TYPE: fs::Super = fs::Super::Independent;
     const NAME: &'static CStr = c_str!("puzzlefs");
     const FLAGS: i32 = fs::flags::USERNS_MOUNT;
     const DCACHE_BASED: bool = true;
 
     fn fill_super(_data: (), sb: fs::NewSuperBlock<'_, Self>) -> Result<&fs::SuperBlock<Self>> {
+        let base_path = CString::try_from_fmt(fmt!("hello world"))?;
+        pr_info!("base_path {:?}\n", base_path);
+        let vfs_mount = Vfsmount::new_private_mount(c_str!("/home/puzzlefs_oci"))?;
+        pr_info!("vfs_mount {:?}\n", vfs_mount);
+
         let sb = sb.init(
-            (),
+            Box::try_new(PuzzlefsInfo {
+                base_path,
+                vfs_mount: Arc::try_new(vfs_mount)?,
+            })?,
             &fs::SuperParams {
                 magic: 0x72757374,
                 ..fs::SuperParams::DEFAULT

@@ -88,14 +109,23 @@ fn fill_super(_data: (), sb: fs::NewSuperBlock<'_, Self>) -> Result<&fs::SuperBl
 
 #[vtable]
 impl file::Operations for FsFile {
+    // must be the same as INodeData
     type OpenData = &'static [u8];
+    type Filesystem = PuzzleFsModule;
+    // this is an Arc because Data must be ForeignOwnable and the only implementors of it are Box,
+    // Arc and (); we cannot pass a reference to read, so we share Vfsmount using and Arc
+    type Data = Arc<Vfsmount>;
 
-    fn open(_context: &Self::OpenData, _file: &file::File) -> Result<Self::Data> {
-        Ok(())
+    fn open(
+        fs_info: &PuzzlefsInfo,
+        _context: &Self::OpenData,
+        _file: &file::File,
+    ) -> Result<Self::Data> {
+        Ok(fs_info.vfs_mount.clone())
     }
 
     fn read(
-        _data: (),
+        data: ArcBorrow<'_, Vfsmount>,
         file: &file::File,
         writer: &mut impl IoBufferWriter,
         offset: u64,

samples/rust/rust_fs.rs +2 -2

index 7527681ee024..c58ed1560e06 100644
@@ -85,8 +85,9 @@ fn fill_super(_data: (), sb: fs::NewSuperBlock<'_, Self>) -> Result<&fs::SuperBl
 #[vtable]
 impl file::Operations for FsFile {
     type OpenData = &'static [u8];
+    type Filesystem = RustFs;
 
-    fn open(_context: &Self::OpenData, _file: &file::File) -> Result<Self::Data> {
+    fn open(_fs_info: (), _context: &Self::OpenData, _file: &file::File) -> Result<Self::Data> {
         Ok(())
     }
 
--

2.41.0

These parameters are passed when mounting puzzlefs using '-o' option of mount:

(quoted diff) +0 -2

-o oci_root_dir="/path/to/oci/dir"
-o image_manifest="root_hash_of_image_manifest"

For a particular manifest in the manifests array in index.json (located in the oci_root_dir), the root hash of the image manifest is found in the digest field.

It would be nicer if we could pass the tag, but we don't support json deserialization.

Example of mount:
mount -t puzzlefs -o oci_root_dir="/home/puzzlefs_oci" -o \
image_manifest="2d6602d678140540dc7e96de652a76a8b16e8aca190bae141297bcffdcae901b" \
none /mnt

Signed-off-by Ariel Miculas <[email protected]>

---
 samples/rust/puzzlefs.rs | 63 ++++++++++++++++++++++++++--------------
 1 file changed, 41 insertions(+), 22 deletions(-)

samples/rust/puzzlefs.rs +30 -22

index dad7ecc76eca..4e9a8aedf0c1 100644
@@ -7,6 +7,7 @@
 use kernel::{
     c_str, file, fs,
     io_buffer::IoBufferWriter,
+    str::CString,
     sync::{Arc, ArcBorrow},
 };
 

@@ -31,27 +32,29 @@ struct PuzzlefsInfo {
     puzzlefs: Arc<PuzzleFS>,
 }
 
+#[derive(Default)]
+struct PuzzleFsParams {
+    oci_root_dir: Option<CString>,
+    image_manifest: Option<CString>,
+}
+
 #[vtable]
 impl fs::Context<Self> for PuzzleFsModule {
-    type Data = ();
-
-    kernel::define_fs_params! {(),
-        {flag, "flag", |_, v| { pr_info!("flag passed-in: {v}\n"); Ok(()) } },
-        {flag_no, "flagno", |_, v| { pr_info!("flagno passed-in: {v}\n"); Ok(()) } },
-        {bool, "bool", |_, v| { pr_info!("bool passed-in: {v}\n"); Ok(()) } },
-        {u32, "u32", |_, v| { pr_info!("u32 passed-in: {v}\n"); Ok(()) } },
-        {u32oct, "u32oct", |_, v| { pr_info!("u32oct passed-in: {v}\n"); Ok(()) } },
-        {u32hex, "u32hex", |_, v| { pr_info!("u32hex passed-in: {v}\n"); Ok(()) } },
-        {s32, "s32", |_, v| { pr_info!("s32 passed-in: {v}\n"); Ok(()) } },
-        {u64, "u64", |_, v| { pr_info!("u64 passed-in: {v}\n"); Ok(()) } },
-        {string, "string", |_, v| { pr_info!("string passed-in: {v}\n"); Ok(()) } },
-        {enum, "enum", [("first", 10), ("second", 20)], |_, v| {
-            pr_info!("enum passed-in: {v}\n"); Ok(()) }
-        },
+    type Data = Box<PuzzleFsParams>;
+
+    kernel::define_fs_params! {Box<PuzzleFsParams>,
+        {string, "oci_root_dir", |s, v| {
+                                      s.oci_root_dir = Some(CString::try_from_fmt(format_args!("{v}"))?);
+                                      Ok(())
+                                  }},
+        {string, "image_manifest", |s, v| {
+                                      s.image_manifest = Some(CString::try_from_fmt(format_args!("{v}"))?);
+                                      Ok(())
+                                  }},
     }
 
-    fn try_new() -> Result {
-        Ok(())
+    fn try_new() -> Result<Self::Data> {
+        Ok(Box::try_new(PuzzleFsParams::default())?)
     }
 }
 

@@ -136,11 +139,27 @@ impl fs::Type for PuzzleFsModule {
     const FLAGS: i32 = fs::flags::USERNS_MOUNT;
     const DCACHE_BASED: bool = true;
 
-    fn fill_super(_data: (), sb: fs::NewSuperBlock<'_, Self>) -> Result<&fs::SuperBlock<Self>> {
-        let puzzlefs = PuzzleFS::open(
-            c_str!("/home/puzzlefs_oci"),
-            c_str!("2d6602d678140540dc7e96de652a76a8b16e8aca190bae141297bcffdcae901b"),
-        );
+    fn fill_super(
+        data: Box<PuzzleFsParams>,
+        sb: fs::NewSuperBlock<'_, Self>,
+    ) -> Result<&fs::SuperBlock<Self>> {
+        let oci_root_dir = match data.oci_root_dir {
+            Some(val) => val,
+            None => {
+                pr_err!("missing oci_root_dir parameter!\n");
+                return Err(ENOTSUPP);
+            }

samples/rust/puzzlefs.rs +1 -1

+        let puzzlefs = PuzzleFS::open(&oci_root_dir, &image_manifest);
 
         if let Err(ref e) = puzzlefs {
             pr_info!("error opening puzzlefs {e}\n");
--

2.41.0

Quoted 106 lines · Ariel Miculas

These parameters are passed when mounting puzzlefs using '-o' option of mount:

(quoted diff) +0 -2

-o oci_root_dir="/path/to/oci/dir"
-o image_manifest="root_hash_of_image_manifest"

For a particular manifest in the manifests array in index.json (located in the oci_root_dir), the root hash of the image manifest is found in the digest field.

It would be nicer if we could pass the tag, but we don't support json deserialization.

Example of mount:
mount -t puzzlefs -o oci_root_dir="/home/puzzlefs_oci" -o \
image_manifest="2d6602d678140540dc7e96de652a76a8b16e8aca190bae141297bcffdcae901b" \
none /mnt

Signed-off-by Ariel Miculas <[email protected]>

---
 samples/rust/puzzlefs.rs | 63 ++++++++++++++++++++++++++--------------
 1 file changed, 41 insertions(+), 22 deletions(-)

samples/rust/puzzlefs.rs +35 -22

index dad7ecc76eca..4e9a8aedf0c1 100644
@@ -7,6 +7,7 @@
 use kernel::{
     c_str, file, fs,
     io_buffer::IoBufferWriter,
+    str::CString,
     sync::{Arc, ArcBorrow},
 };
 

@@ -31,27 +32,29 @@ struct PuzzlefsInfo {
     puzzlefs: Arc<PuzzleFS>,
 }
 
+#[derive(Default)]
+struct PuzzleFsParams {
+    oci_root_dir: Option<CString>,
+    image_manifest: Option<CString>,
+}
+
 #[vtable]
 impl fs::Context<Self> for PuzzleFsModule {
-    type Data = ();
-
-    kernel::define_fs_params! {(),
-        {flag, "flag", |_, v| { pr_info!("flag passed-in: {v}\n"); Ok(()) } },
-        {flag_no, "flagno", |_, v| { pr_info!("flagno passed-in: {v}\n"); Ok(()) } },
-        {bool, "bool", |_, v| { pr_info!("bool passed-in: {v}\n"); Ok(()) } },
-        {u32, "u32", |_, v| { pr_info!("u32 passed-in: {v}\n"); Ok(()) } },
-        {u32oct, "u32oct", |_, v| { pr_info!("u32oct passed-in: {v}\n"); Ok(()) } },
-        {u32hex, "u32hex", |_, v| { pr_info!("u32hex passed-in: {v}\n"); Ok(()) } },
-        {s32, "s32", |_, v| { pr_info!("s32 passed-in: {v}\n"); Ok(()) } },
-        {u64, "u64", |_, v| { pr_info!("u64 passed-in: {v}\n"); Ok(()) } },
-        {string, "string", |_, v| { pr_info!("string passed-in: {v}\n"); Ok(()) } },
-        {enum, "enum", [("first", 10), ("second", 20)], |_, v| {
-            pr_info!("enum passed-in: {v}\n"); Ok(()) }
-        },
+    type Data = Box<PuzzleFsParams>;
+
+    kernel::define_fs_params! {Box<PuzzleFsParams>,
+        {string, "oci_root_dir", |s, v| {
+                                      s.oci_root_dir = Some(CString::try_from_fmt(format_args!("{v}"))?);
+                                      Ok(())
+                                  }},
+        {string, "image_manifest", |s, v| {
+                                      s.image_manifest = Some(CString::try_from_fmt(format_args!("{v}"))?);
+                                      Ok(())
+                                  }},
     }
 
-    fn try_new() -> Result {
-        Ok(())
+    fn try_new() -> Result<Self::Data> {
+        Ok(Box::try_new(PuzzleFsParams::default())?)
     }
 }
 

@@ -136,11 +139,27 @@ impl fs::Type for PuzzleFsModule {
     const FLAGS: i32 = fs::flags::USERNS_MOUNT;
     const DCACHE_BASED: bool = true;
 
-    fn fill_super(_data: (), sb: fs::NewSuperBlock<'_, Self>) -> Result<&fs::SuperBlock<Self>> {
-        let puzzlefs = PuzzleFS::open(
-            c_str!("/home/puzzlefs_oci"),
-            c_str!("2d6602d678140540dc7e96de652a76a8b16e8aca190bae141297bcffdcae901b"),
-        );
+    fn fill_super(
+        data: Box<PuzzleFsParams>,
+        sb: fs::NewSuperBlock<'_, Self>,
+    ) -> Result<&fs::SuperBlock<Self>> {
+        let oci_root_dir = match data.oci_root_dir {
+            Some(val) => val,
+            None => {
+                pr_err!("missing oci_root_dir parameter!\n");
+                return Err(ENOTSUPP);
+            }
+        };
+
+        let image_manifest = match data.image_manifest {
+            Some(val) => val,
+            None => {

samples/rust/puzzlefs.rs impl fs::Type for PuzzleFsModule { · line 161

Quoted 5 lines

samples/rust/puzzlefs.rs +5 -0

+                pr_err!("missing image_manifest parameter!\n");
+                return Err(ENOTSUPP);
+            }
+        };
+

The guard syntax (available since 1.65) can make these kinds of match statements cleaner:

    let Some(oci_root_dir) = data.oci_root_dir else {
        pr_err!("missing oci_root_dir parameter!\n");
        return Err(ENOTSUPP);
    }

    let Some(image_manifest) ...

Quoted 8 lines

samples/rust/puzzlefs.rs +1 -1

+        let puzzlefs = PuzzleFS::open(&oci_root_dir, &image_manifest);
 
         if let Err(ref e) = puzzlefs {
             pr_info!("error opening puzzlefs {e}\n");
--

2.41.0

Quoted 1 line · Trevor Gross

[...]

Quoted 2 lines

The guard syntax (available since 1.65) can make these kinds of match statements cleaner:

This is unstable though.

We try to stay away from unstable features unless we really need them, which I feel is not the case here.

    let Some(oci_root_dir) = data.oci_root_dir else {
        pr_err!("missing oci_root_dir parameter!\n");
        return Err(ENOTSUPP);
    }

[...]

Quoted 9 lines · Wedson Almeida Filho

Quoted 1 line · Trevor Gross

[...]

Quoted 2 lines

The guard syntax (available since 1.65) can make these kinds of match statements cleaner:

This is unstable though.

Quoted 3 lines

We try to stay away from unstable features unless we really need them, which I feel is not the case here.

Let/else has been stable since 1.65 and is in pretty heavy use, the kernel is on 1.68.2 correct? Could you be thinking of let chains (multiple matches joined with &&/||)?

Quoted 6 lines

    let Some(oci_root_dir) = data.oci_root_dir else {
        pr_err!("missing oci_root_dir parameter!\n");
        return Err(ENOTSUPP);
    }

[...]