diff mbox series

[RFC,v2,1/4] fs: add helper to use mount option as path or fd

Message ID 20241011-work-overlayfs-v2-1-1b43328c5a31@kernel.org (mailing list archive)
State New
Headers show
Series ovl: specify layers via file descriptors | expand

Commit Message

Christian Brauner Oct. 11, 2024, 9:45 p.m. UTC 
Allow filesystems to use a mount option either as a
path or a file descriptor.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/fs_parser.c            | 19 +++++++++++++++++++
 include/linux/fs_parser.h |  5 ++++-
 2 files changed, 23 insertions(+), 1 deletion(-)

Comments

Amir Goldstein Oct. 12, 2024, 7:21 a.m. UTC | #1 
On Fri, Oct 11, 2024 at 11:46 PM Christian Brauner <brauner@kernel.org> wrote:
>
> Allow filesystems to use a mount option either as a
> path or a file descriptor.
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Looks sane

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

> ---
>  fs/fs_parser.c            | 19 +++++++++++++++++++
>  include/linux/fs_parser.h |  5 ++++-
>  2 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/fs/fs_parser.c b/fs/fs_parser.c
> index 24727ec34e5aa434364e87879cccf9fe1ec19d37..a017415d8d6bc91608ece5d42fa4bea26e47456b 100644
> --- a/fs/fs_parser.c
> +++ b/fs/fs_parser.c
> @@ -308,6 +308,25 @@ int fs_param_is_fd(struct p_log *log, const struct fs_parameter_spec *p,
>  }
>  EXPORT_SYMBOL(fs_param_is_fd);
>
> +int fs_param_is_fd_or_path(struct p_log *log, const struct fs_parameter_spec *p,
> +                          struct fs_parameter *param,
> +                          struct fs_parse_result *result)
> +{
> +       switch (param->type) {
> +       case fs_value_is_string:
> +               return fs_param_is_string(log, p, param, result);
> +       case fs_value_is_file:
> +               result->uint_32 = param->dirfd;
> +               if (result->uint_32 <= INT_MAX)
> +                       return 0;
> +               break;
> +       default:
> +               break;
> +       }
> +       return fs_param_bad_value(log, param);
> +}
> +EXPORT_SYMBOL(fs_param_is_fd_or_path);
> +
>  int fs_param_is_uid(struct p_log *log, const struct fs_parameter_spec *p,
>                     struct fs_parameter *param, struct fs_parse_result *result)
>  {
> diff --git a/include/linux/fs_parser.h b/include/linux/fs_parser.h
> index 6cf713a7e6c6fc2402a68c87036264eaed921432..73fe4e119ee24b3bed1f0cf2bc23d6b31811cb69 100644
> --- a/include/linux/fs_parser.h
> +++ b/include/linux/fs_parser.h
> @@ -28,7 +28,8 @@ typedef int fs_param_type(struct p_log *,
>   */
>  fs_param_type fs_param_is_bool, fs_param_is_u32, fs_param_is_s32, fs_param_is_u64,
>         fs_param_is_enum, fs_param_is_string, fs_param_is_blob, fs_param_is_blockdev,
> -       fs_param_is_path, fs_param_is_fd, fs_param_is_uid, fs_param_is_gid;
> +       fs_param_is_path, fs_param_is_fd, fs_param_is_uid, fs_param_is_gid,
> +       fs_param_is_fd_or_path;
>
>  /*
>   * Specification of the type of value a parameter wants.
> @@ -133,6 +134,8 @@ static inline bool fs_validate_description(const char *name,
>  #define fsparam_bdev(NAME, OPT)        __fsparam(fs_param_is_blockdev, NAME, OPT, 0, NULL)
>  #define fsparam_path(NAME, OPT)        __fsparam(fs_param_is_path, NAME, OPT, 0, NULL)
>  #define fsparam_fd(NAME, OPT)  __fsparam(fs_param_is_fd, NAME, OPT, 0, NULL)
> +#define fsparam_fd_or_path(NAME, OPT) \
> +                               __fsparam(fs_param_is_fd_or_path, NAME, OPT, 0, NULL)
>  #define fsparam_uid(NAME, OPT) __fsparam(fs_param_is_uid, NAME, OPT, 0, NULL)
>  #define fsparam_gid(NAME, OPT) __fsparam(fs_param_is_gid, NAME, OPT, 0, NULL)
>
>
> --
> 2.45.2
>
Amir Goldstein Oct. 12, 2024, 8:20 a.m. UTC | #2 
On Sat, Oct 12, 2024 at 9:21 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Fri, Oct 11, 2024 at 11:46 PM Christian Brauner <brauner@kernel.org> wrote:
> >
> > Allow filesystems to use a mount option either as a
> > path or a file descriptor.
> >
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
>
> Looks sane
>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
>
> > ---
> >  fs/fs_parser.c            | 19 +++++++++++++++++++
> >  include/linux/fs_parser.h |  5 ++++-
> >  2 files changed, 23 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/fs_parser.c b/fs/fs_parser.c
> > index 24727ec34e5aa434364e87879cccf9fe1ec19d37..a017415d8d6bc91608ece5d42fa4bea26e47456b 100644
> > --- a/fs/fs_parser.c
> > +++ b/fs/fs_parser.c
> > @@ -308,6 +308,25 @@ int fs_param_is_fd(struct p_log *log, const struct fs_parameter_spec *p,
> >  }
> >  EXPORT_SYMBOL(fs_param_is_fd);
> >
> > +int fs_param_is_fd_or_path(struct p_log *log, const struct fs_parameter_spec *p,
> > +                          struct fs_parameter *param,
> > +                          struct fs_parse_result *result)
> > +{
> > +       switch (param->type) {
> > +       case fs_value_is_string:
> > +               return fs_param_is_string(log, p, param, result);
> > +       case fs_value_is_file:
> > +               result->uint_32 = param->dirfd;
> > +               if (result->uint_32 <= INT_MAX)
> > +                       return 0;
> > +               break;
> > +       default:
> > +               break;
> > +       }
> > +       return fs_param_bad_value(log, param);
> > +}
> > +EXPORT_SYMBOL(fs_param_is_fd_or_path);
> > +

I just noticed that it is a little weird that fsparam_is_fd() accepts a numeric
string while fsparam_is_fd_or_path() does not.
Not to mention that fsparam_is_fd_or_path does not accept type filename.

Obviously a helper name fs_param_is_file_or_string() wouldn't have
raised those questions.
I will let you decide if this is something to worry about.

Thanks,
Amir.
Christian Brauner Oct. 14, 2024, 8:20 a.m. UTC | #3 
On Sat, Oct 12, 2024 at 10:20:04AM +0200, Amir Goldstein wrote:
> On Sat, Oct 12, 2024 at 9:21 AM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > On Fri, Oct 11, 2024 at 11:46 PM Christian Brauner <brauner@kernel.org> wrote:
> > >
> > > Allow filesystems to use a mount option either as a
> > > path or a file descriptor.
> > >
> > > Signed-off-by: Christian Brauner <brauner@kernel.org>
> >
> > Looks sane
> >
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> >
> > > ---
> > >  fs/fs_parser.c            | 19 +++++++++++++++++++
> > >  include/linux/fs_parser.h |  5 ++++-
> > >  2 files changed, 23 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/fs/fs_parser.c b/fs/fs_parser.c
> > > index 24727ec34e5aa434364e87879cccf9fe1ec19d37..a017415d8d6bc91608ece5d42fa4bea26e47456b 100644
> > > --- a/fs/fs_parser.c
> > > +++ b/fs/fs_parser.c
> > > @@ -308,6 +308,25 @@ int fs_param_is_fd(struct p_log *log, const struct fs_parameter_spec *p,
> > >  }
> > >  EXPORT_SYMBOL(fs_param_is_fd);
> > >
> > > +int fs_param_is_fd_or_path(struct p_log *log, const struct fs_parameter_spec *p,
> > > +                          struct fs_parameter *param,
> > > +                          struct fs_parse_result *result)
> > > +{
> > > +       switch (param->type) {
> > > +       case fs_value_is_string:
> > > +               return fs_param_is_string(log, p, param, result);
> > > +       case fs_value_is_file:
> > > +               result->uint_32 = param->dirfd;
> > > +               if (result->uint_32 <= INT_MAX)
> > > +                       return 0;
> > > +               break;
> > > +       default:
> > > +               break;
> > > +       }
> > > +       return fs_param_bad_value(log, param);
> > > +}
> > > +EXPORT_SYMBOL(fs_param_is_fd_or_path);
> > > +
> 
> I just noticed that it is a little weird that fsparam_is_fd() accepts a numeric
> string while fsparam_is_fd_or_path() does not.
> Not to mention that fsparam_is_fd_or_path does not accept type filename.
> 
> Obviously a helper name fs_param_is_file_or_string() wouldn't have

Yes, I'll use that. Thanks!
diff mbox series

Patch

diff --git a/fs/fs_parser.c b/fs/fs_parser.c
index 24727ec34e5aa434364e87879cccf9fe1ec19d37..a017415d8d6bc91608ece5d42fa4bea26e47456b 100644
--- a/fs/fs_parser.c
+++ b/fs/fs_parser.c
@@ -308,6 +308,25 @@  int fs_param_is_fd(struct p_log *log, const struct fs_parameter_spec *p,
 }
 EXPORT_SYMBOL(fs_param_is_fd);
 
+int fs_param_is_fd_or_path(struct p_log *log, const struct fs_parameter_spec *p,
+			   struct fs_parameter *param,
+			   struct fs_parse_result *result)
+{
+	switch (param->type) {
+	case fs_value_is_string:
+		return fs_param_is_string(log, p, param, result);
+	case fs_value_is_file:
+		result->uint_32 = param->dirfd;
+		if (result->uint_32 <= INT_MAX)
+			return 0;
+		break;
+	default:
+		break;
+	}
+	return fs_param_bad_value(log, param);
+}
+EXPORT_SYMBOL(fs_param_is_fd_or_path);
+
 int fs_param_is_uid(struct p_log *log, const struct fs_parameter_spec *p,
 		    struct fs_parameter *param, struct fs_parse_result *result)
 {
diff --git a/include/linux/fs_parser.h b/include/linux/fs_parser.h
index 6cf713a7e6c6fc2402a68c87036264eaed921432..73fe4e119ee24b3bed1f0cf2bc23d6b31811cb69 100644
--- a/include/linux/fs_parser.h
+++ b/include/linux/fs_parser.h
@@ -28,7 +28,8 @@  typedef int fs_param_type(struct p_log *,
  */
 fs_param_type fs_param_is_bool, fs_param_is_u32, fs_param_is_s32, fs_param_is_u64,
 	fs_param_is_enum, fs_param_is_string, fs_param_is_blob, fs_param_is_blockdev,
-	fs_param_is_path, fs_param_is_fd, fs_param_is_uid, fs_param_is_gid;
+	fs_param_is_path, fs_param_is_fd, fs_param_is_uid, fs_param_is_gid,
+	fs_param_is_fd_or_path;
 
 /*
  * Specification of the type of value a parameter wants.
@@ -133,6 +134,8 @@  static inline bool fs_validate_description(const char *name,
 #define fsparam_bdev(NAME, OPT)	__fsparam(fs_param_is_blockdev, NAME, OPT, 0, NULL)
 #define fsparam_path(NAME, OPT)	__fsparam(fs_param_is_path, NAME, OPT, 0, NULL)
 #define fsparam_fd(NAME, OPT)	__fsparam(fs_param_is_fd, NAME, OPT, 0, NULL)
+#define fsparam_fd_or_path(NAME, OPT) \
+				__fsparam(fs_param_is_fd_or_path, NAME, OPT, 0, NULL)
 #define fsparam_uid(NAME, OPT) __fsparam(fs_param_is_uid, NAME, OPT, 0, NULL)
 #define fsparam_gid(NAME, OPT) __fsparam(fs_param_is_gid, NAME, OPT, 0, NULL)