[PATCH 0/3] use new mount API for bcachefs

[PATCH 0/3] use new mount API for bcachefs

[Thomas Bertschinger]

This updates bcachefs to use the new fs_context_operations functions for
mounting and remounting.

However, rather than using the parsing utilities defined in fs_parser.h,
I stick with using the existing options parsing code that bcachefs
already has. This simplifies the changes, since integrating the bcachefs
options with the generic parsing utilities in fs_context would otherwise
involve some tricky macro manipulations.

Following this series is a new test in Ktest [1] which exercises mount
and remount functionality to test the new code.

[1] https://evilpiepirate.org/git/ktest.git/

Thomas Bertschinger (3):
  bcachefs: add printbuf arg to bch2_parse_mount_opts()
  bcachefs: Add error code to defer option parsing
  bcachefs: use new mount API

 fs/bcachefs/chardev.c     |   4 +-
 fs/bcachefs/disk_groups.c |   2 +-
 fs/bcachefs/errcode.h     |   3 +-
 fs/bcachefs/fs.c          | 113 ++++++++++++++++++++++++++++-------
 fs/bcachefs/opts.c        | 120 ++++++++++++++++++++++++--------------
 fs/bcachefs/opts.h        |  12 +++-
 6 files changed, 185 insertions(+), 69 deletions(-)

-- 
2.45.0

[PATCH 1/3] bcachefs: add printbuf arg to bch2_parse_mount_opts()

[Thomas Bertschinger]

Mount options that take the name of a device that may be part of a
filesystem, for example "metadata_target", cannot be validated until
after the filesystem has been opened. However, an attempt to parse those
options may be made prior to the filesystem being opened.

This change adds a printbuf parameter to bch2_parse_mount_opts() which
will be used to save those mount options, when they are supplied prior
to the FS being opened, so that they can be parsed later.

This functionality is not currently needed, but will be used after
bcachefs starts using the new mount API to parse mount options. This is
because using the new mount API, we will process mount options prior to
opening the FS, but the new API doesn't provide a convenient way to
"replay" mount option parsing. So we save these options ourselves to
accomplish this.

This change also splits out the code to parse a single option into
bch2_parse_one_mount_opt(), which will be useful when using the new
mount API which deals with a single mount option at a time.

Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>
---
 fs/bcachefs/chardev.c |   4 +-
 fs/bcachefs/fs.c      |   6 +--
 fs/bcachefs/opts.c    | 105 +++++++++++++++++++++++++-----------------
 fs/bcachefs/opts.h    |   5 +-
 4 files changed, 71 insertions(+), 49 deletions(-)

diff --git a/fs/bcachefs/chardev.c b/fs/bcachefs/chardev.c
index 5040c584cd72..b59570f688db 100644
--- a/fs/bcachefs/chardev.c
+++ b/fs/bcachefs/chardev.c
@@ -227,7 +227,7 @@ static long bch2_ioctl_fsck_offline(struct bch_ioctl_fsck_offline __user *user_a
 		}
 
 		ret =   PTR_ERR_OR_ZERO(optstr) ?:
-			bch2_parse_mount_opts(NULL, &thr->opts, optstr);
+			bch2_parse_mount_opts(NULL, &thr->opts, NULL, optstr);
 		kfree(optstr);
 
 		if (ret)
