linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] adfs, affs, befs, hfs, hfsplus: convert to new mount api
@ 2024-09-16 17:26 Eric Sandeen
  2024-09-16 17:26 ` [PATCH 1/5] adfs: convert adfs to use the " Eric Sandeen
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Eric Sandeen @ 2024-09-16 17:26 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: brauner

These were all tested against images I created or obtained, using a
script to test random combinations of valid and invalid mount and
remount options, and comparing the results before and after the changes.

AFAICT, all parsing works as expected and behavior is unchanged.

(Changes since first send: fixing a couple string leaks, added hfs
and hfsplus.)

Thanks,
-Eric



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/5] adfs: convert adfs to use the new mount api
  2024-09-16 17:26 [PATCH 0/5] adfs, affs, befs, hfs, hfsplus: convert to new mount api Eric Sandeen
@ 2024-09-16 17:26 ` Eric Sandeen
  2024-09-17 12:52   ` Jan Kara
  2024-09-16 17:26 ` [PATCH 2/5] affs: convert affs " Eric Sandeen
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Eric Sandeen @ 2024-09-16 17:26 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: brauner, Eric Sandeen

Convert the adfs filesystem to use the new mount API.
Tested by comparing random mount & remount options before and after
the change.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/adfs/super.c | 186 +++++++++++++++++++++++++-----------------------
 1 file changed, 95 insertions(+), 91 deletions(-)

diff --git a/fs/adfs/super.c b/fs/adfs/super.c
index f0b999a4961b..017c48a80203 100644
--- a/fs/adfs/super.c
+++ b/fs/adfs/super.c
@@ -6,7 +6,8 @@
  */
 #include <linux/module.h>
 #include <linux/init.h>
-#include <linux/parser.h>
+#include <linux/fs_parser.h>
+#include <linux/fs_context.h>
 #include <linux/mount.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
@@ -115,87 +116,61 @@ static int adfs_show_options(struct seq_file *seq, struct dentry *root)
 	return 0;
 }
 
-enum {Opt_uid, Opt_gid, Opt_ownmask, Opt_othmask, Opt_ftsuffix, Opt_err};
+enum {Opt_uid, Opt_gid, Opt_ownmask, Opt_othmask, Opt_ftsuffix};
 
-static const match_table_t tokens = {
-	{Opt_uid, "uid=%u"},
-	{Opt_gid, "gid=%u"},
-	{Opt_ownmask, "ownmask=%o"},
-	{Opt_othmask, "othmask=%o"},
-	{Opt_ftsuffix, "ftsuffix=%u"},
-	{Opt_err, NULL}
+static const struct fs_parameter_spec adfs_param_spec[] = {
+	fsparam_uid	("uid",		Opt_uid),
+	fsparam_gid	("gid",		Opt_gid),
+	fsparam_u32oct	("ownmask",	Opt_ownmask),
+	fsparam_u32oct	("othmask",	Opt_othmask),
+	fsparam_u32	("ftsuffix",	Opt_ftsuffix),
+	{}
 };
 
-static int parse_options(struct super_block *sb, struct adfs_sb_info *asb,
-			 char *options)
+static int adfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
 {
-	char *p;
-	int option;
-
-	if (!options)
-		return 0;
-
-	while ((p = strsep(&options, ",")) != NULL) {
-		substring_t args[MAX_OPT_ARGS];
-		int token;
-		if (!*p)
-			continue;
-
-		token = match_token(p, tokens, args);
-		switch (token) {
-		case Opt_uid:
-			if (match_int(args, &option))
-				return -EINVAL;
-			asb->s_uid = make_kuid(current_user_ns(), option);
-			if (!uid_valid(asb->s_uid))
-				return -EINVAL;
-			break;
-		case Opt_gid:
-			if (match_int(args, &option))
-				return -EINVAL;
-			asb->s_gid = make_kgid(current_user_ns(), option);
-			if (!gid_valid(asb->s_gid))
-				return -EINVAL;
-			break;
-		case Opt_ownmask:
-			if (match_octal(args, &option))
-				return -EINVAL;
-			asb->s_owner_mask = option;
-			break;
-		case Opt_othmask:
-			if (match_octal(args, &option))
-				return -EINVAL;
-			asb->s_other_mask = option;
-			break;
-		case Opt_ftsuffix:
-			if (match_int(args, &option))
-				return -EINVAL;
-			asb->s_ftsuffix = option;
-			break;
-		default:
-			adfs_msg(sb, KERN_ERR,
-				 "unrecognised mount option \"%s\" or missing value",
-				 p);
-			return -EINVAL;
-		}
+	struct adfs_sb_info *asb = fc->s_fs_info;
+	struct fs_parse_result result;
+	int opt;
+
+	opt = fs_parse(fc, adfs_param_spec, param, &result);
+	if (opt < 0)
+		return opt;
+
+	switch (opt) {
+	case Opt_uid:
+		asb->s_uid = result.uid;
+		break;
+	case Opt_gid:
+		asb->s_gid = result.gid;
+		break;
+	case Opt_ownmask:
+		asb->s_owner_mask = result.uint_32;
+		break;
+	case Opt_othmask:
+		asb->s_other_mask = result.uint_32;
+		break;
+	case Opt_ftsuffix:
+		asb->s_ftsuffix = result.uint_32;
+		break;
+	default:
+		return -EINVAL;
 	}
 	return 0;
 }
 
-static int adfs_remount(struct super_block *sb, int *flags, char *data)
+static int adfs_reconfigure(struct fs_context *fc)
 {
-	struct adfs_sb_info temp_asb;
-	int ret;
+	struct adfs_sb_info *new_asb = fc->s_fs_info;
+	struct adfs_sb_info *asb = ADFS_SB(fc->root->d_sb);
 
-	sync_filesystem(sb);
-	*flags |= ADFS_SB_FLAGS;
+	sync_filesystem(fc->root->d_sb);
+	fc->sb_flags |= ADFS_SB_FLAGS;
 
-	temp_asb = *ADFS_SB(sb);
-	ret = parse_options(sb, &temp_asb, data);
-	if (ret == 0)
-		*ADFS_SB(sb) = temp_asb;
+	/* Structure copy newly parsed options */
+	*asb = *new_asb;
 
-	return ret;
+	return 0;
 }
 
 static int adfs_statfs(struct dentry *dentry, struct kstatfs *buf)
@@ -273,7 +248,6 @@ static const struct super_operations adfs_sops = {
 	.write_inode	= adfs_write_inode,
 	.put_super	= adfs_put_super,
 	.statfs		= adfs_statfs,
-	.remount_fs	= adfs_remount,
 	.show_options	= adfs_show_options,
 };
 
@@ -361,34 +335,21 @@ static int adfs_validate_dr0(struct super_block *sb, struct buffer_head *bh,
 	return 0;
 }
 
-static int adfs_fill_super(struct super_block *sb, void *data, int silent)
+static int adfs_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	struct adfs_discrecord *dr;
 	struct object_info root_obj;
-	struct adfs_sb_info *asb;
+	struct adfs_sb_info *asb = sb->s_fs_info;
 	struct inode *root;
 	int ret = -EINVAL;
+	int silent = fc->sb_flags & SB_SILENT;
 
 	sb->s_flags |= ADFS_SB_FLAGS;
 
-	asb = kzalloc(sizeof(*asb), GFP_KERNEL);
-	if (!asb)
-		return -ENOMEM;
-
 	sb->s_fs_info = asb;
 	sb->s_magic = ADFS_SUPER_MAGIC;
 	sb->s_time_gran = 10000000;
 
-	/* set default options */
-	asb->s_uid = GLOBAL_ROOT_UID;
-	asb->s_gid = GLOBAL_ROOT_GID;
-	asb->s_owner_mask = ADFS_DEFAULT_OWNER_MASK;
-	asb->s_other_mask = ADFS_DEFAULT_OTHER_MASK;
-	asb->s_ftsuffix = 0;
-
-	if (parse_options(sb, asb, data))
-		goto error;
-
 	/* Try to probe the filesystem boot block */
 	ret = adfs_probe(sb, ADFS_DISCRECORD, 1, adfs_validate_bblk);
 	if (ret == -EILSEQ)
@@ -453,18 +414,61 @@ static int adfs_fill_super(struct super_block *sb, void *data, int silent)
 	return ret;
 }
 
-static struct dentry *adfs_mount(struct file_system_type *fs_type,
-	int flags, const char *dev_name, void *data)
+static int adfs_get_tree(struct fs_context *fc)
+{
+	return get_tree_bdev(fc, adfs_fill_super);
+}
+
+static void adfs_free_fc(struct fs_context *fc)
 {
-	return mount_bdev(fs_type, flags, dev_name, data, adfs_fill_super);
+	struct adfs_context *asb = fc->s_fs_info;
+
+	kfree(asb);
+}
+
+static const struct fs_context_operations adfs_context_ops = {
+	.parse_param	= adfs_parse_param,
+	.get_tree	= adfs_get_tree,
+	.reconfigure	= adfs_reconfigure,
+	.free		= adfs_free_fc,
+};
+
+static int adfs_init_fs_context(struct fs_context *fc)
+{
+	struct adfs_sb_info *asb;
+
+	asb = kzalloc(sizeof(struct adfs_sb_info), GFP_KERNEL);
+	if (!asb)
+		return -ENOMEM;
+
+	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
+		struct super_block *sb = fc->root->d_sb;
+		struct adfs_sb_info *old_asb = ADFS_SB(sb);
+
+		/* structure copy existing options before parsing */
+		*asb = *old_asb;
+	} else {
+		/* set default options */
+		asb->s_uid = GLOBAL_ROOT_UID;
+		asb->s_gid = GLOBAL_ROOT_GID;
+		asb->s_owner_mask = ADFS_DEFAULT_OWNER_MASK;
+		asb->s_other_mask = ADFS_DEFAULT_OTHER_MASK;
+		asb->s_ftsuffix = 0;
+	}
+
+	fc->ops = &adfs_context_ops;
+	fc->s_fs_info = asb;
+
+	return 0;
 }
 
 static struct file_system_type adfs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "adfs",
-	.mount		= adfs_mount,
 	.kill_sb	= kill_block_super,
 	.fs_flags	= FS_REQUIRES_DEV,
