[RFC,v2,10/10] rust: puzzlefs: add oci_root_dir and image_manifest filesystem parameters

  These parameters are passed when mounting puzzlefs using '-o' option of
mount:
-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 <amiculas@cisco.com>
---
 samples/rust/puzzlefs.rs | 63 ++++++++++++++++++++++++++--------------
 1 file changed, 41 insertions(+), 22 deletions(-)
  

On Wed, Jul 26, 2023 at 12:55 PM Ariel Miculas <amiculas@cisco.com> wrote:
>
> These parameters are passed when mounting puzzlefs using '-o' option of
> mount:
> -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 <amiculas@cisco.com>
> ---
>  samples/rust/puzzlefs.rs | 63 ++++++++++++++++++++++++++--------------
>  1 file changed, 41 insertions(+), 22 deletions(-)
>
> diff --git a/samples/rust/puzzlefs.rs b/samples/rust/puzzlefs.rs
> index dad7ecc76eca..4e9a8aedf0c1 100644
> --- a/samples/rust/puzzlefs.rs
> +++ b/samples/rust/puzzlefs.rs
> @@ -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 => {
> +                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) ...

> +        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
>
>
  

On Wed, 26 Jul 2023 at 18:08, Trevor Gross <tmgross@umich.edu> wrote:
>
> On Wed, Jul 26, 2023 at 12:55 PM Ariel Miculas <amiculas@cisco.com> wrote:
> >
> > These parameters are passed when mounting puzzlefs using '-o' option of
> > mount:
> > -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 <amiculas@cisco.com>
> > ---
> >  samples/rust/puzzlefs.rs | 63 ++++++++++++++++++++++++++--------------
> >  1 file changed, 41 insertions(+), 22 deletions(-)
> >
> > diff --git a/samples/rust/puzzlefs.rs b/samples/rust/puzzlefs.rs
> > index dad7ecc76eca..4e9a8aedf0c1 100644
> > --- a/samples/rust/puzzlefs.rs
> > +++ b/samples/rust/puzzlefs.rs
> > @@ -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 => {
> > +                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:

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);
>     }
>
>     let Some(image_manifest) ...
>
> > +        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
> >
> >
  

On Wed, Jul 26, 2023 at 7:48 PM Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
>
> On Wed, 26 Jul 2023 at 18:08, Trevor Gross <tmgross@umich.edu> wrote:
> >
> [...]
> > 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/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 `&&`/`||`)?

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

On Wed, 26 Jul 2023 at 21:02, Trevor Gross <tmgross@umich.edu> wrote:
>
> On Wed, Jul 26, 2023 at 7:48 PM Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
> >
> > On Wed, 26 Jul 2023 at 18:08, Trevor Gross <tmgross@umich.edu> wrote:
> > >
> > [...]
> > > 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/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 `&&`/`||`)?

Oh, my bad. I got it mixed with "if let guard" (issue 51114).

And yeah, we're on 1.68.2, so this code can benefit from this simplification.

Thanks!

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