@@ -862,7 +862,7 @@ static long bch2_ioctl_fsck_online(struct bch_fs *c,
 		char *optstr = strndup_user((char __user *)(unsigned long) arg.opts, 1 << 16);
 
 		ret =   PTR_ERR_OR_ZERO(optstr) ?:
-			bch2_parse_mount_opts(c, &thr->opts, optstr);
+			bch2_parse_mount_opts(c, &thr->opts, NULL, optstr);
 		kfree(optstr);
 
 		if (ret)
diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
index 96040a95cf46..662d0c9bb3c2 100644
--- a/fs/bcachefs/fs.c
+++ b/fs/bcachefs/fs.c
@@ -1714,7 +1714,7 @@ static int bch2_remount(struct super_block *sb, int *flags, char *data)
 	struct bch_opts opts = bch2_opts_empty();
 	int ret;
 
-	ret = bch2_parse_mount_opts(c, &opts, data);
+	ret = bch2_parse_mount_opts(c, &opts, NULL, data);
 	if (ret)
 		goto err;
 
@@ -1887,7 +1887,7 @@ static struct dentry *bch2_mount(struct file_system_type *fs_type,
 
 	opt_set(opts, read_only, (flags & SB_RDONLY) != 0);
 
-	ret = bch2_parse_mount_opts(NULL, &opts, data);
+	ret = bch2_parse_mount_opts(NULL, &opts, NULL, data);
 	if (ret) {
 		ret = bch2_err_class(ret);
 		return ERR_PTR(ret);
@@ -1921,7 +1921,7 @@ static struct dentry *bch2_mount(struct file_system_type *fs_type,
 	}
 
 	/* Some options can't be parsed until after the fs is started: */
-	ret = bch2_parse_mount_opts(c, &opts, data);
+	ret = bch2_parse_mount_opts(c, &opts, NULL, data);
 	if (ret) {
 		bch2_fs_stop(c);
 		sb = ERR_PTR(ret);
diff --git a/fs/bcachefs/opts.c b/fs/bcachefs/opts.c
index bb068fd72465..e794706276cf 100644
--- a/fs/bcachefs/opts.c
+++ b/fs/bcachefs/opts.c
@@ -460,14 +460,70 @@ int bch2_opts_check_may_set(struct bch_fs *c)
 	return 0;
 }
 
+int bch2_parse_one_mount_opt(struct bch_fs *c, struct bch_opts *opts,
+			     struct printbuf *parse_later,
+			     const char *name, const char *val)
+{
+	struct printbuf err = PRINTBUF;
+	u64 v;
+	int ret, id;
+
+	id = bch2_mount_opt_lookup(name);
+
+	/* Check for the form "noopt", negation of a boolean opt: */
+	if (id < 0 &&
+	    !val &&
+	    !strncmp("no", name, 2)) {
+		id = bch2_mount_opt_lookup(name + 2);
+		val = "0";
+	}
+
+	/* Unknown options are ignored: */
+	if (id < 0)
+		return 0;
+
+	if (!(bch2_opt_table[id].flags & OPT_MOUNT))
+		goto bad_opt;
+
+	if (id == Opt_acl &&
+	    !IS_ENABLED(CONFIG_BCACHEFS_POSIX_ACL))
+		goto bad_opt;
+
+	if ((id == Opt_usrquota ||
+	     id == Opt_grpquota) &&
+	    !IS_ENABLED(CONFIG_BCACHEFS_QUOTA))
+		goto bad_opt;
+
+	ret = bch2_opt_parse(c, &bch2_opt_table[id], val, &v, &err);
+	if (ret < 0)
+		goto bad_val;
+
+	if (opts)
+		bch2_opt_set_by_id(opts, id, v);
+
+	ret = 0;
+	goto out;
+
+bad_opt:
+	pr_err("Bad mount option %s", name);
+	ret = -BCH_ERR_option_name;
+	goto out;
+
+bad_val:
+	pr_err("Invalid mount option %s", err.buf);
+	ret = -BCH_ERR_option_value;
+
+out:
+	printbuf_exit(&err);
+	return ret;
+}
+
 int bch2_parse_mount_opts(struct bch_fs *c, struct bch_opts *opts,
-			  char *options)
+			  struct printbuf *parse_later, char *options)
 {
 	char *copied_opts, *copied_opts_start;
 	char *opt, *name, *val;
-	int ret, id;
-	struct printbuf err = PRINTBUF;
-	u64 v;
+	int ret;
 
 	if (!options)
 		return 0;
@@ -488,53 +544,16 @@ int bch2_parse_mount_opts(struct bch_fs *c, struct bch_opts *opts,
 		name	= strsep(&opt, "=");
 		val	= opt;
 
-		id = bch2_mount_opt_lookup(name);
-
-		/* Check for the form "noopt", negation of a boolean opt: */
-		if (id < 0 &&
-		    !val &&
-		    !strncmp("no", name, 2)) {
-			id = bch2_mount_opt_lookup(name + 2);
-			val = "0";
-		}
-
-		/* Unknown options are ignored: */
-		if (id < 0)
-			continue;
-
-		if (!(bch2_opt_table[id].flags & OPT_MOUNT))
-			goto bad_opt;
-
-		if (id == Opt_acl &&
-		    !IS_ENABLED(CONFIG_BCACHEFS_POSIX_ACL))
-			goto bad_opt;
-
-		if ((id == Opt_usrquota ||
-		     id == Opt_grpquota) &&
-		    !IS_ENABLED(CONFIG_BCACHEFS_QUOTA))
-			goto bad_opt;
-
-		ret = bch2_opt_parse(c, &bch2_opt_table[id], val, &v, &err);
+		ret = bch2_parse_one_mount_opt(c, opts, parse_later, name, val);
 		if (ret < 0)
-			goto bad_val;
-
-		bch2_opt_set_by_id(opts, id, v);
+			goto out;
 	}
 
 	ret = 0;
 	goto out;
 
-bad_opt:
-	pr_err("Bad mount option %s", name);
-	ret = -BCH_ERR_option_name;
-	goto out;
-bad_val:
-	pr_err("Invalid mount option %s", err.buf);
-	ret = -BCH_ERR_option_value;
-	goto out;
 out:
 	kfree(copied_opts_start);
-	printbuf_exit(&err);
 	return ret;
 }
 
diff --git a/fs/bcachefs/opts.h b/fs/bcachefs/opts.h
index f902793e1810..03358655d9ab 100644
--- a/fs/bcachefs/opts.h
+++ b/fs/bcachefs/opts.h
@@ -566,7 +566,10 @@ void bch2_opt_to_text(struct printbuf *, struct bch_fs *, struct bch_sb *,
 
 int bch2_opt_check_may_set(struct bch_fs *, int, u64);
 int bch2_opts_check_may_set(struct bch_fs *);
-int bch2_parse_mount_opts(struct bch_fs *, struct bch_opts *, char *);
+int bch2_parse_one_mount_opt(struct bch_fs *, struct bch_opts *,
+			     struct printbuf *, const char *, const char *);
+int bch2_parse_mount_opts(struct bch_fs *, struct bch_opts *, struct printbuf *,
+			  char *);
 
 /* inode opts: */
 
-- 
2.45.0

[PATCH 2/3] bcachefs: Add error code to defer option parsing

[Thomas Bertschinger]

This introduces a new error code, option_needs_open_fs, which is used to
indicate that an attempt was made to parse a mount option prior to
opening a filesystem, when that mount option requires an open filesystem
in order to be validated.

Returning this error results in bch2_parse_one_mount_opt() saving that
option for later parsing, after the filesystem is opened.

Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>
---
 fs/bcachefs/disk_groups.c |  2 +-
 fs/bcachefs/errcode.h     |  3 ++-
 fs/bcachefs/opts.c        | 15 +++++++++++++++
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/fs/bcachefs/disk_groups.c b/fs/bcachefs/disk_groups.c
index 521a86df5e52..5df8de0b8c02 100644
--- a/fs/bcachefs/disk_groups.c
+++ b/fs/bcachefs/disk_groups.c
@@ -511,7 +511,7 @@ int bch2_opt_target_parse(struct bch_fs *c, const char *val, u64 *res,
 		return -EINVAL;
 
 	if (!c)
-		return 0;
+		return -BCH_ERR_option_needs_open_fs;
 
 	if (!strlen(val) || !strcmp(val, "none")) {
 		*res = 0;
diff --git a/fs/bcachefs/errcode.h b/fs/bcachefs/errcode.h
index 58612abf7927..a268af3e52bf 100644
--- a/fs/bcachefs/errcode.h
+++ b/fs/bcachefs/errcode.h
@@ -257,7 +257,8 @@
 	x(BCH_ERR_nopromote,		nopromote_no_writes)			\
 	x(BCH_ERR_nopromote,		nopromote_enomem)			\
 	x(0,				need_inode_lock)			\
-	x(0,				invalid_snapshot_node)
+	x(0,				invalid_snapshot_node)			\
+	x(0,				option_needs_open_fs)
 
 enum bch_errcode {
 	BCH_ERR_START		= 2048,
diff --git a/fs/bcachefs/opts.c b/fs/bcachefs/opts.c
index e794706276cf..e10fc1da71b1 100644
--- a/fs/bcachefs/opts.c
+++ b/fs/bcachefs/opts.c
@@ -378,6 +378,10 @@ int bch2_opt_parse(struct bch_fs *c,
 		break;
 	case BCH_OPT_FN:
 		ret = opt->fn.parse(c, val, res, err);
+
+		if (ret == -BCH_ERR_option_needs_open_fs)
+			return ret;
+
 		if (ret < 0) {
 			if (err)
 				prt_printf(err, "%s: parse error",
@@ -495,6 +499,17 @@ int bch2_parse_one_mount_opt(struct bch_fs *c, struct bch_opts *opts,
 		goto bad_opt;
 
 	ret = bch2_opt_parse(c, &bch2_opt_table[id], val, &v, &err);
+	if (ret == -BCH_ERR_option_needs_open_fs && parse_later) {
+		prt_printf(parse_later, "%s=%s,", name, val);
+		if (parse_later->allocation_failure) {
+			ret = -ENOMEM;
+			goto out;
+		}
+
+		ret = 0;
+		goto out;
+	}
+
 	if (ret < 0)
 		goto bad_val;
 
-- 
2.45.0

[PATCH 3/3] bcachefs: use new mount API

[Thomas Bertschinger]

This updates bcachefs to use the new mount API:

- Update the file_system_type to use the new init_fs_context()
  function.

- Define the new fs_context_operations functions.

- No longer register bch2_mount() and bch2_remount(); these are now
  called via the new fs_context functions.

- Define a new helper type, bch2_opts_parse that includes a struct
  bch_opts and additionally a printbuf used to save options that can't
  be parsed until after the FS is opened. This enables us to parse as
  many options as possible prior to opening the filesystem while saving
  those options that need the open FS for later parsing.

Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>
---
 fs/bcachefs/fs.c   | 113 ++++++++++++++++++++++++++++++++++++---------
 fs/bcachefs/opts.h |   7 +++
 2 files changed, 99 insertions(+), 21 deletions(-)

diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
index 662d0c9bb3c2..29aca0d961b3 100644
--- a/fs/bcachefs/fs.c
+++ b/fs/bcachefs/fs.c
@@ -31,6 +31,7 @@
 #include <linux/backing-dev.h>
 #include <linux/exportfs.h>
 #include <linux/fiemap.h>
+#include <linux/fs_context.h>
 #include <linux/module.h>
 #include <linux/pagemap.h>
 #include <linux/posix_acl.h>
@@ -1708,16 +1709,12 @@ static struct bch_fs *bch2_path_to_fs(const char *path)
 	return c ?: ERR_PTR(-ENOENT);
 }
 
-static int bch2_remount(struct super_block *sb, int *flags, char *data)
+static int bch2_remount(struct super_block *sb, int *flags,
+			struct bch_opts opts)
 {
 	struct bch_fs *c = sb->s_fs_info;
-	struct bch_opts opts = bch2_opts_empty();
 	int ret;
 
-	ret = bch2_parse_mount_opts(c, &opts, NULL, data);
-	if (ret)
-		goto err;
-
 	opt_set(opts, read_only, (*flags & SB_RDONLY) != 0);
 
 	if (opts.read_only != c->opts.read_only) {
@@ -1843,7 +1840,6 @@ static const struct super_operations bch_super_operations = {
 	.statfs		= bch2_statfs,
 	.show_devname	= bch2_show_devname,
 	.show_options	= bch2_show_options,
-	.remount_fs	= bch2_remount,
 	.put_super	= bch2_put_super,
 	.freeze_fs	= bch2_freeze,
 	.unfreeze_fs	= bch2_unfreeze,
@@ -1877,22 +1873,17 @@ static int bch2_test_super(struct super_block *s, void *data)
 }
 
 static struct dentry *bch2_mount(struct file_system_type *fs_type,
-				 int flags, const char *dev_name, void *data)
+				 int flags, const char *dev_name,
+				 struct bch2_opts_parse opts_parse)
 {
 	struct bch_fs *c;
 	struct super_block *sb;
 	struct inode *vinode;
-	struct bch_opts opts = bch2_opts_empty();
+	struct bch_opts opts = opts_parse.opts;
 	int ret;
 
 	opt_set(opts, read_only, (flags & SB_RDONLY) != 0);
 
-	ret = bch2_parse_mount_opts(NULL, &opts, NULL, data);
-	if (ret) {
-		ret = bch2_err_class(ret);
-		return ERR_PTR(ret);
-	}
-
 	if (!dev_name || strlen(dev_name) == 0)
 		return ERR_PTR(-EINVAL);
 
@@ -1921,7 +1912,7 @@ static struct dentry *bch2_mount(struct file_system_type *fs_type,
 	}
 
 	/* Some options can't be parsed until after the fs is started: */
-	ret = bch2_parse_mount_opts(c, &opts, NULL, data);
+	ret = bch2_parse_mount_opts(c, &opts, NULL, opts_parse.parse_later.buf);
 	if (ret) {
 		bch2_fs_stop(c);
 		sb = ERR_PTR(ret);
@@ -2027,12 +2018,92 @@ static void bch2_kill_sb(struct super_block *sb)
 	bch2_fs_free(c);
 }
 
+static void bch2_fs_context_free(struct fs_context *fc)
+{
+	struct bch2_opts_parse *opts = fc->fs_private;
+
+	if (opts) {
+		printbuf_exit(&opts->parse_later);
+		kfree(opts);
+	}
+}
+
+static int bch2_fs_parse_param(struct fs_context *fc,
+			       struct fs_parameter *param)
+{
+	/*
+	 * the "source" param, i.e., the name of the device(s) to mount,
+	 * is handled by the VFS layer.
+	 */
+	if (!strcmp(param->key, "source"))
+		return -ENOPARAM;
+
+	struct bch2_opts_parse *opts = fc->fs_private;
+	struct bch_fs *c = NULL;
+
+	/* for reconfigure, we already have a struct bch_fs */
+	if (fc->root)
+		c = fc->root->d_sb->s_fs_info;
+
+	int ret = bch2_parse_one_mount_opt(c, &opts->opts,
+					   &opts->parse_later, param->key,
+					   param->string);
+
+	return bch2_err_class(ret);
+}
+
+static int bch2_fs_get_tree(struct fs_context *fc)
+{
+	struct bch2_opts_parse *opts = fc->fs_private;
+	const char *dev_name = fc->source;
+	struct dentry *root;
+
+	root = bch2_mount(fc->fs_type, fc->sb_flags, dev_name, *opts);
+
+	if (IS_ERR(root))
+		return PTR_ERR(root);
+
+	fc->root = root;
+
+	return 0;
+}
+
+static int bch2_fs_reconfigure(struct fs_context *fc)
+{
+	struct super_block *sb = fc->root->d_sb;
+	struct bch2_opts_parse *opts = fc->fs_private;
+
+	return bch2_remount(sb, &fc->sb_flags, opts->opts);
+}
+
+static const struct fs_context_operations bch2_context_ops = {
+	.free        = bch2_fs_context_free,
+	.parse_param = bch2_fs_parse_param,
+	.get_tree    = bch2_fs_get_tree,
+	.reconfigure = bch2_fs_reconfigure,
+};
+
+static int bch2_init_fs_context(struct fs_context *fc)
+{
+	struct bch2_opts_parse *opts = kzalloc(sizeof(*opts), GFP_KERNEL);
+
+	if (!opts)
+		return -ENOMEM;
+
+	opts->parse_later = PRINTBUF;
+
+	fc->ops = &bch2_context_ops;
+	fc->fs_private = opts;
+
+	return 0;
+}
+
 static struct file_system_type bcache_fs_type = {
-	.owner		= THIS_MODULE,
-	.name		= "bcachefs",
-	.mount		= bch2_mount,
-	.kill_sb	= bch2_kill_sb,
-	.fs_flags	= FS_REQUIRES_DEV,
+	.owner			= THIS_MODULE,
+	.name			= "bcachefs",
+	.init_fs_context	= bch2_init_fs_context,
+	.kill_sb		= bch2_kill_sb,
+	.fs_flags		= FS_REQUIRES_DEV,
 };
 
 MODULE_ALIAS_FS("bcachefs");
diff --git a/fs/bcachefs/opts.h b/fs/bcachefs/opts.h
index 03358655d9ab..cff35845aaf6 100644
--- a/fs/bcachefs/opts.h
+++ b/fs/bcachefs/opts.h
@@ -488,6 +488,13 @@ struct bch_opts {
 #undef x
 };
 
+struct bch2_opts_parse {
+	struct bch_opts opts;
+
+	/* to save opts that can't be parsed before the FS is opened: */
+	struct printbuf parse_later;
+};
+
 static const __maybe_unused struct bch_opts bch2_opts_default = {
 #define x(_name, _bits, _mode, _type, _sb_opt, _default, ...)		\
 	._name##_defined = true,					\
-- 
2.45.0

[PATCH KTEST] add test to exercise the new mount API for bcachefs

[Thomas Bertschinger]

Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>
---
 tests/bcachefs/single_device.ktest | 33 ++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/tests/bcachefs/single_device.ktest b/tests/bcachefs/single_device.ktest
index 7476a08..e170b8b 100755
--- a/tests/bcachefs/single_device.ktest
+++ b/tests/bcachefs/single_device.ktest
@@ -79,6 +79,39 @@ test_remount_ro_rw()
     check_counters ${ktest_scratch_dev[0]}
 }
 
+test_mount_options()
+{
+    local dev=${ktest_scratch_dev[0]}
+    set_watchdog 10
+
+    run_quiet "" bcachefs format -f --errors=panic $dev
+
+    echo "test: valid mount options"
+    mount -t bcachefs -o metadata_checksum=crc64,metadata_target=$(basename $dev) $dev /mnt
+    mount -t bcachefs | grep --quiet metadata_checksum
+    mount -t bcachefs | grep --quiet metadata_target
+
+    echo "test: valid remount options"
+    mount -o remount,ro,errors=continue /mnt
+    mount -t bcachefs | grep --quiet metadata_target
+    mount -t bcachefs | grep --quiet continue
+    mount -t bcachefs | grep --quiet ro
+    umount /mnt
+
+    echo "test: invalid mount options"
+    ! mount -t bcachefs -o metadata_checksum=invalid $dev /mnt
+    ! mount -t bcachefs -o promote_target=$(basename $dev),metadata_target=not_a_device $dev /mnt
+
+    echo "test: invalid remount options"
+    mount -t bcachefs $dev /mnt
+    ! mount -o remount,promote_target=not_a_device /mnt
+    ! mount -o remount,metadata_replicas=not_a_number /mnt
+    umount /mnt
+
+    bcachefs fsck -ny $dev
+    check_counters $dev
+}
+
 test_extent_merge2()
 {
     local p=/sys/module/bcachefs/parameters/debug_check_iterators
-- 
2.45.0