+	.init_fs_context = adfs_init_fs_context,
+	.parameters	= adfs_param_spec,
 };
 MODULE_ALIAS_FS("adfs");
 
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/5] affs: convert affs to use the new mount api
  2024-09-16 17:26 [PATCH 0/5] adfs, affs, befs, hfs, hfsplus: convert to new mount api Eric Sandeen
  2024-09-16 17:26 ` [PATCH 1/5] adfs: convert adfs to use the " Eric Sandeen
@ 2024-09-16 17:26 ` Eric Sandeen
  2024-09-17 12:51   ` Jan Kara
  2024-09-17 14:53   ` [PATCH 2/5 V2] " Eric Sandeen
  2024-09-16 17:26 ` [PATCH 3/5] befs: convert befs " Eric Sandeen
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 14+ messages in thread
From: Eric Sandeen @ 2024-09-16 17:26 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: brauner, Eric Sandeen, David Sterba

Convert the affs filesystem to use the new mount API.
Tested by comparing random mount & remount options before and after
the change.

Cc: David Sterba <dsterba@suse.com>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/affs/super.c | 375 ++++++++++++++++++++++++------------------------
 1 file changed, 189 insertions(+), 186 deletions(-)

diff --git a/fs/affs/super.c b/fs/affs/super.c
index 3c5821339609..8010ad991dad 100644
--- a/fs/affs/super.c
+++ b/fs/affs/super.c
@@ -14,7 +14,8 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/statfs.h>
-#include <linux/parser.h>
+#include <linux/fs_parser.h>
+#include <linux/fs_context.h>
 #include <linux/magic.h>
 #include <linux/sched.h>
 #include <linux/cred.h>
@@ -27,7 +28,6 @@
 
 static int affs_statfs(struct dentry *dentry, struct kstatfs *buf);
 static int affs_show_options(struct seq_file *m, struct dentry *root);
-static int affs_remount (struct super_block *sb, int *flags, char *data);
 
 static void
 affs_commit_super(struct super_block *sb, int wait)
@@ -155,140 +155,115 @@ static const struct super_operations affs_sops = {
 	.put_super	= affs_put_super,
 	.sync_fs	= affs_sync_fs,
 	.statfs		= affs_statfs,
-	.remount_fs	= affs_remount,
 	.show_options	= affs_show_options,
 };
 
 enum {
 	Opt_bs, Opt_mode, Opt_mufs, Opt_notruncate, Opt_prefix, Opt_protect,
 	Opt_reserved, Opt_root, Opt_setgid, Opt_setuid,
-	Opt_verbose, Opt_volume, Opt_ignore, Opt_err,
+	Opt_verbose, Opt_volume, Opt_ignore,
 };
 
-static const match_table_t tokens = {
-	{Opt_bs, "bs=%u"},
-	{Opt_mode, "mode=%o"},
-	{Opt_mufs, "mufs"},
-	{Opt_notruncate, "nofilenametruncate"},
-	{Opt_prefix, "prefix=%s"},
-	{Opt_protect, "protect"},
-	{Opt_reserved, "reserved=%u"},
-	{Opt_root, "root=%u"},
-	{Opt_setgid, "setgid=%u"},
-	{Opt_setuid, "setuid=%u"},
-	{Opt_verbose, "verbose"},
-	{Opt_volume, "volume=%s"},
-	{Opt_ignore, "grpquota"},
-	{Opt_ignore, "noquota"},
-	{Opt_ignore, "quota"},
-	{Opt_ignore, "usrquota"},
-	{Opt_err, NULL},
+struct affs_context {
+	kuid_t		uid;		/* uid to override */
+	kgid_t		gid;		/* gid to override */
+	unsigned int	mode;		/* mode to override */
+	unsigned int	reserved;	/* Number of reserved blocks */
+	int		root_block;	/* FFS root block number */
+	int		blocksize;	/* Initial device blksize */
+	char		*prefix;	/* Prefix for volumes and assigns */
+	char		volume[32];	/* Vol. prefix for absolute symlinks */
+	unsigned long	mount_flags;	/* Options */
 };
 
-static int
-parse_options(char *options, kuid_t *uid, kgid_t *gid, int *mode, int *reserved, s32 *root,
-		int *blocksize, char **prefix, char *volume, unsigned long *mount_opts)
+static const struct fs_parameter_spec affs_param_spec[] = {
+	fsparam_u32	("bs",		Opt_bs),
+	fsparam_u32oct	("mode",	Opt_mode),
+	fsparam_flag	("mufs",	Opt_mufs),
+	fsparam_flag	("nofilenametruncate",	Opt_notruncate),
+	fsparam_string	("prefix",	Opt_prefix),
+	fsparam_flag	("protect",	Opt_protect),
+	fsparam_u32	("reserved",	Opt_reserved),
+	fsparam_u32	("root",	Opt_root),
+	fsparam_gid	("setgid",	Opt_setgid),
+	fsparam_uid	("setuid",	Opt_setuid),
+	fsparam_flag	("verbose",	Opt_verbose),
+	fsparam_string	("volume",	Opt_volume),
+	fsparam_flag	("grpquota",	Opt_ignore),
+	fsparam_flag	("noquota",	Opt_ignore),
+	fsparam_flag	("quota",	Opt_ignore),
+	fsparam_flag	("usrquota",	Opt_ignore),
+	{},
+};
+
+static int affs_parse_param(struct fs_context *fc, struct fs_parameter *param)
 {
-	char *p;
-	substring_t args[MAX_OPT_ARGS];
-
-	/* Fill in defaults */
-
-	*uid        = current_uid();
-	*gid        = current_gid();
-	*reserved   = 2;
-	*root       = -1;
-	*blocksize  = -1;
-	volume[0]   = ':';
-	volume[1]   = 0;
-	*mount_opts = 0;
-	if (!options)
-		return 1;
-
-	while ((p = strsep(&options, ",")) != NULL) {
-		int token, n, option;
-		if (!*p)
-			continue;
-
-		token = match_token(p, tokens, args);
-		switch (token) {
-		case Opt_bs:
-			if (match_int(&args[0], &n))
-				return 0;
-			if (n != 512 && n != 1024 && n != 2048
-			    && n != 4096) {
-				pr_warn("Invalid blocksize (512, 1024, 2048, 4096 allowed)\n");
-				return 0;
-			}
-			*blocksize = n;
-			break;
-		case Opt_mode:
-			if (match_octal(&args[0], &option))
-				return 0;
-			*mode = option & 0777;
-			affs_set_opt(*mount_opts, SF_SETMODE);
-			break;
-		case Opt_mufs:
-			affs_set_opt(*mount_opts, SF_MUFS);
-			break;
-		case Opt_notruncate:
-			affs_set_opt(*mount_opts, SF_NO_TRUNCATE);
-			break;
-		case Opt_prefix:
-			kfree(*prefix);
-			*prefix = match_strdup(&args[0]);
-			if (!*prefix)
-				return 0;
-			affs_set_opt(*mount_opts, SF_PREFIX);
-			break;
-		case Opt_protect:
-			affs_set_opt(*mount_opts, SF_IMMUTABLE);
-			break;
-		case Opt_reserved:
-			if (match_int(&args[0], reserved))
-				return 0;
-			break;
-		case Opt_root:
-			if (match_int(&args[0], root))
-				return 0;
-			break;
-		case Opt_setgid:
-			if (match_int(&args[0], &option))
-				return 0;
-			*gid = make_kgid(current_user_ns(), option);
-			if (!gid_valid(*gid))
-				return 0;
-			affs_set_opt(*mount_opts, SF_SETGID);
-			break;
-		case Opt_setuid:
-			if (match_int(&args[0], &option))
-				return 0;
-			*uid = make_kuid(current_user_ns(), option);
-			if (!uid_valid(*uid))
-				return 0;
-			affs_set_opt(*mount_opts, SF_SETUID);
-			break;
-		case Opt_verbose:
-			affs_set_opt(*mount_opts, SF_VERBOSE);
-			break;
-		case Opt_volume: {
-			char *vol = match_strdup(&args[0]);
-			if (!vol)
-				return 0;
-			strscpy(volume, vol, 32);
-			kfree(vol);
-			break;
-		}
-		case Opt_ignore:
-		 	/* Silently ignore the quota options */
-			break;
-		default:
-			pr_warn("Unrecognized mount option \"%s\" or missing value\n",
-				p);
-			return 0;
+	struct affs_context *ctx = fc->fs_private;
+	struct fs_parse_result result;
+	int n;
+	int opt;
+
+	opt = fs_parse(fc, affs_param_spec, param, &result);
+	if (opt < 0)
+		return opt;
+
+	switch (opt) {
+	case Opt_bs:
+		n = result.uint_32;
+		if (n != 512 && n != 1024 && n != 2048
+		    && n != 4096) {
+			pr_warn("Invalid blocksize (512, 1024, 2048, 4096 allowed)\n");
+			return -EINVAL;
 		}
+		ctx->blocksize = n;
+		break;
+	case Opt_mode:
+		ctx->mode = result.uint_32 & 0777;
+		affs_set_opt(ctx->mount_flags, SF_SETMODE);
+		break;
+	case Opt_mufs:
+		affs_set_opt(ctx->mount_flags, SF_MUFS);
+		break;
+	case Opt_notruncate:
+		affs_set_opt(ctx->mount_flags, SF_NO_TRUNCATE);
+		break;
+	case Opt_prefix:
+		kfree(ctx->prefix);
+		ctx->prefix = param->string;
+		param->string = NULL;
+		affs_set_opt(ctx->mount_flags, SF_PREFIX);
+		break;
+	case Opt_protect:
+		affs_set_opt(ctx->mount_flags, SF_IMMUTABLE);
+		break;
+	case Opt_reserved:
+		ctx->reserved = result.uint_32;
+		break;
+	case Opt_root:
+		ctx->root_block = result.uint_32;
+		break;
+	case Opt_setgid:
+		ctx->gid = result.gid;
+		affs_set_opt(ctx->mount_flags, SF_SETGID);
+		break;
+	case Opt_setuid:
+		ctx->uid = result.uid;
+		affs_set_opt(ctx->mount_flags, SF_SETUID);
+		break;
+	case Opt_verbose:
+		affs_set_opt(ctx->mount_flags, SF_VERBOSE);
+		break;
+	case Opt_volume: {
+		strscpy(ctx->volume, param->string, 32);
+		break;
 	}
-	return 1;
+	case Opt_ignore:
+	 	/* Silently ignore the quota options */
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
 }
 
 static int affs_show_options(struct seq_file *m, struct dentry *root)
@@ -329,27 +304,22 @@ static int affs_show_options(struct seq_file *m, struct dentry *root)
  * hopefully have the guts to do so. Until then: sorry for the mess.
  */
 
-static int affs_fill_super(struct super_block *sb, void *data, int silent)
+static int affs_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	struct affs_sb_info	*sbi;
+	struct affs_context	*ctx = fc->fs_private;
 	struct buffer_head	*root_bh = NULL;
 	struct buffer_head	*boot_bh;
 	struct inode		*root_inode = NULL;
-	s32			 root_block;
+	int			 silent = fc->sb_flags & SB_SILENT;
 	int			 size, blocksize;
 	u32			 chksum;
 	int			 num_bm;
 	int			 i, j;
-	kuid_t			 uid;
-	kgid_t			 gid;
-	int			 reserved;
-	unsigned long		 mount_flags;
 	int			 tmp_flags;	/* fix remount prototype... */
 	u8			 sig[4];
 	int			 ret;
 
-	pr_debug("read_super(%s)\n", data ? (const char *)data : "no options");
-
 	sb->s_magic             = AFFS_SUPER_MAGIC;
 	sb->s_op                = &affs_sops;
 	sb->s_flags |= SB_NODIRATIME;
@@ -369,19 +339,16 @@ static int affs_fill_super(struct super_block *sb, void *data, int silent)
 	spin_lock_init(&sbi->work_lock);
 	INIT_DELAYED_WORK(&sbi->sb_work, flush_superblock);
 
-	if (!parse_options(data,&uid,&gid,&i,&reserved,&root_block,
-				&blocksize,&sbi->s_prefix,
-				sbi->s_volume, &mount_flags)) {
-		pr_err("Error parsing options\n");
-		return -EINVAL;
-	}
-	/* N.B. after this point s_prefix must be released */
+	sbi->s_flags	= ctx->mount_flags;
+	sbi->s_mode	= ctx->mode;
+	sbi->s_uid	= ctx->uid;
+	sbi->s_gid	= ctx->gid;
+	sbi->s_reserved	= ctx->reserved;
+	sbi->s_prefix	= ctx->prefix;
+	ctx->prefix	= NULL;
+	memcpy(sbi->s_volume, ctx->volume, 32);
 
-	sbi->s_flags   = mount_flags;
-	sbi->s_mode    = i;
-	sbi->s_uid     = uid;
-	sbi->s_gid     = gid;
-	sbi->s_reserved= reserved;
+	/* N.B. after this point s_prefix must be released */
 
 	/* Get the size of the device in 512-byte blocks.
 	 * If we later see that the partition uses bigger
@@ -396,15 +363,16 @@ static int affs_fill_super(struct super_block *sb, void *data, int silent)
 
 	i = bdev_logical_block_size(sb->s_bdev);
 	j = PAGE_SIZE;
+	blocksize = ctx->blocksize;
 	if (blocksize > 0) {
 		i = j = blocksize;
 		size = size / (blocksize / 512);
 	}
 
 	for (blocksize = i; blocksize <= j; blocksize <<= 1, size >>= 1) {
-		sbi->s_root_block = root_block;
-		if (root_block < 0)
-			sbi->s_root_block = (reserved + size - 1) / 2;
+		sbi->s_root_block = ctx->root_block;
+		if (ctx->root_block < 0)
+			sbi->s_root_block = (ctx->reserved + size - 1) / 2;
 		pr_debug("setting blocksize to %d\n", blocksize);
 		affs_set_blocksize(sb, blocksize);
 		sbi->s_partition_size = size;
@@ -424,7 +392,7 @@ static int affs_fill_super(struct super_block *sb, void *data, int silent)
 				"size=%d, reserved=%d\n",
 				sb->s_id,
 				sbi->s_root_block + num_bm,
-				blocksize, size, reserved);
+				ctx->blocksize, size, ctx->reserved);
 			root_bh = affs_bread(sb, sbi->s_root_block + num_bm);
 			if (!root_bh)
 				continue;
@@ -447,7 +415,7 @@ static int affs_fill_super(struct super_block *sb, void *data, int silent)
 got_root:
 	/* Keep super block in cache */
 	sbi->s_root_bh = root_bh;
-	root_block = sbi->s_root_block;
+	ctx->root_block = sbi->s_root_block;
 
 	/* Find out which kind of FS we have */
 	boot_bh = sb_bread(sb, 0);
@@ -506,7 +474,7 @@ static int affs_fill_super(struct super_block *sb, void *data, int silent)
 		return -EINVAL;
 	}
 
-	if (affs_test_opt(mount_flags, SF_VERBOSE)) {
+	if (affs_test_opt(ctx->mount_flags, SF_VERBOSE)) {
 		u8 len = AFFS_ROOT_TAIL(sb, root_bh)->disk_name[0];
 		pr_notice("Mounting volume \"%.*s\": Type=%.3s\\%c, Blocksize=%d\n",
 			len > 31 ? 31 : len,
@@ -528,7 +496,7 @@ static int affs_fill_super(struct super_block *sb, void *data, int silent)
 
 	/* set up enough so that it can read an inode */
 
-	root_inode = affs_iget(sb, root_block);
+	root_inode = affs_iget(sb, ctx->root_block);
 	if (IS_ERR(root_inode))
 		return PTR_ERR(root_inode);
 
@@ -548,56 +516,43 @@ static int affs_fill_super(struct super_block *sb, void *data, int silent)
 	return 0;
 }
 
-static int
-affs_remount(struct super_block *sb, int *flags, char *data)
+static int affs_reconfigure(struct fs_context *fc)
 {
+	struct super_block	*sb = fc->root->d_sb;
+	struct affs_context	*ctx = fc->fs_private;
 	struct affs_sb_info	*sbi = AFFS_SB(sb);
-	int			 blocksize;
-	kuid_t			 uid;
-	kgid_t			 gid;
-	int			 mode;
-	int			 reserved;
-	int			 root_block;
-	unsigned long		 mount_flags;
 	int			 res = 0;
-	char			 volume[32];
-	char			*prefix = NULL;
-
-	pr_debug("%s(flags=0x%x,opts=\"%s\")\n", __func__, *flags, data);
 
 	sync_filesystem(sb);
-	*flags |= SB_NODIRATIME;
-
-	memcpy(volume, sbi->s_volume, 32);
-	if (!parse_options(data, &uid, &gid, &mode, &reserved, &root_block,
-			   &blocksize, &prefix, volume,
-			   &mount_flags)) {
-		kfree(prefix);
-		return -EINVAL;
-	}
+	fc->sb_flags |= SB_NODIRATIME;
 
 	flush_delayed_work(&sbi->sb_work);
 
-	sbi->s_flags = mount_flags;
-	sbi->s_mode  = mode;
-	sbi->s_uid   = uid;
-	sbi->s_gid   = gid;
+	/*
+	 * NB: Historically, only mount_flags, mode, uid, gic, prefix,
+	 * and volume are accepted during remount.
+	 */
+	sbi->s_flags = ctx->mount_flags;
+	sbi->s_mode  = ctx->mode;
+	sbi->s_uid   = ctx->uid;
+	sbi->s_gid   = ctx->gid;
 	/* protect against readers */
 	spin_lock(&sbi->symlink_lock);
-	if (prefix) {
+	if (ctx->prefix) {
 		kfree(sbi->s_prefix);
-		sbi->s_prefix = prefix;
+		sbi->s_prefix = ctx->prefix;
+		ctx->prefix = NULL;
 	}
-	memcpy(sbi->s_volume, volume, 32);
+	memcpy(sbi->s_volume, ctx->volume, 32);
 	spin_unlock(&sbi->symlink_lock);
 
-	if ((bool)(*flags & SB_RDONLY) == sb_rdonly(sb))
+	if ((bool)(fc->sb_flags & SB_RDONLY) == sb_rdonly(sb))
 		return 0;
 
-	if (*flags & SB_RDONLY)
+	if (fc->sb_flags & SB_RDONLY)
 		affs_free_bitmap(sb);
 	else
-		res = affs_init_bitmap(sb, flags);
+		res = affs_init_bitmap(sb, &fc->sb_flags);
 
 	return res;
 }
@@ -624,10 +579,9 @@ affs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	return 0;
 }
 
-static struct dentry *affs_mount(struct file_system_type *fs_type,
-	int flags, const char *dev_name, void *data)
+static int affs_get_tree(struct fs_context *fc)
 {
-	return mount_bdev(fs_type, flags, dev_name, data, affs_fill_super);
+        return get_tree_bdev(fc, affs_fill_super);
 }
 
 static void affs_kill_sb(struct super_block *sb)
@@ -643,12 +597,61 @@ static void affs_kill_sb(struct super_block *sb)
 	}
 }
 
+static void affs_free_fc(struct fs_context *fc)
+{
+	struct affs_context *ctx = fc->fs_private;
+
+	kfree(ctx->prefix);
+	kfree(ctx);
+}
+
+static const struct fs_context_operations affs_context_ops = {
+	.parse_param	= affs_parse_param,
+	.get_tree	= affs_get_tree,
+	.reconfigure	= affs_reconfigure,
+	.free		= affs_free_fc,
+};
+
+static int affs_init_fs_context(struct fs_context *fc)
+{
+	struct affs_context *ctx;
+
+	ctx = kzalloc(sizeof(struct affs_context), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
+		struct super_block *sb = fc->root->d_sb;
+		struct affs_sb_info *sbi = AFFS_SB(sb);
+
+		/*
+		 * NB: historically, no options other than volume were
+		 * preserved across a remount unless they were explicitly
+		 * passed in.
+		 */
+		memcpy(ctx->volume, sbi->s_volume, 32);
+	} else {
+		ctx->uid	= current_uid();
+		ctx->gid	= current_gid();
+		ctx->reserved	= 2;
+		ctx->root_block	= -1;
+		ctx->blocksize	= -1;
+		ctx->volume[0]	= ':';
+	}
+
+        fc->ops = &affs_context_ops;
+        fc->fs_private = ctx;
+
+        return 0;
+}
+
 static struct file_system_type affs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "affs",
-	.mount		= affs_mount,
 	.kill_sb	= affs_kill_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
+	.init_fs_context = affs_init_fs_context,
+	.parameters	= affs_param_spec,
 };
 MODULE_ALIAS_FS("affs");
 
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/5] befs: convert befs to use the new mount api
  2024-09-16 17:26 [PATCH 0/5] adfs, affs, befs, hfs, hfsplus: convert to new mount api Eric Sandeen
  2024-09-16 17:26 ` [PATCH 1/5] adfs: convert adfs to use the " Eric Sandeen
  2024-09-16 17:26 ` [PATCH 2/5] affs: convert affs " Eric Sandeen
@ 2024-09-16 17:26 ` Eric Sandeen
  2024-09-17 15:50   ` Jan Kara
  2024-09-16 17:26 ` [PATCH 4/5] hfs: convert hfs " Eric Sandeen
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Eric Sandeen @ 2024-09-16 17:26 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: brauner, Eric Sandeen, Luis de Bethencourt, Salah Triki

Convert the befs filesystem to use the new mount API.
Tested by comparing random mount & remount options before and after
the change.

Cc: Luis de Bethencourt <luisbg@kernel.org>
Cc: Salah Triki <salah.triki@gmail.com>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/befs/linuxvfs.c | 199 +++++++++++++++++++++++----------------------
 1 file changed, 102 insertions(+), 97 deletions(-)

diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
index f92f108840f5..8f430ff8e445 100644
--- a/fs/befs/linuxvfs.c
+++ b/fs/befs/linuxvfs.c
@@ -11,12 +11,13 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/fs.h>
+#include <linux/fs_context.h>
+#include <linux/fs_parser.h>
 #include <linux/errno.h>
 #include <linux/stat.h>
 #include <linux/nls.h>
 #include <linux/buffer_head.h>
 #include <linux/vfs.h>
-#include <linux/parser.h>
 #include <linux/namei.h>
 #include <linux/sched.h>
 #include <linux/cred.h>
@@ -54,22 +55,20 @@ static int befs_utf2nls(struct super_block *sb, const char *in, int in_len,
 static int befs_nls2utf(struct super_block *sb, const char *in, int in_len,
 			char **out, int *out_len);
 static void befs_put_super(struct super_block *);
-static int befs_remount(struct super_block *, int *, char *);
 static int befs_statfs(struct dentry *, struct kstatfs *);
 static int befs_show_options(struct seq_file *, struct dentry *);
-static int parse_options(char *, struct befs_mount_options *);
 static struct dentry *befs_fh_to_dentry(struct super_block *sb,
 				struct fid *fid, int fh_len, int fh_type);
 static struct dentry *befs_fh_to_parent(struct super_block *sb,
 				struct fid *fid, int fh_len, int fh_type);
 static struct dentry *befs_get_parent(struct dentry *child);
+static void befs_free_fc(struct fs_context *fc);
 
 static const struct super_operations befs_sops = {
 	.alloc_inode	= befs_alloc_inode,	/* allocate a new inode */
 	.free_inode	= befs_free_inode, /* deallocate an inode */
 	.put_super	= befs_put_super,	/* uninit super */
 	.statfs		= befs_statfs,	/* statfs */
-	.remount_fs	= befs_remount,
 	.show_options	= befs_show_options,
 };
 
@@ -672,92 +671,53 @@ static struct dentry *befs_get_parent(struct dentry *child)
 }
 
 enum {
-	Opt_uid, Opt_gid, Opt_charset, Opt_debug, Opt_err,
+	Opt_uid, Opt_gid, Opt_charset, Opt_debug,
 };
 
-static const match_table_t befs_tokens = {
-	{Opt_uid, "uid=%d"},
-	{Opt_gid, "gid=%d"},
-	{Opt_charset, "iocharset=%s"},
-	{Opt_debug, "debug"},
-	{Opt_err, NULL}
+static const struct fs_parameter_spec befs_param_spec[] = {
+	fsparam_uid	("uid",		Opt_uid),
+	fsparam_gid	("gid",		Opt_gid),
+	fsparam_string	("iocharset",	Opt_charset),
+	fsparam_flag	("debug",	Opt_debug),
+	{}
 };
 
 static int
-parse_options(char *options, struct befs_mount_options *opts)
+befs_parse_param(struct fs_context *fc, struct fs_parameter *param)
 {
-	char *p;
-	substring_t args[MAX_OPT_ARGS];
-	int option;
-	kuid_t uid;
-	kgid_t gid;
-
-	/* Initialize options */
-	opts->uid = GLOBAL_ROOT_UID;
-	opts->gid = GLOBAL_ROOT_GID;
-	opts->use_uid = 0;
-	opts->use_gid = 0;
-	opts->iocharset = NULL;
-	opts->debug = 0;
-
-	if (!options)
-		return 1;
-
-	while ((p = strsep(&options, ",")) != NULL) {
-		int token;
-
-		if (!*p)
-			continue;
-
-		token = match_token(p, befs_tokens, args);
-		switch (token) {
-		case Opt_uid:
-			if (match_int(&args[0], &option))
-				return 0;
-			uid = INVALID_UID;
-			if (option >= 0)
-				uid = make_kuid(current_user_ns(), option);
-			if (!uid_valid(uid)) {
-				pr_err("Invalid uid %d, "
-				       "using default\n", option);
-				break;
-			}
-			opts->uid = uid;
-			opts->use_uid = 1;
-			break;
-		case Opt_gid:
-			if (match_int(&args[0], &option))
-				return 0;
-			gid = INVALID_GID;
-			if (option >= 0)
-				gid = make_kgid(current_user_ns(), option);
-			if (!gid_valid(gid)) {
-				pr_err("Invalid gid %d, "
-				       "using default\n", option);
-				break;
-			}
-			opts->gid = gid;
-			opts->use_gid = 1;
-			break;
-		case Opt_charset:
-			kfree(opts->iocharset);
-			opts->iocharset = match_strdup(&args[0]);
-			if (!opts->iocharset) {
-				pr_err("allocation failure for "
-				       "iocharset string\n");
-				return 0;
-			}
-			break;
-		case Opt_debug:
-			opts->debug = 1;
-			break;
-		default:
-			pr_err("Unrecognized mount option \"%s\" "
-			       "or missing value\n", p);
-			return 0;
-		}
+	struct befs_mount_options *opts = fc->fs_private;
+	int token;
+	struct fs_parse_result result;
+
+	/* befs ignores all options on remount */
+	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE)
+		return 0;
+
+	token = fs_parse(fc, befs_param_spec, param, &result);
+	if (token < 0)
+		return token;
+
+	switch (token) {
+	case Opt_uid:
+		opts->uid = result.uid;
+		opts->use_uid = 1;
+		break;
+	case Opt_gid:
+		opts->gid = result.gid;
+		opts->use_gid = 1;
+		break;
+	case Opt_charset:
+		kfree(opts->iocharset);
+		opts->iocharset = param->string;
+		param->string = NULL;
+		break;
+	case Opt_debug:
+		opts->debug = 1;
+		break;
+	default:
+		return -EINVAL;
 	}
-	return 1;
+	return 0;
 }
 
 static int befs_show_options(struct seq_file *m, struct dentry *root)
@@ -793,6 +753,21 @@ befs_put_super(struct super_block *sb)
 	sb->s_fs_info = NULL;
 }
 
+/*
+ * Copy the parsed options into the sbi mount_options member
+ */
+static void
+befs_set_options(struct befs_sb_info *sbi, struct befs_mount_options *opts)
+{
+	sbi->mount_opts.uid = opts->uid;
+	sbi->mount_opts.gid = opts->gid;
+	sbi->mount_opts.use_uid = opts->use_uid;
+	sbi->mount_opts.use_gid = opts->use_gid;
+	sbi->mount_opts.debug = opts->debug;
+	sbi->mount_opts.iocharset = opts->iocharset;
+	opts->iocharset = NULL;
+}
+
 /* Allocate private field of the superblock, fill it.
  *
  * Finish filling the public superblock fields
@@ -800,7 +775,7 @@ befs_put_super(struct super_block *sb)
  * Load a set of NLS translations if needed.
  */
 static int
-befs_fill_super(struct super_block *sb, void *data, int silent)
+befs_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	struct buffer_head *bh;
 	struct befs_sb_info *befs_sb;
@@ -810,6 +785,8 @@ befs_fill_super(struct super_block *sb, void *data, int silent)
 	const unsigned long sb_block = 0;
 	const off_t x86_sb_off = 512;
 	int blocksize;
+	struct befs_mount_options *parsed_opts = fc->fs_private;
+	int silent = fc->sb_flags & SB_SILENT;
 
 	sb->s_fs_info = kzalloc(sizeof(*befs_sb), GFP_KERNEL);
 	if (sb->s_fs_info == NULL)
@@ -817,11 +794,7 @@ befs_fill_super(struct super_block *sb, void *data, int silent)
 
 	befs_sb = BEFS_SB(sb);
 
-	if (!parse_options((char *) data, &befs_sb->mount_opts)) {
-		if (!silent)
-			befs_error(sb, "cannot parse mount options");
-		goto unacquire_priv_sbp;
-	}
+	befs_set_options(befs_sb, parsed_opts);
 
 	befs_debug(sb, "---> %s", __func__);
 
@@ -934,10 +907,10 @@ befs_fill_super(struct super_block *sb, void *data, int silent)
 }
 
 static int
-befs_remount(struct super_block *sb, int *flags, char *data)
+befs_reconfigure(struct fs_context *fc)
 {
-	sync_filesystem(sb);
-	if (!(*flags & SB_RDONLY))
+	sync_filesystem(fc->root->d_sb);
+	if (!(fc->sb_flags & SB_RDONLY))
 		return -EINVAL;
 	return 0;
 }
@@ -965,19 +938,51 @@ befs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	return 0;
 }
 
-static struct dentry *
-befs_mount(struct file_system_type *fs_type, int flags, const char *dev_name,
-	    void *data)
+static int befs_get_tree(struct fs_context *fc)
+{
+	return get_tree_bdev(fc, befs_fill_super);
+}
+
+static const struct fs_context_operations befs_context_ops = {
+	.parse_param	= befs_parse_param,
+	.get_tree	= befs_get_tree,
+	.reconfigure	= befs_reconfigure,
+	.free		= befs_free_fc,
+};
+
+static int befs_init_fs_context(struct fs_context *fc)
+{
+	struct befs_mount_options *opts;
+
+	opts = kzalloc(sizeof(*opts), GFP_KERNEL);
+	if (!opts)
+		return -ENOMEM;
+
+	/* Initialize options */
+	opts->uid = GLOBAL_ROOT_UID;
+	opts->gid = GLOBAL_ROOT_GID;
+
+	fc->fs_private = opts;
+	fc->ops = &befs_context_ops;
+
+	return 0;
+}
+
+static void befs_free_fc(struct fs_context *fc)
 {
-	return mount_bdev(fs_type, flags, dev_name, data, befs_fill_super);
+	struct befs_mount_options *opts = fc->fs_private;
+
+	kfree(opts->iocharset);
+	kfree(fc->fs_private);
 }
 
 static struct file_system_type befs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "befs",
-	.mount		= befs_mount,
 	.kill_sb	= kill_block_super,
 	.fs_flags	= FS_REQUIRES_DEV,
+	.init_fs_context = befs_init_fs_context,
+	.parameters	= befs_param_spec,
 };
 MODULE_ALIAS_FS("befs");
 
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/5] hfs: convert hfs to use the new mount api
  2024-09-16 17:26 [PATCH 0/5] adfs, affs, befs, hfs, hfsplus: convert to new mount api Eric Sandeen
                   ` (2 preceding siblings ...)
  2024-09-16 17:26 ` [PATCH 3/5] befs: convert befs " Eric Sandeen
@ 2024-09-16 17:26 ` Eric Sandeen
  2024-09-17 15:55   ` Jan Kara
  2024-09-16 17:26 ` [PATCH 5/5] hfsplus: convert hfsplus " Eric Sandeen
  2024-09-18  9:46 ` [PATCH 0/5] adfs, affs, befs, hfs, hfsplus: convert to " Christian Brauner
  5 siblings, 1 reply; 14+ messages in thread
From: Eric Sandeen @ 2024-09-16 17:26 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: brauner, Eric Sandeen

Convert the hfs filesystem to use the new mount API.
Tested by comparing random mount & remount options before and after
the change.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/hfs/super.c | 341 ++++++++++++++++++++++---------------------------
 1 file changed, 154 insertions(+), 187 deletions(-)

diff --git a/fs/hfs/super.c b/fs/hfs/super.c
index eeac99765f0d..ee314f3e39f8 100644
--- a/fs/hfs/super.c
+++ b/fs/hfs/super.c
@@ -15,10 +15,11 @@
 #include <linux/module.h>
 #include <linux/blkdev.h>
 #include <linux/backing-dev.h>
+#include <linux/fs_context.h>
+#include <linux/fs_parser.h>
 #include <linux/mount.h>
 #include <linux/init.h>
 #include <linux/nls.h>
-#include <linux/parser.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
 #include <linux/vfs.h>
@@ -111,21 +112,24 @@ static int hfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	return 0;
 }
 
-static int hfs_remount(struct super_block *sb, int *flags, char *data)
+static int hfs_reconfigure(struct fs_context *fc)
 {
+	struct super_block *sb = fc->root->d_sb;
+
 	sync_filesystem(sb);
-	*flags |= SB_NODIRATIME;
-	if ((bool)(*flags & SB_RDONLY) == sb_rdonly(sb))
+	fc->sb_flags |= SB_NODIRATIME;
+	if ((bool)(fc->sb_flags & SB_RDONLY) == sb_rdonly(sb))
 		return 0;
-	if (!(*flags & SB_RDONLY)) {
+
+	if (!(fc->sb_flags & SB_RDONLY)) {
 		if (!(HFS_SB(sb)->mdb->drAtrb & cpu_to_be16(HFS_SB_ATTRIB_UNMNT))) {
 			pr_warn("filesystem was not cleanly unmounted, running fsck.hfs is recommended.  leaving read-only.\n");
 			sb->s_flags |= SB_RDONLY;
-			*flags |= SB_RDONLY;
+			fc->sb_flags |= SB_RDONLY;
 		} else if (HFS_SB(sb)->mdb->drAtrb & cpu_to_be16(HFS_SB_ATTRIB_SLOCK)) {
 			pr_warn("filesystem is marked locked, leaving read-only.\n");
 			sb->s_flags |= SB_RDONLY;
-			*flags |= SB_RDONLY;
+			fc->sb_flags |= SB_RDONLY;
 		}
 	}
 	return 0;
@@ -180,7 +184,6 @@ static const struct super_operations hfs_super_operations = {
 	.put_super	= hfs_put_super,
 	.sync_fs	= hfs_sync_fs,
 	.statfs		= hfs_statfs,
-	.remount_fs     = hfs_remount,
 	.show_options	= hfs_show_options,
 };
 
@@ -188,181 +191,112 @@ enum {
 	opt_uid, opt_gid, opt_umask, opt_file_umask, opt_dir_umask,
 	opt_part, opt_session, opt_type, opt_creator, opt_quiet,
 	opt_codepage, opt_iocharset,
-	opt_err
 };
 
-static const match_table_t tokens = {
-	{ opt_uid, "uid=%u" },
-	{ opt_gid, "gid=%u" },
-	{ opt_umask, "umask=%o" },
-	{ opt_file_umask, "file_umask=%o" },
-	{ opt_dir_umask, "dir_umask=%o" },
-	{ opt_part, "part=%u" },
-	{ opt_session, "session=%u" },
-	{ opt_type, "type=%s" },
-	{ opt_creator, "creator=%s" },
-	{ opt_quiet, "quiet" },
-	{ opt_codepage, "codepage=%s" },
-	{ opt_iocharset, "iocharset=%s" },
-	{ opt_err, NULL }
+static const struct fs_parameter_spec hfs_param_spec[] = {
+	fsparam_u32	("uid",		opt_uid),
+	fsparam_u32	("gid",		opt_gid),
+	fsparam_u32oct	("umask",	opt_umask),
+	fsparam_u32oct	("file_umask",	opt_file_umask),
+	fsparam_u32oct	("dir_umask",	opt_dir_umask),
+	fsparam_u32	("part",	opt_part),
+	fsparam_u32	("session",	opt_session),
+	fsparam_string	("type",	opt_type),
+	fsparam_string	("creator",	opt_creator),
+	fsparam_flag	("quiet",	opt_quiet),
+	fsparam_string	("codepage",	opt_codepage),
+	fsparam_string	("iocharset",	opt_iocharset),
+	{}
 };
 
-static inline int match_fourchar(substring_t *arg, u32 *result)
-{
-	if (arg->to - arg->from != 4)
-		return -EINVAL;
-	memcpy(result, arg->from, 4);
-	return 0;
-}
-
 /*
- * parse_options()
+ * hfs_parse_param()
  *
- * adapted from linux/fs/msdos/inode.c written 1992,93 by Werner Almesberger
- * This function is called by hfs_read_super() to parse the mount options.
+ * This function is called by the vfs to parse the mount options.
  */
-static int parse_options(char *options, struct hfs_sb_info *hsb)
+static int hfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
 {
-	char *p;
-	substring_t args[MAX_OPT_ARGS];
-	int tmp, token;
-
-	/* initialize the sb with defaults */
-	hsb->s_uid = current_uid();
-	hsb->s_gid = current_gid();
-	hsb->s_file_umask = 0133;
-	hsb->s_dir_umask = 0022;
-	hsb->s_type = hsb->s_creator = cpu_to_be32(0x3f3f3f3f);	/* == '????' */
-	hsb->s_quiet = 0;
-	hsb->part = -1;
-	hsb->session = -1;
-
-	if (!options)
-		return 1;
-
-	while ((p = strsep(&options, ",")) != NULL) {
-		if (!*p)
-			continue;
-
-		token = match_token(p, tokens, args);
-		switch (token) {
-		case opt_uid:
-			if (match_int(&args[0], &tmp)) {
-				pr_err("uid requires an argument\n");
-				return 0;
-			}
-			hsb->s_uid = make_kuid(current_user_ns(), (uid_t)tmp);
-			if (!uid_valid(hsb->s_uid)) {
-				pr_err("invalid uid %d\n", tmp);
-				return 0;
-			}
-			break;
-		case opt_gid:
-			if (match_int(&args[0], &tmp)) {
-				pr_err("gid requires an argument\n");
-				return 0;
-			}
-			hsb->s_gid = make_kgid(current_user_ns(), (gid_t)tmp);
-			if (!gid_valid(hsb->s_gid)) {
-				pr_err("invalid gid %d\n", tmp);
-				return 0;
-			}
-			break;
-		case opt_umask:
-			if (match_octal(&args[0], &tmp)) {
-				pr_err("umask requires a value\n");
-				return 0;
-			}
-			hsb->s_file_umask = (umode_t)tmp;
-			hsb->s_dir_umask = (umode_t)tmp;
-			break;
-		case opt_file_umask:
-			if (match_octal(&args[0], &tmp)) {
-				pr_err("file_umask requires a value\n");
-				return 0;
-			}
-			hsb->s_file_umask = (umode_t)tmp;
-			break;
-		case opt_dir_umask:
-			if (match_octal(&args[0], &tmp)) {
-				pr_err("dir_umask requires a value\n");
-				return 0;
-			}
-			hsb->s_dir_umask = (umode_t)tmp;
-			break;
-		case opt_part:
-			if (match_int(&args[0], &hsb->part)) {
-				pr_err("part requires an argument\n");
-				return 0;
-			}
-			break;
-		case opt_session:
-			if (match_int(&args[0], &hsb->session)) {
-				pr_err("session requires an argument\n");
-				return 0;
-			}
-			break;
-		case opt_type:
-			if (match_fourchar(&args[0], &hsb->s_type)) {
-				pr_err("type requires a 4 character value\n");
-				return 0;
-			}
-			break;
-		case opt_creator:
-			if (match_fourchar(&args[0], &hsb->s_creator)) {
-				pr_err("creator requires a 4 character value\n");
-				return 0;
-			}
-			break;
-		case opt_quiet:
-			hsb->s_quiet = 1;
-			break;
-		case opt_codepage:
-			if (hsb->nls_disk) {
-				pr_err("unable to change codepage\n");
-				return 0;
-			}
-			p = match_strdup(&args[0]);
-			if (p)
-				hsb->nls_disk = load_nls(p);
-			if (!hsb->nls_disk) {
-				pr_err("unable to load codepage \"%s\"\n", p);
-				kfree(p);
-				return 0;
-			}
-			kfree(p);
-			break;
-		case opt_iocharset:
-			if (hsb->nls_io) {
-				pr_err("unable to change iocharset\n");
-				return 0;
-			}
-			p = match_strdup(&args[0]);
-			if (p)
-				hsb->nls_io = load_nls(p);
-			if (!hsb->nls_io) {
-				pr_err("unable to load iocharset \"%s\"\n", p);
-				kfree(p);
-				return 0;
-			}
-			kfree(p);
-			break;
-		default:
-			return 0;
-		}
-	}
+	struct hfs_sb_info *hsb = fc->s_fs_info;
+	struct fs_parse_result result;
+	int opt;
+
+	/* hfs does not honor any fs-specific options on remount */
+	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE)
+		return 0;
 
-	if (hsb->nls_disk && !hsb->nls_io) {
-		hsb->nls_io = load_nls_default();
+	opt = fs_parse(fc, hfs_param_spec, param, &result);
+	if (opt < 0)
+		return opt;
+
+	switch (opt) {
+	case opt_uid:
+		hsb->s_uid = result.uid;
+		break;
+	case opt_gid:
+		hsb->s_gid = result.gid;
+		break;
+	case opt_umask:
+		hsb->s_file_umask = (umode_t)result.uint_32;
+		hsb->s_dir_umask = (umode_t)result.uint_32;
+		break;
+	case opt_file_umask:
+		hsb->s_file_umask = (umode_t)result.uint_32;
+		break;
+	case opt_dir_umask:
+		hsb->s_dir_umask = (umode_t)result.uint_32;
+		break;
+	case opt_part:
+		hsb->part = result.uint_32;
+		break;
+	case opt_session:
+		hsb->session = result.uint_32;
+		break;
+	case opt_type:
+		if (strlen(param->string) != 4) {
+			pr_err("type requires a 4 character value\n");
+			return -EINVAL;
+		}
+		memcpy(&hsb->s_type, param->string, 4);
+		break;
+	case opt_creator:
+		if (strlen(param->string) != 4) {
+			pr_err("creator requires a 4 character value\n");
+			return -EINVAL;
+		}
+		memcpy(&hsb->s_creator, param->string, 4);
+		break;
+	case opt_quiet:
+		hsb->s_quiet = 1;
+		break;
+	case opt_codepage:
+		if (hsb->nls_disk) {
+			pr_err("unable to change codepage\n");
+			return -EINVAL;
+		}
+		hsb->nls_disk = load_nls(param->string);
+		if (!hsb->nls_disk) {
+			pr_err("unable to load codepage \"%s\"\n",
+					param->string);
+			return -EINVAL;
+		}
+		break;
+	case opt_iocharset:
+		if (hsb->nls_io) {
+			pr_err("unable to change iocharset\n");
+			return -EINVAL;
+		}
+		hsb->nls_io = load_nls(param->string);
 		if (!hsb->nls_io) {
-			pr_err("unable to load default iocharset\n");
-			return 0;
+			pr_err("unable to load iocharset \"%s\"\n",
+					param->string);
+			return -EINVAL;
 		}
+		break;
+	default:
+		return -EINVAL;
 	}
-	hsb->s_dir_umask &= 0777;
-	hsb->s_file_umask &= 0577;
 
-	return 1;
+	return 0;
 }
 
 /*
@@ -376,29 +310,24 @@ static int parse_options(char *options, struct hfs_sb_info *hsb)
  * hfs_btree_init() to get the necessary data about the extents and
  * catalog B-trees and, finally, reading the root inode into memory.
  */
-static int hfs_fill_super(struct super_block *sb, void *data, int silent)
+static int hfs_fill_super(struct super_block *sb, struct fs_context *fc)
 {
-	struct hfs_sb_info *sbi;
+	struct hfs_sb_info *sbi = HFS_SB(sb);
 	struct hfs_find_data fd;
 	hfs_cat_rec rec;
 	struct inode *root_inode;
+	int silent = fc->sb_flags & SB_SILENT;
 	int res;
 
-	sbi = kzalloc(sizeof(struct hfs_sb_info), GFP_KERNEL);
-	if (!sbi)
-		return -ENOMEM;
+	/* load_nls_default does not fail */
+	if (sbi->nls_disk && !sbi->nls_io)
+		sbi->nls_io = load_nls_default();
+	sbi->s_dir_umask &= 0777;
+	sbi->s_file_umask &= 0577;
 
-	sbi->sb = sb;
-	sb->s_fs_info = sbi;
 	spin_lock_init(&sbi->work_lock);
 	INIT_DELAYED_WORK(&sbi->mdb_work, flush_mdb);
 
-	res = -EINVAL;
-	if (!parse_options((char *)data, sbi)) {
-		pr_err("unable to parse mount options\n");
-		goto bail;
-	}
-
 	sb->s_op = &hfs_super_operations;
 	sb->s_xattr = hfs_xattr_handlers;
 	sb->s_flags |= SB_NODIRATIME;
@@ -451,18 +380,56 @@ static int hfs_fill_super(struct super_block *sb, void *data, int silent)
 	return res;
 }
 
-static struct dentry *hfs_mount(struct file_system_type *fs_type,
-		      int flags, const char *dev_name, void *data)
+static int hfs_get_tree(struct fs_context *fc)
 {
-	return mount_bdev(fs_type, flags, dev_name, data, hfs_fill_super);
+	return get_tree_bdev(fc, hfs_fill_super);
+}
+
+static void hfs_free_fc(struct fs_context *fc)
+{
+	kfree(fc->s_fs_info);
+}
+
+static const struct fs_context_operations hfs_context_ops = {
+	.parse_param	= hfs_parse_param,
+	.get_tree	= hfs_get_tree,
+	.reconfigure	= hfs_reconfigure,
+	.free		= hfs_free_fc,
+};
+
+static int hfs_init_fs_context(struct fs_context *fc)
+{
+	struct hfs_sb_info *hsb;
+
+	hsb = kzalloc(sizeof(struct hfs_sb_info), GFP_KERNEL);
+	if (!hsb)
+		return -ENOMEM;
+
+	fc->s_fs_info = hsb;
+	fc->ops = &hfs_context_ops;
+
+	if (fc->purpose != FS_CONTEXT_FOR_RECONFIGURE) {
+		/* initialize options with defaults */
+		hsb->s_uid = current_uid();
+		hsb->s_gid = current_gid();
+		hsb->s_file_umask = 0133;
+		hsb->s_dir_umask = 0022;
+		hsb->s_type = cpu_to_be32(0x3f3f3f3f); /* == '????' */
+		hsb->s_creator = cpu_to_be32(0x3f3f3f3f); /* == '????' */
+		hsb->s_quiet = 0;
+		hsb->part = -1;
+		hsb->session = -1;
+	}
+
+	return 0;
 }
 
 static struct file_system_type hfs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "hfs",
-	.mount		= hfs_mount,
 	.kill_sb	= kill_block_super,
 	.fs_flags	= FS_REQUIRES_DEV,
+	.init_fs_context = hfs_init_fs_context,
 };
 MODULE_ALIAS_FS("hfs");
 
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/5] hfsplus: convert hfsplus to use the new mount api
  2024-09-16 17:26 [PATCH 0/5] adfs, affs, befs, hfs, hfsplus: convert to new mount api Eric Sandeen
                   ` (3 preceding siblings ...)
  2024-09-16 17:26 ` [PATCH 4/5] hfs: convert hfs " Eric Sandeen
@ 2024-09-16 17:26 ` Eric Sandeen
  2024-09-17 16:11   ` Jan Kara
  2024-09-18  9:46 ` [PATCH 0/5] adfs, affs, befs, hfs, hfsplus: convert to " Christian Brauner
  5 siblings, 1 reply; 14+ messages in thread
From: Eric Sandeen @ 2024-09-16 17:26 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: brauner, Eric Sandeen

Convert the hfsplus filesystem to use the new mount API.
Tested by comparing random mount & remount options before and after
the change.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/hfsplus/hfsplus_fs.h |   4 +-
 fs/hfsplus/options.c    | 263 ++++++++++++++--------------------------
 fs/hfsplus/super.c      |  84 ++++++++-----
 3 files changed, 149 insertions(+), 202 deletions(-)

diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index 9e78f181c24f..53cb3c7dfc73 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -21,6 +21,7 @@
 #include <linux/mutex.h>
 #include <linux/buffer_head.h>
 #include <linux/blkdev.h>
+#include <linux/fs_context.h>
 #include "hfsplus_raw.h"
 
 #define DBG_BNODE_REFS	0x00000001
@@ -496,8 +497,7 @@ long hfsplus_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
 
 /* options.c */
 void hfsplus_fill_defaults(struct hfsplus_sb_info *opts);
-int hfsplus_parse_options_remount(char *input, int *force);
-int hfsplus_parse_options(char *input, struct hfsplus_sb_info *sbi);
+int hfsplus_parse_param(struct fs_context *fc, struct fs_parameter *param);
 int hfsplus_show_options(struct seq_file *seq, struct dentry *root);
 
 /* part_tbl.c */
diff --git a/fs/hfsplus/options.c b/fs/hfsplus/options.c
index c94a58762ad6..a66a09a56bf7 100644
--- a/fs/hfsplus/options.c
+++ b/fs/hfsplus/options.c
@@ -12,7 +12,8 @@
 #include <linux/string.h>
 #include <linux/kernel.h>
 #include <linux/sched.h>
-#include <linux/parser.h>
+#include <linux/fs_context.h>
+#include <linux/fs_parser.h>
 #include <linux/nls.h>
 #include <linux/mount.h>
 #include <linux/seq_file.h>
@@ -23,26 +24,23 @@ enum {
 	opt_creator, opt_type,
 	opt_umask, opt_uid, opt_gid,
 	opt_part, opt_session, opt_nls,
-	opt_nodecompose, opt_decompose,
-	opt_barrier, opt_nobarrier,
-	opt_force, opt_err
+	opt_decompose, opt_barrier,
+	opt_force,
 };
 
-static const match_table_t tokens = {
-	{ opt_creator, "creator=%s" },
-	{ opt_type, "type=%s" },
-	{ opt_umask, "umask=%o" },
-	{ opt_uid, "uid=%u" },
-	{ opt_gid, "gid=%u" },
-	{ opt_part, "part=%u" },
-	{ opt_session, "session=%u" },
-	{ opt_nls, "nls=%s" },
-	{ opt_decompose, "decompose" },
-	{ opt_nodecompose, "nodecompose" },
-	{ opt_barrier, "barrier" },
-	{ opt_nobarrier, "nobarrier" },
-	{ opt_force, "force" },
-	{ opt_err, NULL }
+static const struct fs_parameter_spec hfs_param_spec[] = {
+	fsparam_string	("creator",	opt_creator),
+	fsparam_string	("type",	opt_type),
+	fsparam_u32oct	("umask",	opt_umask),
+	fsparam_u32	("uid",		opt_uid),
+	fsparam_u32	("gid",		opt_gid),
+	fsparam_u32	("part",	opt_part),
+	fsparam_u32	("session",	opt_session),
+	fsparam_string	("nls",		opt_nls),
+	fsparam_flag_no	("decompose",	opt_decompose),
+	fsparam_flag_no	("barrier",	opt_barrier),
+	fsparam_flag	("force",	opt_force),
+	{}
 };
 
 /* Initialize an options object to reasonable defaults */
@@ -60,162 +58,89 @@ void hfsplus_fill_defaults(struct hfsplus_sb_info *opts)
 	opts->session = -1;
 }
 
-/* convert a "four byte character" to a 32 bit int with error checks */
-static inline int match_fourchar(substring_t *arg, u32 *result)
+/* Parse options from mount. Returns nonzero errno on failure */
+int hfsplus_parse_param(struct fs_context *fc, struct fs_parameter *param)
 {
-	if (arg->to - arg->from != 4)
-		return -EINVAL;
-	memcpy(result, arg->from, 4);
-	return 0;
-}
-
-int hfsplus_parse_options_remount(char *input, int *force)
-{
-	char *p;
-	substring_t args[MAX_OPT_ARGS];
-	int token;
-
-	if (!input)
-		return 1;
-
-	while ((p = strsep(&input, ",")) != NULL) {
-		if (!*p)
-			continue;
-
-		token = match_token(p, tokens, args);
-		switch (token) {
-		case opt_force:
-			*force = 1;
-			break;
-		default:
-			break;
+	struct hfsplus_sb_info *sbi = fc->s_fs_info;
+	struct fs_parse_result result;
+	int opt;
+
+	/*
+	 * Only the force option is examined during remount, all others
+	 * are ignored.
+	 */
+	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE &&
+	    strncmp(param->key, "force", 5))
+		return 0;
+
+	opt = fs_parse(fc, hfs_param_spec, param, &result);
+	if (opt < 0)
+		return opt;
+
+	switch (opt) {
+	case opt_creator:
+		if (strlen(param->string) != 4) {
+			pr_err("creator requires a 4 character value\n");
+			return -EINVAL;
 		}
-	}
-
-	return 1;
-}
-
-/* Parse options from mount. Returns 0 on failure */
-/* input is the options passed to mount() as a string */
-int hfsplus_parse_options(char *input, struct hfsplus_sb_info *sbi)
-{
-	char *p;
-	substring_t args[MAX_OPT_ARGS];
-	int tmp, token;
-
-	if (!input)
-		goto done;
-
-	while ((p = strsep(&input, ",")) != NULL) {
-		if (!*p)
-			continue;
-
-		token = match_token(p, tokens, args);
-		switch (token) {
-		case opt_creator:
-			if (match_fourchar(&args[0], &sbi->creator)) {
-				pr_err("creator requires a 4 character value\n");
-				return 0;
-			}
-			break;
-		case opt_type:
-			if (match_fourchar(&args[0], &sbi->type)) {
-				pr_err("type requires a 4 character value\n");
-				return 0;
-			}
-			break;
-		case opt_umask:
-			if (match_octal(&args[0], &tmp)) {
-				pr_err("umask requires a value\n");
-				return 0;
-			}
-			sbi->umask = (umode_t)tmp;
-			break;
-		case opt_uid:
-			if (match_int(&args[0], &tmp)) {
-				pr_err("uid requires an argument\n");
-				return 0;
-			}
-			sbi->uid = make_kuid(current_user_ns(), (uid_t)tmp);
-			if (!uid_valid(sbi->uid)) {
-				pr_err("invalid uid specified\n");
-				return 0;
-			} else {
-				set_bit(HFSPLUS_SB_UID, &sbi->flags);
-			}
-			break;
-		case opt_gid:
-			if (match_int(&args[0], &tmp)) {
-				pr_err("gid requires an argument\n");
-				return 0;
-			}
-			sbi->gid = make_kgid(current_user_ns(), (gid_t)tmp);
-			if (!gid_valid(sbi->gid)) {
-				pr_err("invalid gid specified\n");
-				return 0;
-			} else {
-				set_bit(HFSPLUS_SB_GID, &sbi->flags);
-			}
-			break;
-		case opt_part:
-			if (match_int(&args[0], &sbi->part)) {
-				pr_err("part requires an argument\n");
-				return 0;
-			}
-			break;
-		case opt_session:
-			if (match_int(&args[0], &sbi->session)) {
-				pr_err("session requires an argument\n");
-				return 0;
-			}
-			break;
-		case opt_nls:
-			if (sbi->nls) {
-				pr_err("unable to change nls mapping\n");
-				return 0;
-			}
-			p = match_strdup(&args[0]);
-			if (p)
-				sbi->nls = load_nls(p);
-			if (!sbi->nls) {
-				pr_err("unable to load nls mapping \"%s\"\n",
-				       p);
-				kfree(p);
-				return 0;
-			}
-			kfree(p);
-			break;
-		case opt_decompose:
-			clear_bit(HFSPLUS_SB_NODECOMPOSE, &sbi->flags);
-			break;
-		case opt_nodecompose:
+		memcpy(&sbi->creator, param->string, 4);
+		break;
+	case opt_type:
+		if (strlen(param->string) != 4) {
+			pr_err("type requires a 4 character value\n");
+			return -EINVAL;
+		}
+		memcpy(&sbi->type, param->string, 4);
+		break;
+	case opt_umask:
+		sbi->umask = (umode_t)result.uint_32;
+		break;
+	case opt_uid:
+		sbi->uid = result.uid;
+		set_bit(HFSPLUS_SB_UID, &sbi->flags);
+		break;
+	case opt_gid:
+		sbi->gid = result.gid;
+		set_bit(HFSPLUS_SB_GID, &sbi->flags);
+		break;
+	case opt_part:
+		sbi->part = result.uint_32;
+		break;
+	case opt_session:
+		sbi->session = result.uint_32;
+		break;
+	case opt_nls:
+		if (sbi->nls) {
+			pr_err("unable to change nls mapping\n");
+			return -EINVAL;
+		}
+		sbi->nls = load_nls(param->string);
+		if (!sbi->nls) {
+			pr_err("unable to load nls mapping \"%s\"\n",
+			       param->string);
+			return -EINVAL;
+		}
+		break;
+	case opt_decompose:
+		if (result.negated)
 			set_bit(HFSPLUS_SB_NODECOMPOSE, &sbi->flags);
-			break;
-		case opt_barrier:
-			clear_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags);
-			break;
-		case opt_nobarrier:
+		else
+			clear_bit(HFSPLUS_SB_NODECOMPOSE, &sbi->flags);
+		break;
+	case opt_barrier:
+		if (result.negated)
 			set_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags);
-			break;
-		case opt_force:
-			set_bit(HFSPLUS_SB_FORCE, &sbi->flags);
-			break;
-		default:
-			return 0;
-		}
-	}
-
-done:
-	if (!sbi->nls) {
-		/* try utf8 first, as this is the old default behaviour */
-		sbi->nls = load_nls("utf8");
-		if (!sbi->nls)
-			sbi->nls = load_nls_default();
-		if (!sbi->nls)
-			return 0;
+		else
+			clear_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags);
+		break;
+	case opt_force:
+		set_bit(HFSPLUS_SB_FORCE, &sbi->flags);
+		break;
+	default:
+		return -EINVAL;
 	}
 
-	return 1;
+	return 0;
 }
 
 int hfsplus_show_options(struct seq_file *seq, struct dentry *root)
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index 8f986f30a1ce..261af016efd3 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -14,6 +14,7 @@
 #include <linux/blkdev.h>
 #include <linux/backing-dev.h>
 #include <linux/fs.h>
+#include <linux/fs_context.h>
 #include <linux/slab.h>
 #include <linux/vfs.h>
 #include <linux/nls.h>
@@ -332,34 +333,33 @@ static int hfsplus_statfs(struct dentry *dentry, struct kstatfs *buf)
 	return 0;
 }
 
-static int hfsplus_remount(struct super_block *sb, int *flags, char *data)
+static int hfsplus_reconfigure(struct fs_context *fc)
 {
+	struct super_block *sb = fc->root->d_sb;
+
 	sync_filesystem(sb);
-	if ((bool)(*flags & SB_RDONLY) == sb_rdonly(sb))
+	if ((bool)(fc->sb_flags & SB_RDONLY) == sb_rdonly(sb))
 		return 0;
-	if (!(*flags & SB_RDONLY)) {
-		struct hfsplus_vh *vhdr = HFSPLUS_SB(sb)->s_vhdr;
-		int force = 0;
-
-		if (!hfsplus_parse_options_remount(data, &force))
-			return -EINVAL;
+	if (!(fc->sb_flags & SB_RDONLY)) {
+		struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
+		struct hfsplus_vh *vhdr = sbi->s_vhdr;
 
 		if (!(vhdr->attributes & cpu_to_be32(HFSPLUS_VOL_UNMNT))) {
 			pr_warn("filesystem was not cleanly unmounted, running fsck.hfsplus is recommended.  leaving read-only.\n");
 			sb->s_flags |= SB_RDONLY;
-			*flags |= SB_RDONLY;
-		} else if (force) {
+			fc->sb_flags |= SB_RDONLY;
+		} else if (test_bit(HFSPLUS_SB_FORCE, &sbi->flags)) {
 			/* nothing */
 		} else if (vhdr->attributes &
 				cpu_to_be32(HFSPLUS_VOL_SOFTLOCK)) {
 			pr_warn("filesystem is marked locked, leaving read-only.\n");
 			sb->s_flags |= SB_RDONLY;
-			*flags |= SB_RDONLY;
+			fc->sb_flags |= SB_RDONLY;
 		} else if (vhdr->attributes &
 				cpu_to_be32(HFSPLUS_VOL_JOURNALED)) {
 			pr_warn("filesystem is marked journaled, leaving read-only.\n");
 			sb->s_flags |= SB_RDONLY;
-			*flags |= SB_RDONLY;
+			fc->sb_flags |= SB_RDONLY;
 		}
 	}
 	return 0;
@@ -373,38 +373,33 @@ static const struct super_operations hfsplus_sops = {
 	.put_super	= hfsplus_put_super,
 	.sync_fs	= hfsplus_sync_fs,
 	.statfs		= hfsplus_statfs,
-	.remount_fs	= hfsplus_remount,
 	.show_options	= hfsplus_show_options,
 };
 
-static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
+static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	struct hfsplus_vh *vhdr;
-	struct hfsplus_sb_info *sbi;
+	struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
 	hfsplus_cat_entry entry;
 	struct hfs_find_data fd;
 	struct inode *root, *inode;
 	struct qstr str;
-	struct nls_table *nls = NULL;
+	struct nls_table *nls;
 	u64 last_fs_block, last_fs_page;
+	int silent = fc->sb_flags & SB_SILENT;
 	int err;
 
-	err = -ENOMEM;
-	sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
-	if (!sbi)
-		goto out;
-
-	sb->s_fs_info = sbi;
 	mutex_init(&sbi->alloc_mutex);
 	mutex_init(&sbi->vh_mutex);
 	spin_lock_init(&sbi->work_lock);
 	INIT_DELAYED_WORK(&sbi->sync_work, delayed_sync_fs);
-	hfsplus_fill_defaults(sbi);
 
 	err = -EINVAL;
-	if (!hfsplus_parse_options(data, sbi)) {
-		pr_err("unable to parse mount options\n");
-		goto out_unload_nls;
+	if (!sbi->nls) {
+		/* try utf8 first, as this is the old default behaviour */
+		sbi->nls = load_nls("utf8");
+		if (!sbi->nls)
+			sbi->nls = load_nls_default();
 	}
 
 	/* temporarily use utf8 to correctly find the hidden dir below */
@@ -616,7 +611,6 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
 	unload_nls(sbi->nls);
 	unload_nls(nls);
 	kfree(sbi);
-out:
 	return err;
 }
 
@@ -641,18 +635,46 @@ static void hfsplus_free_inode(struct inode *inode)
 
 #define HFSPLUS_INODE_SIZE	sizeof(struct hfsplus_inode_info)
 
-static struct dentry *hfsplus_mount(struct file_system_type *fs_type,
-			  int flags, const char *dev_name, void *data)
+static int hfsplus_get_tree(struct fs_context *fc)
+{
+	return get_tree_bdev(fc, hfsplus_fill_super);
+}
+
+static void hfsplus_free_fc(struct fs_context *fc)
 {
-	return mount_bdev(fs_type, flags, dev_name, data, hfsplus_fill_super);
+	kfree(fc->s_fs_info);
+}
+
+static const struct fs_context_operations hfsplus_context_ops = {
+	.parse_param	= hfsplus_parse_param,
+	.get_tree	= hfsplus_get_tree,
+	.reconfigure	= hfsplus_reconfigure,
+	.free		= hfsplus_free_fc,
+};
+
+static int hfsplus_init_fs_context(struct fs_context *fc)
+{
+	struct hfsplus_sb_info *sbi;
+
+	sbi = kzalloc(sizeof(struct hfsplus_sb_info), GFP_KERNEL);
+	if (!sbi)
+		return -ENOMEM;
+
+	if (fc->purpose != FS_CONTEXT_FOR_RECONFIGURE)
+		hfsplus_fill_defaults(sbi);
+
+	fc->s_fs_info = sbi;
+	fc->ops = &hfsplus_context_ops;
+
+	return 0;
 }
 
 static struct file_system_type hfsplus_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "hfsplus",
-	.mount		= hfsplus_mount,
 	.kill_sb	= kill_block_super,
 	.fs_flags	= FS_REQUIRES_DEV,
+	.init_fs_context = hfsplus_init_fs_context,
 };
 MODULE_ALIAS_FS("hfsplus");
 
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/5] affs: convert affs to use the new mount api
  2024-09-16 17:26 ` [PATCH 2/5] affs: convert affs " Eric Sandeen
@ 2024-09-17 12:51   ` Jan Kara
  2024-09-17 14:53   ` [PATCH 2/5 V2] " Eric Sandeen
  1 sibling, 0 replies; 14+ messages in thread
From: Jan Kara @ 2024-09-17 12:51 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: linux-fsdevel, brauner, David Sterba

On Mon 16-09-24 13:26:19, Eric Sandeen wrote:
> Convert the affs filesystem to use the new mount API.
> Tested by comparing random mount & remount options before and after
> the change.
> 
> Cc: David Sterba <dsterba@suse.com>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Just one nit, otherwise feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

> +	case Opt_verbose:
> +		affs_set_opt(ctx->mount_flags, SF_VERBOSE);
> +		break;
> +	case Opt_volume: {

I guess there's no need for the braces anymore?


> +		strscpy(ctx->volume, param->string, 32);
> +		break;
>  	}

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/5] adfs: convert adfs to use the new mount api
  2024-09-16 17:26 ` [PATCH 1/5] adfs: convert adfs to use the " Eric Sandeen
@ 2024-09-17 12:52   ` Jan Kara
  0 siblings, 0 replies; 14+ messages in thread
From: Jan Kara @ 2024-09-17 12:52 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: linux-fsdevel, brauner

On Mon 16-09-24 13:26:18, Eric Sandeen wrote:
> Convert the adfs filesystem to use the new mount API.
> Tested by comparing random mount & remount options before and after
> the change.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

									Honza

> ---
>  fs/adfs/super.c | 186 +++++++++++++++++++++++++-----------------------
>  1 file changed, 95 insertions(+), 91 deletions(-)
> 
> diff --git a/fs/adfs/super.c b/fs/adfs/super.c
> index f0b999a4961b..017c48a80203 100644
> --- a/fs/adfs/super.c
> +++ b/fs/adfs/super.c
> @@ -6,7 +6,8 @@
>   */
>  #include <linux/module.h>
>  #include <linux/init.h>
> -#include <linux/parser.h>
> +#include <linux/fs_parser.h>
> +#include <linux/fs_context.h>
>  #include <linux/mount.h>
>  #include <linux/seq_file.h>
>  #include <linux/slab.h>
> @@ -115,87 +116,61 @@ static int adfs_show_options(struct seq_file *seq, struct dentry *root)
>  	return 0;
>  }
>  
> -enum {Opt_uid, Opt_gid, Opt_ownmask, Opt_othmask, Opt_ftsuffix, Opt_err};
> +enum {Opt_uid, Opt_gid, Opt_ownmask, Opt_othmask, Opt_ftsuffix};
>  
> -static const match_table_t tokens = {
> -	{Opt_uid, "uid=%u"},
> -	{Opt_gid, "gid=%u"},
> -	{Opt_ownmask, "ownmask=%o"},
> -	{Opt_othmask, "othmask=%o"},
> -	{Opt_ftsuffix, "ftsuffix=%u"},
> -	{Opt_err, NULL}
> +static const struct fs_parameter_spec adfs_param_spec[] = {
> +	fsparam_uid	("uid",		Opt_uid),
> +	fsparam_gid	("gid",		Opt_gid),
> +	fsparam_u32oct	("ownmask",	Opt_ownmask),
> +	fsparam_u32oct	("othmask",	Opt_othmask),
> +	fsparam_u32	("ftsuffix",	Opt_ftsuffix),
> +	{}
>  };
>  
> -static int parse_options(struct super_block *sb, struct adfs_sb_info *asb,
> -			 char *options)
> +static int adfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
>  {
> -	char *p;
> -	int option;
> -
> -	if (!options)
> -		return 0;
> -
> -	while ((p = strsep(&options, ",")) != NULL) {
> -		substring_t args[MAX_OPT_ARGS];
> -		int token;
> -		if (!*p)
> -			continue;
> -
> -		token = match_token(p, tokens, args);
> -		switch (token) {
> -		case Opt_uid:
> -			if (match_int(args, &option))
> -				return -EINVAL;
> -			asb->s_uid = make_kuid(current_user_ns(), option);
> -			if (!uid_valid(asb->s_uid))
> -				return -EINVAL;
> -			break;
> -		case Opt_gid:
> -			if (match_int(args, &option))
> -				return -EINVAL;
> -			asb->s_gid = make_kgid(current_user_ns(), option);
> -			if (!gid_valid(asb->s_gid))
> -				return -EINVAL;
> -			break;
> -		case Opt_ownmask:
> -			if (match_octal(args, &option))
> -				return -EINVAL;
> -			asb->s_owner_mask = option;
> -			break;
> -		case Opt_othmask:
> -			if (match_octal(args, &option))
> -				return -EINVAL;
> -			asb->s_other_mask = option;
> -			break;
> -		case Opt_ftsuffix:
> -			if (match_int(args, &option))
> -				return -EINVAL;
> -			asb->s_ftsuffix = option;
> -			break;
> -		default:
> -			adfs_msg(sb, KERN_ERR,
> -				 "unrecognised mount option \"%s\" or missing value",
> -				 p);
> -			return -EINVAL;
> -		}
> +	struct adfs_sb_info *asb = fc->s_fs_info;
> +	struct fs_parse_result result;
> +	int opt;
> +
> +	opt = fs_parse(fc, adfs_param_spec, param, &result);
> +	if (opt < 0)
> +		return opt;
> +
> +	switch (opt) {
> +	case Opt_uid:
> +		asb->s_uid = result.uid;
> +		break;
> +	case Opt_gid:
> +		asb->s_gid = result.gid;
> +		break;
> +	case Opt_ownmask:
> +		asb->s_owner_mask = result.uint_32;
> +		break;
> +	case Opt_othmask:
> +		asb->s_other_mask = result.uint_32;
> +		break;
> +	case Opt_ftsuffix:
> +		asb->s_ftsuffix = result.uint_32;
> +		break;
> +	default:
> +		return -EINVAL;
>  	}
>  	return 0;
>  }
>  
> -static int adfs_remount(struct super_block *sb, int *flags, char *data)
> +static int adfs_reconfigure(struct fs_context *fc)
>  {
> -	struct adfs_sb_info temp_asb;
> -	int ret;
> +	struct adfs_sb_info *new_asb = fc->s_fs_info;
> +	struct adfs_sb_info *asb = ADFS_SB(fc->root->d_sb);
>  
> -	sync_filesystem(sb);
> -	*flags |= ADFS_SB_FLAGS;
> +	sync_filesystem(fc->root->d_sb);
> +	fc->sb_flags |= ADFS_SB_FLAGS;
>  
> -	temp_asb = *ADFS_SB(sb);
> -	ret = parse_options(sb, &temp_asb, data);
> -	if (ret == 0)
> -		*ADFS_SB(sb) = temp_asb;
> +	/* Structure copy newly parsed options */
> +	*asb = *new_asb;
>  
> -	return ret;
> +	return 0;
>  }
>  
>  static int adfs_statfs(struct dentry *dentry, struct kstatfs *buf)
> @@ -273,7 +248,6 @@ static const struct super_operations adfs_sops = {
>  	.write_inode	= adfs_write_inode,
>  	.put_super	= adfs_put_super,
>  	.statfs		= adfs_statfs,
> -	.remount_fs	= adfs_remount,
>  	.show_options	= adfs_show_options,
>  };
>  
> @@ -361,34 +335,21 @@ static int adfs_validate_dr0(struct super_block *sb, struct buffer_head *bh,
>  	return 0;
>  }
>  
> -static int adfs_fill_super(struct super_block *sb, void *data, int silent)
> +static int adfs_fill_super(struct super_block *sb, struct fs_context *fc)
>  {
>  	struct adfs_discrecord *dr;
>  	struct object_info root_obj;
> -	struct adfs_sb_info *asb;
> +	struct adfs_sb_info *asb = sb->s_fs_info;
>  	struct inode *root;
>  	int ret = -EINVAL;
> +	int silent = fc->sb_flags & SB_SILENT;
>  
>  	sb->s_flags |= ADFS_SB_FLAGS;
>  
> -	asb = kzalloc(sizeof(*asb), GFP_KERNEL);
> -	if (!asb)
> -		return -ENOMEM;
> -
>  	sb->s_fs_info = asb;
>  	sb->s_magic = ADFS_SUPER_MAGIC;
>  	sb->s_time_gran = 10000000;
>  
> -	/* set default options */
> -	asb->s_uid = GLOBAL_ROOT_UID;
> -	asb->s_gid = GLOBAL_ROOT_GID;
> -	asb->s_owner_mask = ADFS_DEFAULT_OWNER_MASK;
> -	asb->s_other_mask = ADFS_DEFAULT_OTHER_MASK;
> -	asb->s_ftsuffix = 0;
> -
> -	if (parse_options(sb, asb, data))
> -		goto error;
> -
>  	/* Try to probe the filesystem boot block */
>  	ret = adfs_probe(sb, ADFS_DISCRECORD, 1, adfs_validate_bblk);
>  	if (ret == -EILSEQ)
> @@ -453,18 +414,61 @@ static int adfs_fill_super(struct super_block *sb, void *data, int silent)
>  	return ret;
>  }
>  
> -static struct dentry *adfs_mount(struct file_system_type *fs_type,
> -	int flags, const char *dev_name, void *data)
> +static int adfs_get_tree(struct fs_context *fc)
> +{
> +	return get_tree_bdev(fc, adfs_fill_super);
> +}
> +
> +static void adfs_free_fc(struct fs_context *fc)
>  {
> -	return mount_bdev(fs_type, flags, dev_name, data, adfs_fill_super);
> +	struct adfs_context *asb = fc->s_fs_info;
> +
> +	kfree(asb);
> +}
> +
> +static const struct fs_context_operations adfs_context_ops = {
> +	.parse_param	= adfs_parse_param,
> +	.get_tree	= adfs_get_tree,
> +	.reconfigure	= adfs_reconfigure,
> +	.free		= adfs_free_fc,
> +};
> +
> +static int adfs_init_fs_context(struct fs_context *fc)
> +{
> +	struct adfs_sb_info *asb;
> +
> +	asb = kzalloc(sizeof(struct adfs_sb_info), GFP_KERNEL);
> +	if (!asb)
> +		return -ENOMEM;
> +
> +	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
> +		struct super_block *sb = fc->root->d_sb;
> +		struct adfs_sb_info *old_asb = ADFS_SB(sb);
> +
> +		/* structure copy existing options before parsing */
> +		*asb = *old_asb;
> +	} else {
> +		/* set default options */
> +		asb->s_uid = GLOBAL_ROOT_UID;
> +		asb->s_gid = GLOBAL_ROOT_GID;
> +		asb->s_owner_mask = ADFS_DEFAULT_OWNER_MASK;
> +		asb->s_other_mask = ADFS_DEFAULT_OTHER_MASK;
> +		asb->s_ftsuffix = 0;
> +	}
> +
> +	fc->ops = &adfs_context_ops;
> +	fc->s_fs_info = asb;
> +
> +	return 0;
>  }
>  
>  static struct file_system_type adfs_fs_type = {
>  	.owner		= THIS_MODULE,
>  	.name		= "adfs",
> -	.mount		= adfs_mount,
>  	.kill_sb	= kill_block_super,
>  	.fs_flags	= FS_REQUIRES_DEV,
> +	.init_fs_context = adfs_init_fs_context,
> +	.parameters	= adfs_param_spec,
>  };
>  MODULE_ALIAS_FS("adfs");
>  
> -- 
> 2.46.0
> 
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/5 V2] affs: convert affs to use the new mount api
  2024-09-16 17:26 ` [PATCH 2/5] affs: convert affs " Eric Sandeen
  2024-09-17 12:51   ` Jan Kara
@ 2024-09-17 14:53   ` Eric Sandeen
  2024-09-17 15:37     ` David Sterba
  1 sibling, 1 reply; 14+ messages in thread
From: Eric Sandeen @ 2024-09-17 14:53 UTC (permalink / raw)
  To: Eric Sandeen, linux-fsdevel; +Cc: brauner, David Sterba, Jan Kara

Convert the affs filesystem to use the new mount API.
Tested by comparing random mount & remount options before and after
the change.

Cc: David Sterba <dsterba@suse.com>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---

V2: Remove extra braces and stray whitespace (oops)

diff --git a/fs/affs/super.c b/fs/affs/super.c
index 3c5821339609..2fa40337776d 100644
--- a/fs/affs/super.c
+++ b/fs/affs/super.c
@@ -14,7 +14,8 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/statfs.h>
-#include <linux/parser.h>
+#include <linux/fs_parser.h>
+#include <linux/fs_context.h>
 #include <linux/magic.h>
 #include <linux/sched.h>
 #include <linux/cred.h>
@@ -27,7 +28,6 @@
 
 static int affs_statfs(struct dentry *dentry, struct kstatfs *buf);
 static int affs_show_options(struct seq_file *m, struct dentry *root);
-static int affs_remount (struct super_block *sb, int *flags, char *data);
 
 static void
 affs_commit_super(struct super_block *sb, int wait)
@@ -155,140 +155,114 @@ static const struct super_operations affs_sops = {
 	.put_super	= affs_put_super,
 	.sync_fs	= affs_sync_fs,
 	.statfs		= affs_statfs,
-	.remount_fs	= affs_remount,
 	.show_options	= affs_show_options,
 };
 
 enum {
 	Opt_bs, Opt_mode, Opt_mufs, Opt_notruncate, Opt_prefix, Opt_protect,
 	Opt_reserved, Opt_root, Opt_setgid, Opt_setuid,
-	Opt_verbose, Opt_volume, Opt_ignore, Opt_err,
+	Opt_verbose, Opt_volume, Opt_ignore,
 };
 
-static const match_table_t tokens = {
-	{Opt_bs, "bs=%u"},
-	{Opt_mode, "mode=%o"},
-	{Opt_mufs, "mufs"},
-	{Opt_notruncate, "nofilenametruncate"},
-	{Opt_prefix, "prefix=%s"},
-	{Opt_protect, "protect"},
-	{Opt_reserved, "reserved=%u"},
-	{Opt_root, "root=%u"},
-	{Opt_setgid, "setgid=%u"},
-	{Opt_setuid, "setuid=%u"},
-	{Opt_verbose, "verbose"},
-	{Opt_volume, "volume=%s"},
-	{Opt_ignore, "grpquota"},
-	{Opt_ignore, "noquota"},
-	{Opt_ignore, "quota"},
-	{Opt_ignore, "usrquota"},
-	{Opt_err, NULL},
+struct affs_context {
+	kuid_t		uid;		/* uid to override */
+	kgid_t		gid;		/* gid to override */
+	unsigned int	mode;		/* mode to override */
+	unsigned int	reserved;	/* Number of reserved blocks */
+	int		root_block;	/* FFS root block number */
+	int		blocksize;	/* Initial device blksize */
+	char		*prefix;	/* Prefix for volumes and assigns */
+	char		volume[32];	/* Vol. prefix for absolute symlinks */
+	unsigned long	mount_flags;	/* Options */
 };
 
-static int
-parse_options(char *options, kuid_t *uid, kgid_t *gid, int *mode, int *reserved, s32 *root,
-		int *blocksize, char **prefix, char *volume, unsigned long *mount_opts)
+static const struct fs_parameter_spec affs_param_spec[] = {
+	fsparam_u32	("bs",		Opt_bs),
+	fsparam_u32oct	("mode",	Opt_mode),
+	fsparam_flag	("mufs",	Opt_mufs),
+	fsparam_flag	("nofilenametruncate",	Opt_notruncate),
+	fsparam_string	("prefix",	Opt_prefix),
+	fsparam_flag	("protect",	Opt_protect),
+	fsparam_u32	("reserved",	Opt_reserved),
+	fsparam_u32	("root",	Opt_root),
+	fsparam_gid	("setgid",	Opt_setgid),
+	fsparam_uid	("setuid",	Opt_setuid),
+	fsparam_flag	("verbose",	Opt_verbose),
+	fsparam_string	("volume",	Opt_volume),
+	fsparam_flag	("grpquota",	Opt_ignore),
+	fsparam_flag	("noquota",	Opt_ignore),
+	fsparam_flag	("quota",	Opt_ignore),
+	fsparam_flag	("usrquota",	Opt_ignore),
+	{},
+};
+
+static int affs_parse_param(struct fs_context *fc, struct fs_parameter *param)
 {
-	char *p;
-	substring_t args[MAX_OPT_ARGS];
-
-	/* Fill in defaults */
-
-	*uid        = current_uid();
-	*gid        = current_gid();
-	*reserved   = 2;
-	*root       = -1;
-	*blocksize  = -1;
-	volume[0]   = ':';
-	volume[1]   = 0;
-	*mount_opts = 0;
-	if (!options)
-		return 1;
-
-	while ((p = strsep(&options, ",")) != NULL) {
-		int token, n, option;
-		if (!*p)
-			continue;
-
-		token = match_token(p, tokens, args);
-		switch (token) {
-		case Opt_bs:
-			if (match_int(&args[0], &n))
-				return 0;
-			if (n != 512 && n != 1024 && n != 2048
-			    && n != 4096) {
-				pr_warn("Invalid blocksize (512, 1024, 2048, 4096 allowed)\n");
-				return 0;
-			}
-			*blocksize = n;
-			break;
-		case Opt_mode:
-			if (match_octal(&args[0], &option))
-				return 0;
-			*mode = option & 0777;
-			affs_set_opt(*mount_opts, SF_SETMODE);
-			break;
-		case Opt_mufs:
-			affs_set_opt(*mount_opts, SF_MUFS);
-			break;
-		case Opt_notruncate:
-			affs_set_opt(*mount_opts, SF_NO_TRUNCATE);
-			break;
-		case Opt_prefix:
-			kfree(*prefix);
-			*prefix = match_strdup(&args[0]);
-			if (!*prefix)
-				return 0;
-			affs_set_opt(*mount_opts, SF_PREFIX);
-			break;
-		case Opt_protect:
-			affs_set_opt(*mount_opts, SF_IMMUTABLE);
-			break;
-		case Opt_reserved:
-			if (match_int(&args[0], reserved))
-				return 0;
-			break;
-		case Opt_root:
-			if (match_int(&args[0], root))
-				return 0;
-			break;
-		case Opt_setgid:
-			if (match_int(&args[0], &option))
-				return 0;
-			*gid = make_kgid(current_user_ns(), option);
-			if (!gid_valid(*gid))
-				return 0;
-			affs_set_opt(*mount_opts, SF_SETGID);
-			break;
-		case Opt_setuid:
-			if (match_int(&args[0], &option))
-				return 0;
-			*uid = make_kuid(current_user_ns(), option);
-			if (!uid_valid(*uid))
-				return 0;
-			affs_set_opt(*mount_opts, SF_SETUID);
-			break;
-		case Opt_verbose:
-			affs_set_opt(*mount_opts, SF_VERBOSE);
-			break;
-		case Opt_volume: {
-			char *vol = match_strdup(&args[0]);
-			if (!vol)
-				return 0;
-			strscpy(volume, vol, 32);
-			kfree(vol);
-			break;
-		}
-		case Opt_ignore:
-		 	/* Silently ignore the quota options */
-			break;
-		default:
-			pr_warn("Unrecognized mount option \"%s\" or missing value\n",
-				p);
-			return 0;
+	struct affs_context *ctx = fc->fs_private;
+	struct fs_parse_result result;
+	int n;
+	int opt;
+
+	opt = fs_parse(fc, affs_param_spec, param, &result);
+	if (opt < 0)
+		return opt;
+
+	switch (opt) {
+	case Opt_bs:
+		n = result.uint_32;
+		if (n != 512 && n != 1024 && n != 2048
+		    && n != 4096) {
+			pr_warn("Invalid blocksize (512, 1024, 2048, 4096 allowed)\n");
+			return -EINVAL;
 		}
+		ctx->blocksize = n;
+		break;
+	case Opt_mode:
+		ctx->mode = result.uint_32 & 0777;
+		affs_set_opt(ctx->mount_flags, SF_SETMODE);
+		break;
+	case Opt_mufs:
+		affs_set_opt(ctx->mount_flags, SF_MUFS);
+		break;
+	case Opt_notruncate:
+		affs_set_opt(ctx->mount_flags, SF_NO_TRUNCATE);
+		break;
+	case Opt_prefix:
+		kfree(ctx->prefix);
+		ctx->prefix = param->string;
+		param->string = NULL;
+		affs_set_opt(ctx->mount_flags, SF_PREFIX);
+		break;
+	case Opt_protect:
+		affs_set_opt(ctx->mount_flags, SF_IMMUTABLE);
+		break;
+	case Opt_reserved:
+		ctx->reserved = result.uint_32;
+		break;
+	case Opt_root:
+		ctx->root_block = result.uint_32;
+		break;
+	case Opt_setgid:
+		ctx->gid = result.gid;
+		affs_set_opt(ctx->mount_flags, SF_SETGID);
+		break;
+	case Opt_setuid:
+		ctx->uid = result.uid;
+		affs_set_opt(ctx->mount_flags, SF_SETUID);
+		break;
+	case Opt_verbose:
+		affs_set_opt(ctx->mount_flags, SF_VERBOSE);
+		break;
+	case Opt_volume:
+		strscpy(ctx->volume, param->string, 32);
+		break;
+	case Opt_ignore:
+		/* Silently ignore the quota options */
+		break;
+	default:
+		return -EINVAL;
 	}
-	return 1;
+	return 0;
 }
 
 static int affs_show_options(struct seq_file *m, struct dentry *root)
@@ -329,27 +303,22 @@ static int affs_show_options(struct seq_file *m, struct dentry *root)
  * hopefully have the guts to do so. Until then: sorry for the mess.
  */
 
-static int affs_fill_super(struct super_block *sb, void *data, int silent)
+static int affs_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	struct affs_sb_info	*sbi;
+	struct affs_context	*ctx = fc->fs_private;
 	struct buffer_head	*root_bh = NULL;
 	struct buffer_head	*boot_bh;
 	struct inode		*root_inode = NULL;
-	s32			 root_block;
+	int			 silent = fc->sb_flags & SB_SILENT;
 	int			 size, blocksize;
 	u32			 chksum;
 	int			 num_bm;
 	int			 i, j;
-	kuid_t			 uid;
-	kgid_t			 gid;
-	int			 reserved;
-	unsigned long		 mount_flags;
 	int			 tmp_flags;	/* fix remount prototype... */
 	u8			 sig[4];
 	int			 ret;
 
-	pr_debug("read_super(%s)\n", data ? (const char *)data : "no options");
-
 	sb->s_magic             = AFFS_SUPER_MAGIC;
 	sb->s_op                = &affs_sops;
 	sb->s_flags |= SB_NODIRATIME;
@@ -369,19 +338,16 @@ static int affs_fill_super(struct super_block *sb, void *data, int silent)
 	spin_lock_init(&sbi->work_lock);
 	INIT_DELAYED_WORK(&sbi->sb_work, flush_superblock);
 
-	if (!parse_options(data,&uid,&gid,&i,&reserved,&root_block,
-				&blocksize,&sbi->s_prefix,
-				sbi->s_volume, &mount_flags)) {
-		pr_err("Error parsing options\n");
-		return -EINVAL;
-	}
-	/* N.B. after this point s_prefix must be released */
+	sbi->s_flags	= ctx->mount_flags;
+	sbi->s_mode	= ctx->mode;
+	sbi->s_uid	= ctx->uid;
+	sbi->s_gid	= ctx->gid;
+	sbi->s_reserved	= ctx->reserved;
+	sbi->s_prefix	= ctx->prefix;
+	ctx->prefix	= NULL;
+	memcpy(sbi->s_volume, ctx->volume, 32);
 
-	sbi->s_flags   = mount_flags;
-	sbi->s_mode    = i;
-	sbi->s_uid     = uid;
-	sbi->s_gid     = gid;
-	sbi->s_reserved= reserved;
+	/* N.B. after this point s_prefix must be released */
 
 	/* Get the size of the device in 512-byte blocks.
 	 * If we later see that the partition uses bigger
@@ -396,15 +362,16 @@ static int affs_fill_super(struct super_block *sb, void *data, int silent)
 
 	i = bdev_logical_block_size(sb->s_bdev);
 	j = PAGE_SIZE;
+	blocksize = ctx->blocksize;
 	if (blocksize > 0) {
 		i = j = blocksize;
 		size = size / (blocksize / 512);
 	}
 
 	for (blocksize = i; blocksize <= j; blocksize <<= 1, size >>= 1) {
-		sbi->s_root_block = root_block;
-		if (root_block < 0)
-			sbi->s_root_block = (reserved + size - 1) / 2;
+		sbi->s_root_block = ctx->root_block;
+		if (ctx->root_block < 0)
+			sbi->s_root_block = (ctx->reserved + size - 1) / 2;
 		pr_debug("setting blocksize to %d\n", blocksize);
 		affs_set_blocksize(sb, blocksize);
 		sbi->s_partition_size = size;
@@ -424,7 +391,7 @@ static int affs_fill_super(struct super_block *sb, void *data, int silent)
 				"size=%d, reserved=%d\n",
 				sb->s_id,
 				sbi->s_root_block + num_bm,
-				blocksize, size, reserved);
+				ctx->blocksize, size, ctx->reserved);
 			root_bh = affs_bread(sb, sbi->s_root_block + num_bm);
 			if (!root_bh)
 				continue;
@@ -447,7 +414,7 @@ static int affs_fill_super(struct super_block *sb, void *data, int silent)
 got_root:
 	/* Keep super block in cache */
 	sbi->s_root_bh = root_bh;
-	root_block = sbi->s_root_block;
+	ctx->root_block = sbi->s_root_block;
 
 	/* Find out which kind of FS we have */
 	boot_bh = sb_bread(sb, 0);
@@ -506,7 +473,7 @@ static int affs_fill_super(struct super_block *sb, void *data, int silent)
 		return -EINVAL;
 	}
 
-	if (affs_test_opt(mount_flags, SF_VERBOSE)) {
+	if (affs_test_opt(ctx->mount_flags, SF_VERBOSE)) {
 		u8 len = AFFS_ROOT_TAIL(sb, root_bh)->disk_name[0];
 		pr_notice("Mounting volume \"%.*s\": Type=%.3s\\%c, Blocksize=%d\n",
 			len > 31 ? 31 : len,
@@ -528,7 +495,7 @@ static int affs_fill_super(struct super_block *sb, void *data, int silent)
 
 	/* set up enough so that it can read an inode */
 
-	root_inode = affs_iget(sb, root_block);
+	root_inode = affs_iget(sb, ctx->root_block);
 	if (IS_ERR(root_inode))
 		return PTR_ERR(root_inode);
 
@@ -548,56 +515,43 @@ static int affs_fill_super(struct super_block *sb, void *data, int silent)
 	return 0;
 }
 
-static int
-affs_remount(struct super_block *sb, int *flags, char *data)
+static int affs_reconfigure(struct fs_context *fc)
 {
+	struct super_block	*sb = fc->root->d_sb;
+	struct affs_context	*ctx = fc->fs_private;
 	struct affs_sb_info	*sbi = AFFS_SB(sb);
-	int			 blocksize;
-	kuid_t			 uid;
-	kgid_t			 gid;
-	int			 mode;
-	int			 reserved;
-	int			 root_block;
-	unsigned long		 mount_flags;
 	int			 res = 0;
-	char			 volume[32];
-	char			*prefix = NULL;
-
-	pr_debug("%s(flags=0x%x,opts=\"%s\")\n", __func__, *flags, data);
 
 	sync_filesystem(sb);
-	*flags |= SB_NODIRATIME;
-
-	memcpy(volume, sbi->s_volume, 32);
-	if (!parse_options(data, &uid, &gid, &mode, &reserved, &root_block,
-			   &blocksize, &prefix, volume,
-			   &mount_flags)) {
-		kfree(prefix);
-		return -EINVAL;
-	}
+	fc->sb_flags |= SB_NODIRATIME;
 
 	flush_delayed_work(&sbi->sb_work);
 
-	sbi->s_flags = mount_flags;
-	sbi->s_mode  = mode;
-	sbi->s_uid   = uid;
-	sbi->s_gid   = gid;
+	/*
+	 * NB: Historically, only mount_flags, mode, uid, gic, prefix,
+	 * and volume are accepted during remount.
+	 */
+	sbi->s_flags = ctx->mount_flags;
+	sbi->s_mode  = ctx->mode;
+	sbi->s_uid   = ctx->uid;
+	sbi->s_gid   = ctx->gid;
 	/* protect against readers */
 	spin_lock(&sbi->symlink_lock);
-	if (prefix) {
+	if (ctx->prefix) {
 		kfree(sbi->s_prefix);
-		sbi->s_prefix = prefix;
+		sbi->s_prefix = ctx->prefix;
+		ctx->prefix = NULL;
 	}
-	memcpy(sbi->s_volume, volume, 32);
+	memcpy(sbi->s_volume, ctx->volume, 32);
 	spin_unlock(&sbi->symlink_lock);
 
-	if ((bool)(*flags & SB_RDONLY) == sb_rdonly(sb))
+	if ((bool)(fc->sb_flags & SB_RDONLY) == sb_rdonly(sb))
 		return 0;
 
-	if (*flags & SB_RDONLY)
+	if (fc->sb_flags & SB_RDONLY)
 		affs_free_bitmap(sb);
 	else
-		res = affs_init_bitmap(sb, flags);
+		res = affs_init_bitmap(sb, &fc->sb_flags);
 
 	return res;
 }
@@ -624,10 +578,9 @@ affs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	return 0;
 }
 
-static struct dentry *affs_mount(struct file_system_type *fs_type,
-	int flags, const char *dev_name, void *data)
+static int affs_get_tree(struct fs_context *fc)
 {
-	return mount_bdev(fs_type, flags, dev_name, data, affs_fill_super);
+	return get_tree_bdev(fc, affs_fill_super);
 }
 
 static void affs_kill_sb(struct super_block *sb)
@@ -643,12 +596,61 @@ static void affs_kill_sb(struct super_block *sb)
 	}
 }
 
+static void affs_free_fc(struct fs_context *fc)
+{
+	struct affs_context *ctx = fc->fs_private;
+
+	kfree(ctx->prefix);
+	kfree(ctx);
+}
+
+static const struct fs_context_operations affs_context_ops = {
+	.parse_param	= affs_parse_param,
+	.get_tree	= affs_get_tree,
+	.reconfigure	= affs_reconfigure,
+	.free		= affs_free_fc,
+};
+
+static int affs_init_fs_context(struct fs_context *fc)
+{
+	struct affs_context *ctx;
+
+	ctx = kzalloc(sizeof(struct affs_context), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
+		struct super_block *sb = fc->root->d_sb;
+		struct affs_sb_info *sbi = AFFS_SB(sb);
+
+		/*
+		 * NB: historically, no options other than volume were
+		 * preserved across a remount unless they were explicitly
+		 * passed in.
+		 */
+		memcpy(ctx->volume, sbi->s_volume, 32);
+	} else {
+		ctx->uid	= current_uid();
+		ctx->gid	= current_gid();
+		ctx->reserved	= 2;
+		ctx->root_block	= -1;
+		ctx->blocksize	= -1;
+		ctx->volume[0]	= ':';
+	}
+
+	fc->ops = &affs_context_ops;
+	fc->fs_private = ctx;
+
+	return 0;
+}
+
 static struct file_system_type affs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "affs",
-	.mount		= affs_mount,
 	.kill_sb	= affs_kill_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
+	.init_fs_context = affs_init_fs_context,
+	.parameters	= affs_param_spec,
 };
 MODULE_ALIAS_FS("affs");
 


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/5 V2] affs: convert affs to use the new mount api
  2024-09-17 14:53   ` [PATCH 2/5 V2] " Eric Sandeen
@ 2024-09-17 15:37     ` David Sterba
  0 siblings, 0 replies; 14+ messages in thread
From: David Sterba @ 2024-09-17 15:37 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: Eric Sandeen, linux-fsdevel, brauner, David Sterba, Jan Kara

On Tue, Sep 17, 2024 at 09:53:09AM -0500, Eric Sandeen wrote:
> Convert the affs filesystem to use the new mount API.
> Tested by comparing random mount & remount options before and after
> the change.
> 
> Cc: David Sterba <dsterba@suse.com>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Reviewed-by: Jan Kara <jack@suse.cz>
> ---
> 
> V2: Remove extra braces and stray whitespace (oops)

Reviewed-by: David Sterba <dsterba@suse.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/5] befs: convert befs to use the new mount api
  2024-09-16 17:26 ` [PATCH 3/5] befs: convert befs " Eric Sandeen
@ 2024-09-17 15:50   ` Jan Kara
  0 siblings, 0 replies; 14+ messages in thread
From: Jan Kara @ 2024-09-17 15:50 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: linux-fsdevel, brauner, Luis de Bethencourt, Salah Triki

On Mon 16-09-24 13:26:20, Eric Sandeen wrote:
> Convert the befs filesystem to use the new mount API.
> Tested by comparing random mount & remount options before and after
> the change.
> 
> Cc: Luis de Bethencourt <luisbg@kernel.org>
> Cc: Salah Triki <salah.triki@gmail.com>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/befs/linuxvfs.c | 199 +++++++++++++++++++++++----------------------
>  1 file changed, 102 insertions(+), 97 deletions(-)
> 
> diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
> index f92f108840f5..8f430ff8e445 100644
> --- a/fs/befs/linuxvfs.c
> +++ b/fs/befs/linuxvfs.c
> @@ -11,12 +11,13 @@
>  #include <linux/module.h>
>  #include <linux/slab.h>
>  #include <linux/fs.h>
> +#include <linux/fs_context.h>
> +#include <linux/fs_parser.h>
>  #include <linux/errno.h>
>  #include <linux/stat.h>
>  #include <linux/nls.h>
>  #include <linux/buffer_head.h>
>  #include <linux/vfs.h>
> -#include <linux/parser.h>
>  #include <linux/namei.h>
>  #include <linux/sched.h>
>  #include <linux/cred.h>
> @@ -54,22 +55,20 @@ static int befs_utf2nls(struct super_block *sb, const char *in, int in_len,
>  static int befs_nls2utf(struct super_block *sb, const char *in, int in_len,
>  			char **out, int *out_len);
>  static void befs_put_super(struct super_block *);
> -static int befs_remount(struct super_block *, int *, char *);
>  static int befs_statfs(struct dentry *, struct kstatfs *);
>  static int befs_show_options(struct seq_file *, struct dentry *);
> -static int parse_options(char *, struct befs_mount_options *);
>  static struct dentry *befs_fh_to_dentry(struct super_block *sb,
>  				struct fid *fid, int fh_len, int fh_type);
>  static struct dentry *befs_fh_to_parent(struct super_block *sb,
>  				struct fid *fid, int fh_len, int fh_type);
>  static struct dentry *befs_get_parent(struct dentry *child);
> +static void befs_free_fc(struct fs_context *fc);
>  
>  static const struct super_operations befs_sops = {
>  	.alloc_inode	= befs_alloc_inode,	/* allocate a new inode */
>  	.free_inode	= befs_free_inode, /* deallocate an inode */
>  	.put_super	= befs_put_super,	/* uninit super */
>  	.statfs		= befs_statfs,	/* statfs */
> -	.remount_fs	= befs_remount,
>  	.show_options	= befs_show_options,
>  };
>  
> @@ -672,92 +671,53 @@ static struct dentry *befs_get_parent(struct dentry *child)
>  }
>  
>  enum {
> -	Opt_uid, Opt_gid, Opt_charset, Opt_debug, Opt_err,
> +	Opt_uid, Opt_gid, Opt_charset, Opt_debug,
>  };
>  
> -static const match_table_t befs_tokens = {
> -	{Opt_uid, "uid=%d"},
> -	{Opt_gid, "gid=%d"},
> -	{Opt_charset, "iocharset=%s"},
> -	{Opt_debug, "debug"},
> -	{Opt_err, NULL}
> +static const struct fs_parameter_spec befs_param_spec[] = {
> +	fsparam_uid	("uid",		Opt_uid),
> +	fsparam_gid	("gid",		Opt_gid),
> +	fsparam_string	("iocharset",	Opt_charset),
> +	fsparam_flag	("debug",	Opt_debug),
> +	{}
>  };
>  
>  static int
> -parse_options(char *options, struct befs_mount_options *opts)
> +befs_parse_param(struct fs_context *fc, struct fs_parameter *param)
>  {
> -	char *p;
> -	substring_t args[MAX_OPT_ARGS];
> -	int option;
> -	kuid_t uid;
> -	kgid_t gid;
> -
> -	/* Initialize options */
> -	opts->uid = GLOBAL_ROOT_UID;
> -	opts->gid = GLOBAL_ROOT_GID;
> -	opts->use_uid = 0;
> -	opts->use_gid = 0;
> -	opts->iocharset = NULL;
> -	opts->debug = 0;
> -
> -	if (!options)
> -		return 1;
> -
> -	while ((p = strsep(&options, ",")) != NULL) {
> -		int token;
> -
> -		if (!*p)
> -			continue;
> -
> -		token = match_token(p, befs_tokens, args);
> -		switch (token) {
> -		case Opt_uid:
> -			if (match_int(&args[0], &option))
> -				return 0;
> -			uid = INVALID_UID;
> -			if (option >= 0)
> -				uid = make_kuid(current_user_ns(), option);
> -			if (!uid_valid(uid)) {
> -				pr_err("Invalid uid %d, "
> -				       "using default\n", option);
> -				break;
> -			}
> -			opts->uid = uid;
> -			opts->use_uid = 1;
> -			break;
> -		case Opt_gid:
> -			if (match_int(&args[0], &option))
> -				return 0;
> -			gid = INVALID_GID;
> -			if (option >= 0)
> -				gid = make_kgid(current_user_ns(), option);
> -			if (!gid_valid(gid)) {
> -				pr_err("Invalid gid %d, "
> -				       "using default\n", option);
> -				break;
> -			}
> -			opts->gid = gid;
> -			opts->use_gid = 1;
> -			break;
> -		case Opt_charset:
> -			kfree(opts->iocharset);
> -			opts->iocharset = match_strdup(&args[0]);
> -			if (!opts->iocharset) {
> -				pr_err("allocation failure for "
> -				       "iocharset string\n");
> -				return 0;
> -			}
> -			break;
> -		case Opt_debug:
> -			opts->debug = 1;
> -			break;
> -		default:
> -			pr_err("Unrecognized mount option \"%s\" "
> -			       "or missing value\n", p);
> -			return 0;
> -		}
> +	struct befs_mount_options *opts = fc->fs_private;
> +	int token;
> +	struct fs_parse_result result;
> +
> +	/* befs ignores all options on remount */
> +	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE)
> +		return 0;
> +
> +	token = fs_parse(fc, befs_param_spec, param, &result);
> +	if (token < 0)
> +		return token;
> +
> +	switch (token) {
> +	case Opt_uid:
> +		opts->uid = result.uid;
> +		opts->use_uid = 1;
> +		break;
> +	case Opt_gid:
> +		opts->gid = result.gid;
> +		opts->use_gid = 1;
> +		break;
> +	case Opt_charset:
> +		kfree(opts->iocharset);
> +		opts->iocharset = param->string;
> +		param->string = NULL;
> +		break;
> +	case Opt_debug:
> +		opts->debug = 1;
> +		break;
> +	default:
> +		return -EINVAL;
>  	}
> -	return 1;
> +	return 0;
>  }
>  
>  static int befs_show_options(struct seq_file *m, struct dentry *root)
> @@ -793,6 +753,21 @@ befs_put_super(struct super_block *sb)
>  	sb->s_fs_info = NULL;
>  }
>  
> +/*
> + * Copy the parsed options into the sbi mount_options member
> + */
> +static void
> +befs_set_options(struct befs_sb_info *sbi, struct befs_mount_options *opts)
> +{
> +	sbi->mount_opts.uid = opts->uid;
> +	sbi->mount_opts.gid = opts->gid;
> +	sbi->mount_opts.use_uid = opts->use_uid;
> +	sbi->mount_opts.use_gid = opts->use_gid;
> +	sbi->mount_opts.debug = opts->debug;
> +	sbi->mount_opts.iocharset = opts->iocharset;
> +	opts->iocharset = NULL;
> +}
> +
>  /* Allocate private field of the superblock, fill it.
>   *
>   * Finish filling the public superblock fields
> @@ -800,7 +775,7 @@ befs_put_super(struct super_block *sb)
>   * Load a set of NLS translations if needed.
>   */
>  static int
> -befs_fill_super(struct super_block *sb, void *data, int silent)
> +befs_fill_super(struct super_block *sb, struct fs_context *fc)
>  {
>  	struct buffer_head *bh;
>  	struct befs_sb_info *befs_sb;
> @@ -810,6 +785,8 @@ befs_fill_super(struct super_block *sb, void *data, int silent)
>  	const unsigned long sb_block = 0;
>  	const off_t x86_sb_off = 512;
>  	int blocksize;
> +	struct befs_mount_options *parsed_opts = fc->fs_private;
> +	int silent = fc->sb_flags & SB_SILENT;
>  
>  	sb->s_fs_info = kzalloc(sizeof(*befs_sb), GFP_KERNEL);
>  	if (sb->s_fs_info == NULL)
> @@ -817,11 +794,7 @@ befs_fill_super(struct super_block *sb, void *data, int silent)
>  
>  	befs_sb = BEFS_SB(sb);
>  
> -	if (!parse_options((char *) data, &befs_sb->mount_opts)) {
> -		if (!silent)
> -			befs_error(sb, "cannot parse mount options");
> -		goto unacquire_priv_sbp;
> -	}
> +	befs_set_options(befs_sb, parsed_opts);
>  
>  	befs_debug(sb, "---> %s", __func__);
>  
> @@ -934,10 +907,10 @@ befs_fill_super(struct super_block *sb, void *data, int silent)
>  }
>  
>  static int
> -befs_remount(struct super_block *sb, int *flags, char *data)
> +befs_reconfigure(struct fs_context *fc)
>  {
> -	sync_filesystem(sb);
> -	if (!(*flags & SB_RDONLY))
> +	sync_filesystem(fc->root->d_sb);
> +	if (!(fc->sb_flags & SB_RDONLY))
>  		return -EINVAL;
>  	return 0;
>  }
> @@ -965,19 +938,51 @@ befs_statfs(struct dentry *dentry, struct kstatfs *buf)
>  	return 0;
>  }
>  
> -static struct dentry *
> -befs_mount(struct file_system_type *fs_type, int flags, const char *dev_name,
> -	    void *data)
> +static int befs_get_tree(struct fs_context *fc)
> +{
> +	return get_tree_bdev(fc, befs_fill_super);
> +}
> +
> +static const struct fs_context_operations befs_context_ops = {
> +	.parse_param	= befs_parse_param,
> +	.get_tree	= befs_get_tree,
> +	.reconfigure	= befs_reconfigure,
> +	.free		= befs_free_fc,
> +};
> +
> +static int befs_init_fs_context(struct fs_context *fc)
> +{
> +	struct befs_mount_options *opts;
> +
> +	opts = kzalloc(sizeof(*opts), GFP_KERNEL);
> +	if (!opts)
> +		return -ENOMEM;
> +
> +	/* Initialize options */
> +	opts->uid = GLOBAL_ROOT_UID;
> +	opts->gid = GLOBAL_ROOT_GID;
> +
> +	fc->fs_private = opts;
> +	fc->ops = &befs_context_ops;
> +
> +	return 0;
> +}
> +
> +static void befs_free_fc(struct fs_context *fc)
>  {
> -	return mount_bdev(fs_type, flags, dev_name, data, befs_fill_super);
> +	struct befs_mount_options *opts = fc->fs_private;
> +
> +	kfree(opts->iocharset);
> +	kfree(fc->fs_private);
>  }
>  
>  static struct file_system_type befs_fs_type = {
>  	.owner		= THIS_MODULE,
>  	.name		= "befs",
> -	.mount		= befs_mount,
>  	.kill_sb	= kill_block_super,
>  	.fs_flags	= FS_REQUIRES_DEV,
> +	.init_fs_context = befs_init_fs_context,
> +	.parameters	= befs_param_spec,
>  };
>  MODULE_ALIAS_FS("befs");
>  
> -- 
> 2.46.0
> 
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/5] hfs: convert hfs to use the new mount api
  2024-09-16 17:26 ` [PATCH 4/5] hfs: convert hfs " Eric Sandeen
@ 2024-09-17 15:55   ` Jan Kara
  0 siblings, 0 replies; 14+ messages in thread
From: Jan Kara @ 2024-09-17 15:55 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: linux-fsdevel, brauner

On Mon 16-09-24 13:26:21, Eric Sandeen wrote:
> Convert the hfs filesystem to use the new mount API.
> Tested by comparing random mount & remount options before and after
> the change.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/hfs/super.c | 341 ++++++++++++++++++++++---------------------------
>  1 file changed, 154 insertions(+), 187 deletions(-)
> 
> diff --git a/fs/hfs/super.c b/fs/hfs/super.c
> index eeac99765f0d..ee314f3e39f8 100644
> --- a/fs/hfs/super.c
> +++ b/fs/hfs/super.c
> @@ -15,10 +15,11 @@
>  #include <linux/module.h>
>  #include <linux/blkdev.h>
>  #include <linux/backing-dev.h>
> +#include <linux/fs_context.h>
> +#include <linux/fs_parser.h>
>  #include <linux/mount.h>
>  #include <linux/init.h>
>  #include <linux/nls.h>
> -#include <linux/parser.h>
>  #include <linux/seq_file.h>
>  #include <linux/slab.h>
>  #include <linux/vfs.h>
> @@ -111,21 +112,24 @@ static int hfs_statfs(struct dentry *dentry, struct kstatfs *buf)
>  	return 0;
>  }
>  
> -static int hfs_remount(struct super_block *sb, int *flags, char *data)
> +static int hfs_reconfigure(struct fs_context *fc)
>  {
> +	struct super_block *sb = fc->root->d_sb;
> +
>  	sync_filesystem(sb);
> -	*flags |= SB_NODIRATIME;
> -	if ((bool)(*flags & SB_RDONLY) == sb_rdonly(sb))
> +	fc->sb_flags |= SB_NODIRATIME;
> +	if ((bool)(fc->sb_flags & SB_RDONLY) == sb_rdonly(sb))
>  		return 0;
> -	if (!(*flags & SB_RDONLY)) {
> +
> +	if (!(fc->sb_flags & SB_RDONLY)) {
>  		if (!(HFS_SB(sb)->mdb->drAtrb & cpu_to_be16(HFS_SB_ATTRIB_UNMNT))) {
>  			pr_warn("filesystem was not cleanly unmounted, running fsck.hfs is recommended.  leaving read-only.\n");
>  			sb->s_flags |= SB_RDONLY;
> -			*flags |= SB_RDONLY;
> +			fc->sb_flags |= SB_RDONLY;
>  		} else if (HFS_SB(sb)->mdb->drAtrb & cpu_to_be16(HFS_SB_ATTRIB_SLOCK)) {
>  			pr_warn("filesystem is marked locked, leaving read-only.\n");
>  			sb->s_flags |= SB_RDONLY;
> -			*flags |= SB_RDONLY;
> +			fc->sb_flags |= SB_RDONLY;
>  		}
>  	}
>  	return 0;
> @@ -180,7 +184,6 @@ static const struct super_operations hfs_super_operations = {
>  	.put_super	= hfs_put_super,
>  	.sync_fs	= hfs_sync_fs,
>  	.statfs		= hfs_statfs,
> -	.remount_fs     = hfs_remount,
>  	.show_options	= hfs_show_options,
>  };
>  
> @@ -188,181 +191,112 @@ enum {
>  	opt_uid, opt_gid, opt_umask, opt_file_umask, opt_dir_umask,
>  	opt_part, opt_session, opt_type, opt_creator, opt_quiet,
>  	opt_codepage, opt_iocharset,
> -	opt_err
>  };
>  
> -static const match_table_t tokens = {
> -	{ opt_uid, "uid=%u" },
> -	{ opt_gid, "gid=%u" },
> -	{ opt_umask, "umask=%o" },
> -	{ opt_file_umask, "file_umask=%o" },
> -	{ opt_dir_umask, "dir_umask=%o" },
> -	{ opt_part, "part=%u" },
> -	{ opt_session, "session=%u" },
> -	{ opt_type, "type=%s" },
> -	{ opt_creator, "creator=%s" },
> -	{ opt_quiet, "quiet" },
> -	{ opt_codepage, "codepage=%s" },
> -	{ opt_iocharset, "iocharset=%s" },
> -	{ opt_err, NULL }
> +static const struct fs_parameter_spec hfs_param_spec[] = {
> +	fsparam_u32	("uid",		opt_uid),
> +	fsparam_u32	("gid",		opt_gid),
> +	fsparam_u32oct	("umask",	opt_umask),
> +	fsparam_u32oct	("file_umask",	opt_file_umask),
> +	fsparam_u32oct	("dir_umask",	opt_dir_umask),
> +	fsparam_u32	("part",	opt_part),
> +	fsparam_u32	("session",	opt_session),
> +	fsparam_string	("type",	opt_type),
> +	fsparam_string	("creator",	opt_creator),
> +	fsparam_flag	("quiet",	opt_quiet),
> +	fsparam_string	("codepage",	opt_codepage),
> +	fsparam_string	("iocharset",	opt_iocharset),
> +	{}
>  };
>  
> -static inline int match_fourchar(substring_t *arg, u32 *result)
> -{
> -	if (arg->to - arg->from != 4)
> -		return -EINVAL;
> -	memcpy(result, arg->from, 4);
> -	return 0;
> -}
> -
>  /*
> - * parse_options()
> + * hfs_parse_param()
>   *
> - * adapted from linux/fs/msdos/inode.c written 1992,93 by Werner Almesberger
> - * This function is called by hfs_read_super() to parse the mount options.
> + * This function is called by the vfs to parse the mount options.
>   */
> -static int parse_options(char *options, struct hfs_sb_info *hsb)
> +static int hfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
>  {
> -	char *p;
> -	substring_t args[MAX_OPT_ARGS];
> -	int tmp, token;
> -
> -	/* initialize the sb with defaults */
> -	hsb->s_uid = current_uid();
> -	hsb->s_gid = current_gid();
> -	hsb->s_file_umask = 0133;
> -	hsb->s_dir_umask = 0022;
> -	hsb->s_type = hsb->s_creator = cpu_to_be32(0x3f3f3f3f);	/* == '????' */
> -	hsb->s_quiet = 0;
> -	hsb->part = -1;
> -	hsb->session = -1;
> -
> -	if (!options)
> -		return 1;
> -
> -	while ((p = strsep(&options, ",")) != NULL) {
> -		if (!*p)
> -			continue;
> -
> -		token = match_token(p, tokens, args);
> -		switch (token) {
> -		case opt_uid:
> -			if (match_int(&args[0], &tmp)) {
> -				pr_err("uid requires an argument\n");
> -				return 0;
> -			}
> -			hsb->s_uid = make_kuid(current_user_ns(), (uid_t)tmp);
> -			if (!uid_valid(hsb->s_uid)) {
> -				pr_err("invalid uid %d\n", tmp);
> -				return 0;
> -			}
> -			break;
> -		case opt_gid:
> -			if (match_int(&args[0], &tmp)) {
> -				pr_err("gid requires an argument\n");
> -				return 0;
> -			}
> -			hsb->s_gid = make_kgid(current_user_ns(), (gid_t)tmp);
> -			if (!gid_valid(hsb->s_gid)) {
> -				pr_err("invalid gid %d\n", tmp);
> -				return 0;
> -			}
> -			break;
> -		case opt_umask:
> -			if (match_octal(&args[0], &tmp)) {
> -				pr_err("umask requires a value\n");
> -				return 0;
> -			}
> -			hsb->s_file_umask = (umode_t)tmp;
> -			hsb->s_dir_umask = (umode_t)tmp;
> -			break;
> -		case opt_file_umask:
> -			if (match_octal(&args[0], &tmp)) {
> -				pr_err("file_umask requires a value\n");
> -				return 0;
> -			}
> -			hsb->s_file_umask = (umode_t)tmp;
> -			break;
> -		case opt_dir_umask:
> -			if (match_octal(&args[0], &tmp)) {
> -				pr_err("dir_umask requires a value\n");
> -				return 0;
> -			}
> -			hsb->s_dir_umask = (umode_t)tmp;
> -			break;
> -		case opt_part:
> -			if (match_int(&args[0], &hsb->part)) {
> -				pr_err("part requires an argument\n");
> -				return 0;
> -			}
> -			break;
> -		case opt_session:
> -			if (match_int(&args[0], &hsb->session)) {
> -				pr_err("session requires an argument\n");
> -				return 0;
> -			}
> -			break;
> -		case opt_type:
> -			if (match_fourchar(&args[0], &hsb->s_type)) {
> -				pr_err("type requires a 4 character value\n");
> -				return 0;
> -			}
> -			break;
> -		case opt_creator:
> -			if (match_fourchar(&args[0], &hsb->s_creator)) {
> -				pr_err("creator requires a 4 character value\n");
> -				return 0;
> -			}
> -			break;
> -		case opt_quiet:
> -			hsb->s_quiet = 1;
> -			break;
> -		case opt_codepage:
> -			if (hsb->nls_disk) {
> -				pr_err("unable to change codepage\n");
> -				return 0;
> -			}
> -			p = match_strdup(&args[0]);
> -			if (p)
> -				hsb->nls_disk = load_nls(p);
> -			if (!hsb->nls_disk) {
> -				pr_err("unable to load codepage \"%s\"\n", p);
> -				kfree(p);
> -				return 0;
> -			}
> -			kfree(p);
> -			break;
> -		case opt_iocharset:
> -			if (hsb->nls_io) {
> -				pr_err("unable to change iocharset\n");
> -				return 0;
> -			}
> -			p = match_strdup(&args[0]);
> -			if (p)
> -				hsb->nls_io = load_nls(p);
> -			if (!hsb->nls_io) {
> -				pr_err("unable to load iocharset \"%s\"\n", p);
> -				kfree(p);
> -				return 0;
> -			}
> -			kfree(p);
> -			break;
> -		default:
> -			return 0;
> -		}
> -	}
> +	struct hfs_sb_info *hsb = fc->s_fs_info;
> +	struct fs_parse_result result;
> +	int opt;
> +
> +	/* hfs does not honor any fs-specific options on remount */
> +	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE)
> +		return 0;
>  
> -	if (hsb->nls_disk && !hsb->nls_io) {
> -		hsb->nls_io = load_nls_default();
> +	opt = fs_parse(fc, hfs_param_spec, param, &result);
> +	if (opt < 0)
> +		return opt;
> +
> +	switch (opt) {
> +	case opt_uid:
> +		hsb->s_uid = result.uid;
> +		break;
> +	case opt_gid:
> +		hsb->s_gid = result.gid;
> +		break;
> +	case opt_umask:
> +		hsb->s_file_umask = (umode_t)result.uint_32;
> +		hsb->s_dir_umask = (umode_t)result.uint_32;
> +		break;
> +	case opt_file_umask:
> +		hsb->s_file_umask = (umode_t)result.uint_32;
> +		break;
> +	case opt_dir_umask:
> +		hsb->s_dir_umask = (umode_t)result.uint_32;
> +		break;
> +	case opt_part:
> +		hsb->part = result.uint_32;
> +		break;
> +	case opt_session:
> +		hsb->session = result.uint_32;
> +		break;
> +	case opt_type:
> +		if (strlen(param->string) != 4) {
> +			pr_err("type requires a 4 character value\n");
> +			return -EINVAL;
> +		}
> +		memcpy(&hsb->s_type, param->string, 4);
> +		break;
> +	case opt_creator:
> +		if (strlen(param->string) != 4) {
> +			pr_err("creator requires a 4 character value\n");
> +			return -EINVAL;
> +		}
> +		memcpy(&hsb->s_creator, param->string, 4);
> +		break;
> +	case opt_quiet:
> +		hsb->s_quiet = 1;
> +		break;
> +	case opt_codepage:
> +		if (hsb->nls_disk) {
> +			pr_err("unable to change codepage\n");
> +			return -EINVAL;
> +		}
> +		hsb->nls_disk = load_nls(param->string);
> +		if (!hsb->nls_disk) {
> +			pr_err("unable to load codepage \"%s\"\n",
> +					param->string);
> +			return -EINVAL;
> +		}
> +		break;
> +	case opt_iocharset:
> +		if (hsb->nls_io) {
> +			pr_err("unable to change iocharset\n");
> +			return -EINVAL;
> +		}
> +		hsb->nls_io = load_nls(param->string);
>  		if (!hsb->nls_io) {
> -			pr_err("unable to load default iocharset\n");
> -			return 0;
> +			pr_err("unable to load iocharset \"%s\"\n",
> +					param->string);
> +			return -EINVAL;
>  		}
> +		break;
> +	default:
> +		return -EINVAL;
>  	}
> -	hsb->s_dir_umask &= 0777;
> -	hsb->s_file_umask &= 0577;
>  
> -	return 1;
> +	return 0;
>  }
>  
>  /*
> @@ -376,29 +310,24 @@ static int parse_options(char *options, struct hfs_sb_info *hsb)
>   * hfs_btree_init() to get the necessary data about the extents and
>   * catalog B-trees and, finally, reading the root inode into memory.
>   */
> -static int hfs_fill_super(struct super_block *sb, void *data, int silent)
> +static int hfs_fill_super(struct super_block *sb, struct fs_context *fc)
>  {
> -	struct hfs_sb_info *sbi;
> +	struct hfs_sb_info *sbi = HFS_SB(sb);
>  	struct hfs_find_data fd;
>  	hfs_cat_rec rec;
>  	struct inode *root_inode;
> +	int silent = fc->sb_flags & SB_SILENT;
>  	int res;
>  
> -	sbi = kzalloc(sizeof(struct hfs_sb_info), GFP_KERNEL);
> -	if (!sbi)
> -		return -ENOMEM;
> +	/* load_nls_default does not fail */
> +	if (sbi->nls_disk && !sbi->nls_io)
> +		sbi->nls_io = load_nls_default();
> +	sbi->s_dir_umask &= 0777;
> +	sbi->s_file_umask &= 0577;
>  
> -	sbi->sb = sb;
> -	sb->s_fs_info = sbi;
>  	spin_lock_init(&sbi->work_lock);
>  	INIT_DELAYED_WORK(&sbi->mdb_work, flush_mdb);
>  
> -	res = -EINVAL;
> -	if (!parse_options((char *)data, sbi)) {
> -		pr_err("unable to parse mount options\n");
> -		goto bail;
> -	}
> -
>  	sb->s_op = &hfs_super_operations;
>  	sb->s_xattr = hfs_xattr_handlers;
>  	sb->s_flags |= SB_NODIRATIME;
> @@ -451,18 +380,56 @@ static int hfs_fill_super(struct super_block *sb, void *data, int silent)
>  	return res;
>  }
>  
> -static struct dentry *hfs_mount(struct file_system_type *fs_type,
> -		      int flags, const char *dev_name, void *data)
> +static int hfs_get_tree(struct fs_context *fc)
>  {
> -	return mount_bdev(fs_type, flags, dev_name, data, hfs_fill_super);
> +	return get_tree_bdev(fc, hfs_fill_super);
> +}
> +
> +static void hfs_free_fc(struct fs_context *fc)
> +{
> +	kfree(fc->s_fs_info);
> +}
> +
> +static const struct fs_context_operations hfs_context_ops = {
> +	.parse_param	= hfs_parse_param,
> +	.get_tree	= hfs_get_tree,
> +	.reconfigure	= hfs_reconfigure,
> +	.free		= hfs_free_fc,
> +};
> +
> +static int hfs_init_fs_context(struct fs_context *fc)
> +{
> +	struct hfs_sb_info *hsb;
> +
> +	hsb = kzalloc(sizeof(struct hfs_sb_info), GFP_KERNEL);
> +	if (!hsb)
> +		return -ENOMEM;
> +
> +	fc->s_fs_info = hsb;
> +	fc->ops = &hfs_context_ops;
> +
> +	if (fc->purpose != FS_CONTEXT_FOR_RECONFIGURE) {
> +		/* initialize options with defaults */
> +		hsb->s_uid = current_uid();
> +		hsb->s_gid = current_gid();
> +		hsb->s_file_umask = 0133;
> +		hsb->s_dir_umask = 0022;
> +		hsb->s_type = cpu_to_be32(0x3f3f3f3f); /* == '????' */
> +		hsb->s_creator = cpu_to_be32(0x3f3f3f3f); /* == '????' */
> +		hsb->s_quiet = 0;
> +		hsb->part = -1;
> +		hsb->session = -1;
> +	}
> +
> +	return 0;
>  }
>  
>  static struct file_system_type hfs_fs_type = {
>  	.owner		= THIS_MODULE,
>  	.name		= "hfs",
> -	.mount		= hfs_mount,
>  	.kill_sb	= kill_block_super,
>  	.fs_flags	= FS_REQUIRES_DEV,
> +	.init_fs_context = hfs_init_fs_context,
>  };
>  MODULE_ALIAS_FS("hfs");
>  
> -- 
> 2.46.0
> 
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 5/5] hfsplus: convert hfsplus to use the new mount api
  2024-09-16 17:26 ` [PATCH 5/5] hfsplus: convert hfsplus " Eric Sandeen
@ 2024-09-17 16:11   ` Jan Kara
  0 siblings, 0 replies; 14+ messages in thread
From: Jan Kara @ 2024-09-17 16:11 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: linux-fsdevel, brauner

On Mon 16-09-24 13:26:22, Eric Sandeen wrote:
> Convert the hfsplus filesystem to use the new mount API.
> Tested by comparing random mount & remount options before and after
> the change.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/hfsplus/hfsplus_fs.h |   4 +-
>  fs/hfsplus/options.c    | 263 ++++++++++++++--------------------------
>  fs/hfsplus/super.c      |  84 ++++++++-----
>  3 files changed, 149 insertions(+), 202 deletions(-)
> 
> diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
> index 9e78f181c24f..53cb3c7dfc73 100644
> --- a/fs/hfsplus/hfsplus_fs.h
> +++ b/fs/hfsplus/hfsplus_fs.h
> @@ -21,6 +21,7 @@
>  #include <linux/mutex.h>
>  #include <linux/buffer_head.h>
>  #include <linux/blkdev.h>
> +#include <linux/fs_context.h>
>  #include "hfsplus_raw.h"
>  
>  #define DBG_BNODE_REFS	0x00000001
> @@ -496,8 +497,7 @@ long hfsplus_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
>  
>  /* options.c */
>  void hfsplus_fill_defaults(struct hfsplus_sb_info *opts);
> -int hfsplus_parse_options_remount(char *input, int *force);
> -int hfsplus_parse_options(char *input, struct hfsplus_sb_info *sbi);
> +int hfsplus_parse_param(struct fs_context *fc, struct fs_parameter *param);
>  int hfsplus_show_options(struct seq_file *seq, struct dentry *root);
>  
>  /* part_tbl.c */
> diff --git a/fs/hfsplus/options.c b/fs/hfsplus/options.c
> index c94a58762ad6..a66a09a56bf7 100644
> --- a/fs/hfsplus/options.c
> +++ b/fs/hfsplus/options.c
> @@ -12,7 +12,8 @@
>  #include <linux/string.h>
>  #include <linux/kernel.h>
>  #include <linux/sched.h>
> -#include <linux/parser.h>
> +#include <linux/fs_context.h>
> +#include <linux/fs_parser.h>
>  #include <linux/nls.h>
>  #include <linux/mount.h>
>  #include <linux/seq_file.h>
> @@ -23,26 +24,23 @@ enum {
>  	opt_creator, opt_type,
>  	opt_umask, opt_uid, opt_gid,
>  	opt_part, opt_session, opt_nls,
> -	opt_nodecompose, opt_decompose,
> -	opt_barrier, opt_nobarrier,
> -	opt_force, opt_err
> +	opt_decompose, opt_barrier,
> +	opt_force,
>  };
>  
> -static const match_table_t tokens = {
> -	{ opt_creator, "creator=%s" },
> -	{ opt_type, "type=%s" },
> -	{ opt_umask, "umask=%o" },
> -	{ opt_uid, "uid=%u" },
> -	{ opt_gid, "gid=%u" },
> -	{ opt_part, "part=%u" },
> -	{ opt_session, "session=%u" },
> -	{ opt_nls, "nls=%s" },
> -	{ opt_decompose, "decompose" },
> -	{ opt_nodecompose, "nodecompose" },
> -	{ opt_barrier, "barrier" },
> -	{ opt_nobarrier, "nobarrier" },
> -	{ opt_force, "force" },
> -	{ opt_err, NULL }
> +static const struct fs_parameter_spec hfs_param_spec[] = {
> +	fsparam_string	("creator",	opt_creator),
> +	fsparam_string	("type",	opt_type),
> +	fsparam_u32oct	("umask",	opt_umask),
> +	fsparam_u32	("uid",		opt_uid),
> +	fsparam_u32	("gid",		opt_gid),
> +	fsparam_u32	("part",	opt_part),
> +	fsparam_u32	("session",	opt_session),
> +	fsparam_string	("nls",		opt_nls),
> +	fsparam_flag_no	("decompose",	opt_decompose),
> +	fsparam_flag_no	("barrier",	opt_barrier),
> +	fsparam_flag	("force",	opt_force),
> +	{}
>  };
>  
>  /* Initialize an options object to reasonable defaults */
> @@ -60,162 +58,89 @@ void hfsplus_fill_defaults(struct hfsplus_sb_info *opts)
>  	opts->session = -1;
>  }
>  
> -/* convert a "four byte character" to a 32 bit int with error checks */
> -static inline int match_fourchar(substring_t *arg, u32 *result)
> +/* Parse options from mount. Returns nonzero errno on failure */
> +int hfsplus_parse_param(struct fs_context *fc, struct fs_parameter *param)
>  {
> -	if (arg->to - arg->from != 4)
> -		return -EINVAL;
> -	memcpy(result, arg->from, 4);
> -	return 0;
> -}
> -
> -int hfsplus_parse_options_remount(char *input, int *force)
> -{
> -	char *p;
> -	substring_t args[MAX_OPT_ARGS];
> -	int token;
> -
> -	if (!input)
> -		return 1;
> -
> -	while ((p = strsep(&input, ",")) != NULL) {
> -		if (!*p)
> -			continue;
> -
> -		token = match_token(p, tokens, args);
> -		switch (token) {
> -		case opt_force:
> -			*force = 1;
> -			break;
> -		default:
> -			break;
> +	struct hfsplus_sb_info *sbi = fc->s_fs_info;
> +	struct fs_parse_result result;
> +	int opt;
> +
> +	/*
> +	 * Only the force option is examined during remount, all others
> +	 * are ignored.
> +	 */
> +	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE &&
> +	    strncmp(param->key, "force", 5))
> +		return 0;
> +
> +	opt = fs_parse(fc, hfs_param_spec, param, &result);
> +	if (opt < 0)
> +		return opt;
> +
> +	switch (opt) {
> +	case opt_creator:
> +		if (strlen(param->string) != 4) {
> +			pr_err("creator requires a 4 character value\n");
> +			return -EINVAL;
>  		}
> -	}
> -
> -	return 1;
> -}
> -
> -/* Parse options from mount. Returns 0 on failure */
> -/* input is the options passed to mount() as a string */
> -int hfsplus_parse_options(char *input, struct hfsplus_sb_info *sbi)
> -{
> -	char *p;
> -	substring_t args[MAX_OPT_ARGS];
> -	int tmp, token;
> -
> -	if (!input)
> -		goto done;
> -
> -	while ((p = strsep(&input, ",")) != NULL) {
> -		if (!*p)
> -			continue;
> -
> -		token = match_token(p, tokens, args);
> -		switch (token) {
> -		case opt_creator:
> -			if (match_fourchar(&args[0], &sbi->creator)) {
> -				pr_err("creator requires a 4 character value\n");
> -				return 0;
> -			}
> -			break;
> -		case opt_type:
> -			if (match_fourchar(&args[0], &sbi->type)) {
> -				pr_err("type requires a 4 character value\n");
> -				return 0;
> -			}
> -			break;
> -		case opt_umask:
> -			if (match_octal(&args[0], &tmp)) {
> -				pr_err("umask requires a value\n");
> -				return 0;
> -			}
> -			sbi->umask = (umode_t)tmp;
> -			break;
> -		case opt_uid:
> -			if (match_int(&args[0], &tmp)) {
> -				pr_err("uid requires an argument\n");
> -				return 0;
> -			}
> -			sbi->uid = make_kuid(current_user_ns(), (uid_t)tmp);
> -			if (!uid_valid(sbi->uid)) {
> -				pr_err("invalid uid specified\n");
> -				return 0;
> -			} else {
> -				set_bit(HFSPLUS_SB_UID, &sbi->flags);
> -			}
> -			break;
> -		case opt_gid:
> -			if (match_int(&args[0], &tmp)) {
> -				pr_err("gid requires an argument\n");
> -				return 0;
> -			}
> -			sbi->gid = make_kgid(current_user_ns(), (gid_t)tmp);
> -			if (!gid_valid(sbi->gid)) {
> -				pr_err("invalid gid specified\n");
> -				return 0;
> -			} else {
> -				set_bit(HFSPLUS_SB_GID, &sbi->flags);
> -			}
> -			break;
> -		case opt_part:
> -			if (match_int(&args[0], &sbi->part)) {
> -				pr_err("part requires an argument\n");
> -				return 0;
> -			}
> -			break;
> -		case opt_session:
> -			if (match_int(&args[0], &sbi->session)) {
> -				pr_err("session requires an argument\n");
> -				return 0;
> -			}
> -			break;
> -		case opt_nls:
> -			if (sbi->nls) {
> -				pr_err("unable to change nls mapping\n");
> -				return 0;
> -			}
> -			p = match_strdup(&args[0]);
> -			if (p)
> -				sbi->nls = load_nls(p);
> -			if (!sbi->nls) {
> -				pr_err("unable to load nls mapping \"%s\"\n",
> -				       p);
> -				kfree(p);
> -				return 0;
> -			}
> -			kfree(p);
> -			break;
> -		case opt_decompose:
> -			clear_bit(HFSPLUS_SB_NODECOMPOSE, &sbi->flags);
> -			break;
> -		case opt_nodecompose:
> +		memcpy(&sbi->creator, param->string, 4);
> +		break;
> +	case opt_type:
> +		if (strlen(param->string) != 4) {
> +			pr_err("type requires a 4 character value\n");
> +			return -EINVAL;
> +		}
> +		memcpy(&sbi->type, param->string, 4);
> +		break;
> +	case opt_umask:
> +		sbi->umask = (umode_t)result.uint_32;
> +		break;
> +	case opt_uid:
> +		sbi->uid = result.uid;
> +		set_bit(HFSPLUS_SB_UID, &sbi->flags);
> +		break;
> +	case opt_gid:
> +		sbi->gid = result.gid;
> +		set_bit(HFSPLUS_SB_GID, &sbi->flags);
> +		break;
> +	case opt_part:
> +		sbi->part = result.uint_32;
> +		break;
> +	case opt_session:
> +		sbi->session = result.uint_32;
> +		break;
> +	case opt_nls:
> +		if (sbi->nls) {
> +			pr_err("unable to change nls mapping\n");
> +			return -EINVAL;
> +		}
> +		sbi->nls = load_nls(param->string);
> +		if (!sbi->nls) {
> +			pr_err("unable to load nls mapping \"%s\"\n",
> +			       param->string);
> +			return -EINVAL;
> +		}
> +		break;
> +	case opt_decompose:
> +		if (result.negated)
>  			set_bit(HFSPLUS_SB_NODECOMPOSE, &sbi->flags);
> -			break;
> -		case opt_barrier:
> -			clear_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags);
> -			break;
> -		case opt_nobarrier:
> +		else
> +			clear_bit(HFSPLUS_SB_NODECOMPOSE, &sbi->flags);
> +		break;
> +	case opt_barrier:
> +		if (result.negated)
>  			set_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags);
> -			break;
> -		case opt_force:
> -			set_bit(HFSPLUS_SB_FORCE, &sbi->flags);
> -			break;
> -		default:
> -			return 0;
> -		}
> -	}
> -
> -done:
> -	if (!sbi->nls) {
> -		/* try utf8 first, as this is the old default behaviour */
> -		sbi->nls = load_nls("utf8");
> -		if (!sbi->nls)
> -			sbi->nls = load_nls_default();
> -		if (!sbi->nls)
> -			return 0;
> +		else
> +			clear_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags);
> +		break;
> +	case opt_force:
> +		set_bit(HFSPLUS_SB_FORCE, &sbi->flags);
> +		break;
> +	default:
> +		return -EINVAL;
>  	}
>  
> -	return 1;
> +	return 0;
>  }
>  
>  int hfsplus_show_options(struct seq_file *seq, struct dentry *root)
> diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
> index 8f986f30a1ce..261af016efd3 100644
> --- a/fs/hfsplus/super.c
> +++ b/fs/hfsplus/super.c
> @@ -14,6 +14,7 @@
>  #include <linux/blkdev.h>
>  #include <linux/backing-dev.h>
>  #include <linux/fs.h>
> +#include <linux/fs_context.h>
>  #include <linux/slab.h>
>  #include <linux/vfs.h>
>  #include <linux/nls.h>
> @@ -332,34 +333,33 @@ static int hfsplus_statfs(struct dentry *dentry, struct kstatfs *buf)
>  	return 0;
>  }
>  
> -static int hfsplus_remount(struct super_block *sb, int *flags, char *data)
> +static int hfsplus_reconfigure(struct fs_context *fc)
>  {
> +	struct super_block *sb = fc->root->d_sb;
> +
>  	sync_filesystem(sb);
> -	if ((bool)(*flags & SB_RDONLY) == sb_rdonly(sb))
> +	if ((bool)(fc->sb_flags & SB_RDONLY) == sb_rdonly(sb))
>  		return 0;
> -	if (!(*flags & SB_RDONLY)) {
> -		struct hfsplus_vh *vhdr = HFSPLUS_SB(sb)->s_vhdr;
> -		int force = 0;
> -
> -		if (!hfsplus_parse_options_remount(data, &force))
> -			return -EINVAL;
> +	if (!(fc->sb_flags & SB_RDONLY)) {
> +		struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
> +		struct hfsplus_vh *vhdr = sbi->s_vhdr;
>  
>  		if (!(vhdr->attributes & cpu_to_be32(HFSPLUS_VOL_UNMNT))) {
>  			pr_warn("filesystem was not cleanly unmounted, running fsck.hfsplus is recommended.  leaving read-only.\n");
>  			sb->s_flags |= SB_RDONLY;
> -			*flags |= SB_RDONLY;
> -		} else if (force) {
> +			fc->sb_flags |= SB_RDONLY;
> +		} else if (test_bit(HFSPLUS_SB_FORCE, &sbi->flags)) {
>  			/* nothing */
>  		} else if (vhdr->attributes &
>  				cpu_to_be32(HFSPLUS_VOL_SOFTLOCK)) {
>  			pr_warn("filesystem is marked locked, leaving read-only.\n");
>  			sb->s_flags |= SB_RDONLY;
> -			*flags |= SB_RDONLY;
> +			fc->sb_flags |= SB_RDONLY;
>  		} else if (vhdr->attributes &
>  				cpu_to_be32(HFSPLUS_VOL_JOURNALED)) {
>  			pr_warn("filesystem is marked journaled, leaving read-only.\n");
>  			sb->s_flags |= SB_RDONLY;
> -			*flags |= SB_RDONLY;
> +			fc->sb_flags |= SB_RDONLY;
>  		}
>  	}
>  	return 0;
> @@ -373,38 +373,33 @@ static const struct super_operations hfsplus_sops = {
>  	.put_super	= hfsplus_put_super,
>  	.sync_fs	= hfsplus_sync_fs,
>  	.statfs		= hfsplus_statfs,
> -	.remount_fs	= hfsplus_remount,
>  	.show_options	= hfsplus_show_options,
>  };
>  
> -static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
> +static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
>  {
>  	struct hfsplus_vh *vhdr;
> -	struct hfsplus_sb_info *sbi;
> +	struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
>  	hfsplus_cat_entry entry;
>  	struct hfs_find_data fd;
>  	struct inode *root, *inode;
>  	struct qstr str;
> -	struct nls_table *nls = NULL;
> +	struct nls_table *nls;
>  	u64 last_fs_block, last_fs_page;
> +	int silent = fc->sb_flags & SB_SILENT;
>  	int err;
>  
> -	err = -ENOMEM;
> -	sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
> -	if (!sbi)
> -		goto out;
> -
> -	sb->s_fs_info = sbi;
>  	mutex_init(&sbi->alloc_mutex);
>  	mutex_init(&sbi->vh_mutex);
>  	spin_lock_init(&sbi->work_lock);
>  	INIT_DELAYED_WORK(&sbi->sync_work, delayed_sync_fs);
> -	hfsplus_fill_defaults(sbi);
>  
>  	err = -EINVAL;
> -	if (!hfsplus_parse_options(data, sbi)) {
> -		pr_err("unable to parse mount options\n");
> -		goto out_unload_nls;
> +	if (!sbi->nls) {
> +		/* try utf8 first, as this is the old default behaviour */
> +		sbi->nls = load_nls("utf8");
> +		if (!sbi->nls)
> +			sbi->nls = load_nls_default();
>  	}
>  
>  	/* temporarily use utf8 to correctly find the hidden dir below */
> @@ -616,7 +611,6 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
>  	unload_nls(sbi->nls);
>  	unload_nls(nls);
>  	kfree(sbi);
> -out:
>  	return err;
>  }
>  
> @@ -641,18 +635,46 @@ static void hfsplus_free_inode(struct inode *inode)
>  
>  #define HFSPLUS_INODE_SIZE	sizeof(struct hfsplus_inode_info)
>  
> -static struct dentry *hfsplus_mount(struct file_system_type *fs_type,
> -			  int flags, const char *dev_name, void *data)
> +static int hfsplus_get_tree(struct fs_context *fc)
> +{
> +	return get_tree_bdev(fc, hfsplus_fill_super);
> +}
> +
> +static void hfsplus_free_fc(struct fs_context *fc)
>  {
> -	return mount_bdev(fs_type, flags, dev_name, data, hfsplus_fill_super);
> +	kfree(fc->s_fs_info);
> +}
> +
> +static const struct fs_context_operations hfsplus_context_ops = {
> +	.parse_param	= hfsplus_parse_param,
> +	.get_tree	= hfsplus_get_tree,
> +	.reconfigure	= hfsplus_reconfigure,
> +	.free		= hfsplus_free_fc,
> +};
> +
> +static int hfsplus_init_fs_context(struct fs_context *fc)
> +{
> +	struct hfsplus_sb_info *sbi;
> +
> +	sbi = kzalloc(sizeof(struct hfsplus_sb_info), GFP_KERNEL);
> +	if (!sbi)
> +		return -ENOMEM;
> +
> +	if (fc->purpose != FS_CONTEXT_FOR_RECONFIGURE)
> +		hfsplus_fill_defaults(sbi);
> +
> +	fc->s_fs_info = sbi;
> +	fc->ops = &hfsplus_context_ops;
> +
> +	return 0;
>  }
>  
>  static struct file_system_type hfsplus_fs_type = {
>  	.owner		= THIS_MODULE,
>  	.name		= "hfsplus",
> -	.mount		= hfsplus_mount,
>  	.kill_sb	= kill_block_super,
>  	.fs_flags	= FS_REQUIRES_DEV,
> +	.init_fs_context = hfsplus_init_fs_context,
>  };
>  MODULE_ALIAS_FS("hfsplus");
>  
> -- 
> 2.46.0
> 
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/5] adfs, affs, befs, hfs, hfsplus: convert to new mount api
  2024-09-16 17:26 [PATCH 0/5] adfs, affs, befs, hfs, hfsplus: convert to new mount api Eric Sandeen
                   ` (4 preceding siblings ...)
  2024-09-16 17:26 ` [PATCH 5/5] hfsplus: convert hfsplus " Eric Sandeen
@ 2024-09-18  9:46 ` Christian Brauner
  5 siblings, 0 replies; 14+ messages in thread
From: Christian Brauner @ 2024-09-18  9:46 UTC (permalink / raw)
  To: linux-fsdevel, Eric Sandeen; +Cc: Christian Brauner

On Mon, 16 Sep 2024 13:26:17 -0400, Eric Sandeen wrote:
> These were all tested against images I created or obtained, using a
> script to test random combinations of valid and invalid mount and
> remount options, and comparing the results before and after the changes.
> 
> AFAICT, all parsing works as expected and behavior is unchanged.
> 
> (Changes since first send: fixing a couple string leaks, added hfs
> and hfsplus.)
> 
> [...]

Applied to the vfs.mount.api.v6.13 branch of the vfs/vfs.git tree.
Patches in the vfs.mount.api.v6.13 branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.mount.api.v6.13

[1/5] adfs: convert adfs to use the new mount api
      https://git.kernel.org/vfs/vfs/c/5fadeed64d27
[2/5] affs: convert affs to use the new mount api
      https://git.kernel.org/vfs/vfs/c/de25e36d83fc
[3/5] befs: convert befs to use the new mount api
      https://git.kernel.org/vfs/vfs/c/c3099e72bf4f
[4/5] hfs: convert hfs to use the new mount api
      https://git.kernel.org/vfs/vfs/c/c87d1f1aa91c
[5/5] hfsplus: convert hfsplus to use the new mount api
      https://git.kernel.org/vfs/vfs/c/3c8fb5d57b49

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2024-09-18  9:46 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-16 17:26 [PATCH 0/5] adfs, affs, befs, hfs, hfsplus: convert to new mount api Eric Sandeen
2024-09-16 17:26 ` [PATCH 1/5] adfs: convert adfs to use the " Eric Sandeen
2024-09-17 12:52   ` Jan Kara
2024-09-16 17:26 ` [PATCH 2/5] affs: convert affs " Eric Sandeen
2024-09-17 12:51   ` Jan Kara
2024-09-17 14:53   ` [PATCH 2/5 V2] " Eric Sandeen
2024-09-17 15:37     ` David Sterba
2024-09-16 17:26 ` [PATCH 3/5] befs: convert befs " Eric Sandeen
2024-09-17 15:50   ` Jan Kara
2024-09-16 17:26 ` [PATCH 4/5] hfs: convert hfs " Eric Sandeen
2024-09-17 15:55   ` Jan Kara
2024-09-16 17:26 ` [PATCH 5/5] hfsplus: convert hfsplus " Eric Sandeen
2024-09-17 16:11   ` Jan Kara
2024-09-18  9:46 ` [PATCH 0/5] adfs, affs, befs, hfs, hfsplus: convert to " Christian Brauner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).